reftable: avoid leaks on realloc error
When realloc(3) fails, it returns NULL and keeps the original allocation intact. REFTABLE_ALLOC_GROW overwrites both the original pointer and the allocation count variable in that case, simultaneously leaking the original allocation and misrepresenting the number of storable items. parse_names() and reftable_buf_add() avoid leaking by restoring the original pointer value on failure, but all other callers seem to be OK with losing the old allocation. Add a new variant of the macro, REFTABLE_ALLOC_GROW_OR_NULL, which plugs the leak and zeros the allocation counter. Use it for those callers. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe committed
Dec 28, 2024 at 10:47 UTC
8db127d43f5b0eff254a851f9c966b7b85d91992
7 files changed
+61
-16
reftable/basics.h
+10
@@ -129,6 +129,16 @@ char *reftable_strdup(const char *str);
129
REFTABLE_REALLOC_ARRAY(x, alloc); \
130
} \
131
} while (0)
132
+
133
+#define REFTABLE_ALLOC_GROW_OR_NULL(x, nr, alloc) do { \
134
+ void *reftable_alloc_grow_or_null_orig_ptr = (x); \
135
+ REFTABLE_ALLOC_GROW((x), (nr), (alloc)); \
136
+ if (!(x)) { \
137
+ reftable_free(reftable_alloc_grow_or_null_orig_ptr); \
138
+ alloc = 0; \
139
+ } \
140
+} while (0)
141
+
142
#define REFTABLE_FREE_AND_NULL(p) do { reftable_free(p); (p) = NULL; } while (0)
143
144
#ifndef REFTABLE_ALLOW_BANNED_ALLOCATORS
reftable/block.c
+6
-4
@@ -53,7 +53,8 @@ static int block_writer_register_restart(struct block_writer *w, int n,
53
if (2 + 3 * rlen + n > w->block_size - w->next)
54
return -1;
55
if (is_restart) {
56
- REFTABLE_ALLOC_GROW(w->restarts, w->restart_len + 1, w->restart_cap);
56
+ REFTABLE_ALLOC_GROW_OR_NULL(w->restarts, w->restart_len + 1,
57
+ w->restart_cap);
58
if (!w->restarts)
59
return REFTABLE_OUT_OF_MEMORY_ERROR;
60
w->restarts[w->restart_len++] = w->next;
@@ -176,7 +177,8 @@ int block_writer_finish(struct block_writer *w)
177
* is guaranteed to return `Z_STREAM_END`.
178
*/
179
compressed_len = deflateBound(w->zstream, src_len);
179
- REFTABLE_ALLOC_GROW(w->compressed, compressed_len, w->compressed_cap);
180
+ REFTABLE_ALLOC_GROW_OR_NULL(w->compressed, compressed_len,
181
+ w->compressed_cap);
182
if (!w->compressed) {
183
ret = REFTABLE_OUT_OF_MEMORY_ERROR;
184
return ret;
@@ -235,8 +237,8 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
237
uLong src_len = block->len - block_header_skip;
238
239
/* Log blocks specify the *uncompressed* size in their header. */
238
- REFTABLE_ALLOC_GROW(br->uncompressed_data, sz,
239
- br->uncompressed_cap);
240
+ REFTABLE_ALLOC_GROW_OR_NULL(br->uncompressed_data, sz,
241
+ br->uncompressed_cap);
242
if (!br->uncompressed_data) {
243
err = REFTABLE_OUT_OF_MEMORY_ERROR;
244
goto done;
reftable/pq.c
+1
-1
@@ -49,7 +49,7 @@ int merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry
49
{
50
size_t i = 0;
51
52
- REFTABLE_ALLOC_GROW(pq->heap, pq->len + 1, pq->cap);
52
+ REFTABLE_ALLOC_GROW_OR_NULL(pq->heap, pq->len + 1, pq->cap);
53
if (!pq->heap)
54
return REFTABLE_OUT_OF_MEMORY_ERROR;
55
pq->heap[pq->len++] = *e;
reftable/record.c
+6
-6
@@ -246,8 +246,8 @@ static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
246
if (src->refname) {
247
size_t refname_len = strlen(src->refname);
248
249
- REFTABLE_ALLOC_GROW(ref->refname, refname_len + 1,
250
- ref->refname_cap);
249
+ REFTABLE_ALLOC_GROW_OR_NULL(ref->refname, refname_len + 1,
250
+ ref->refname_cap);
251
if (!ref->refname) {
252
err = REFTABLE_OUT_OF_MEMORY_ERROR;
253
goto out;
@@ -385,7 +385,7 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
385
SWAP(r->refname, refname);
386
SWAP(r->refname_cap, refname_cap);
387
388
- REFTABLE_ALLOC_GROW(r->refname, key.len + 1, r->refname_cap);
388
+ REFTABLE_ALLOC_GROW_OR_NULL(r->refname, key.len + 1, r->refname_cap);
389
if (!r->refname) {
390
err = REFTABLE_OUT_OF_MEMORY_ERROR;
391
goto done;
@@ -839,7 +839,7 @@ static int reftable_log_record_decode(void *rec, struct reftable_buf key,
839
if (key.len <= 9 || key.buf[key.len - 9] != 0)
840
return REFTABLE_FORMAT_ERROR;
841
842
- REFTABLE_ALLOC_GROW(r->refname, key.len - 8, r->refname_cap);
842
+ REFTABLE_ALLOC_GROW_OR_NULL(r->refname, key.len - 8, r->refname_cap);
843
if (!r->refname) {
844
err = REFTABLE_OUT_OF_MEMORY_ERROR;
845
goto done;
@@ -947,8 +947,8 @@ static int reftable_log_record_decode(void *rec, struct reftable_buf key,
947
}
948
string_view_consume(&in, n);
949
950
- REFTABLE_ALLOC_GROW(r->value.update.message, scratch->len + 1,
951
- r->value.update.message_cap);
950
+ REFTABLE_ALLOC_GROW_OR_NULL(r->value.update.message, scratch->len + 1,
951
+ r->value.update.message_cap);
952
if (!r->value.update.message) {
953
err = REFTABLE_OUT_OF_MEMORY_ERROR;
954
goto done;
reftable/stack.c
+5
-3
@@ -317,7 +317,9 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
317
* thus need to keep them alive here, which we
318
* do by bumping their refcount.
319
*/
320
- REFTABLE_ALLOC_GROW(reused, reused_len + 1, reused_alloc);
320
+ REFTABLE_ALLOC_GROW_OR_NULL(reused,
321
+ reused_len + 1,
322
+ reused_alloc);
323
if (!reused) {
324
err = REFTABLE_OUT_OF_MEMORY_ERROR;
325
goto done;
@@ -949,8 +951,8 @@ int reftable_addition_add(struct reftable_addition *add,
951
if (err < 0)
952
goto done;
953
952
- REFTABLE_ALLOC_GROW(add->new_tables, add->new_tables_len + 1,
953
- add->new_tables_cap);
954
+ REFTABLE_ALLOC_GROW_OR_NULL(add->new_tables, add->new_tables_len + 1,
955
+ add->new_tables_cap);
956
if (!add->new_tables) {
957
err = REFTABLE_OUT_OF_MEMORY_ERROR;
958
goto done;
reftable/writer.c
+3
-2
@@ -254,7 +254,8 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
254
if (key->offset_len > 0 && key->offsets[key->offset_len - 1] == off)
255
return 0;
256
257
- REFTABLE_ALLOC_GROW(key->offsets, key->offset_len + 1, key->offset_cap);
257
+ REFTABLE_ALLOC_GROW_OR_NULL(key->offsets, key->offset_len + 1,
258
+ key->offset_cap);
259
if (!key->offsets)
260
return REFTABLE_OUT_OF_MEMORY_ERROR;
261
key->offsets[key->offset_len++] = off;
@@ -820,7 +821,7 @@ static int writer_flush_nonempty_block(struct reftable_writer *w)
821
* Note that this also applies when flushing index blocks, in which
822
* case we will end up with a multi-level index.
823
*/
823
- REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap);
824
+ REFTABLE_ALLOC_GROW_OR_NULL(w->index, w->index_len + 1, w->index_cap);
825
if (!w->index)
826
return REFTABLE_OUT_OF_MEMORY_ERROR;
827
t/unit-tests/t-reftable-basics.c
+30
@@ -20,6 +20,11 @@ static int integer_needle_lesseq(size_t i, void *_args)
20
return args->needle <= args->haystack[i];
21
}
22
23
+static void *realloc_stub(void *p UNUSED, size_t size UNUSED)
24
+{
25
+ return NULL;
26
+}
27
+
28
int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
29
{
30
if_test ("binary search with binsearch works") {
@@ -141,5 +146,30 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
146
check_int(in, ==, out);
147
}
148
149
+ if_test ("REFTABLE_ALLOC_GROW_OR_NULL works") {
150
+ int *arr = NULL;
151
+ size_t alloc = 0, old_alloc;
152
+
153
+ REFTABLE_ALLOC_GROW_OR_NULL(arr, 1, alloc);
154
+ check(arr != NULL);
155
+ check_uint(alloc, >=, 1);
156
+ arr[0] = 42;
157
+
158
+ old_alloc = alloc;
159
+ REFTABLE_ALLOC_GROW_OR_NULL(arr, old_alloc + 1, alloc);
160
+ check(arr != NULL);
161
+ check_uint(alloc, >, old_alloc);
162
+ arr[alloc - 1] = 42;
163
+
164
+ old_alloc = alloc;
165
+ reftable_set_alloc(malloc, realloc_stub, free);
166
+ REFTABLE_ALLOC_GROW_OR_NULL(arr, old_alloc + 1, alloc);
167
+ check(arr == NULL);
168
+ check_uint(alloc, ==, 0);
169
+ reftable_set_alloc(malloc, realloc, free);
170
+
171
+ reftable_free(arr);
172
+ }
173
+
174
return test_done();
175
}