From 8e27ee9220883cf5a0629c752e1642daaba4ce14 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 22 Dec 2024 08:24:28 +0100 Subject: [PATCH 1/4] reftable/stack: don't perform auto-compaction with less than two tables In order to compact tables we need at least two tables. Bail out early from `reftable_stack_auto_compact()` in case we have less than two tables. In the original, `stack_table_sizes_for_compaction()` yields an array that has the same length as the number of tables. This array is then passed on to `suggest_compaction_segment()`, which returns an empty segment in case we have less than two tables. The segment is then passed to `segment_size()`, which will return `0` because both start and end of the segment are `0`. And because we only call `stack_compact_range()` in case we have a positive segment size we don't perform auto-compaction at all. Consequently, this change does not result in a user-visible change in behaviour when called with a single table. But when called with no tables this protects us against a potential out-of-memory error: `stack_table_sizes_for_compaction()` would try to allocate a zero-byte object when there aren't any tables, and that may lead to a `NULL` pointer on some platforms like NonStop which causes us to bail out with an out-of-memory error. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reftable/stack.c b/reftable/stack.c index 63976e5cea..ae274cd51c 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1552,6 +1552,9 @@ int reftable_stack_auto_compact(struct reftable_stack *st) struct segment seg; uint64_t *sizes; + if (st->merged->readers_len < 2) + return 0; + sizes = stack_table_sizes_for_compaction(st); if (!sizes) return REFTABLE_OUT_OF_MEMORY_ERROR; From 5ab83521cfe687e4295f5748f2c5d2aa7477efe5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 22 Dec 2024 08:24:29 +0100 Subject: [PATCH 2/4] reftable/merged: fix zero-sized allocation when there are no readers It was reported [1] that Git started to fail with an out-of-memory error when initializing repositories with the reftable backend on NonStop platforms. A bisect led to 802c0646ac (reftable/merged: handle allocation failures in `merged_table_init_iter()`, 2024-10-02), which changed how we allocate memory when initializing a merged table. The root cause of this seems to be that NonStop returns a `NULL` pointer when doing a zero-sized allocation. This would've already happened before the above change, but we never noticed because we did not check the result. Now we do notice and thus return an out-of-memory error to the caller. Fix the issue by skipping the allocation altogether in case there are no readers. [1]: <00ad01db5017$aa9ce340$ffd6a9c0$@nexbridge.com> Reported-by: Randall S. Becker Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/merged.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index 514d6facf4..5ff4bc35c2 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -238,14 +238,16 @@ int merged_table_init_iter(struct reftable_merged_table *mt, struct reftable_iterator *it, uint8_t typ) { - struct merged_subiter *subiters; + struct merged_subiter *subiters = NULL; struct merged_iter *mi = NULL; int ret; - REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len); - if (!subiters) { - ret = REFTABLE_OUT_OF_MEMORY_ERROR; - goto out; + if (mt->readers_len) { + REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len); + if (!subiters) { + ret = REFTABLE_OUT_OF_MEMORY_ERROR; + goto out; + } } for (size_t i = 0; i < mt->readers_len; i++) { From 2d3cb4b4b5401e2fd5a40600277f424032fc72f0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 22 Dec 2024 08:24:30 +0100 Subject: [PATCH 3/4] reftable/stack: fix zero-sized allocation when there are no readers Similar as the preceding commit, we may try to do a zero-sized allocation when reloading a reftable stack that ain't got any tables. It is implementation-defined whether malloc(3p) returns a NULL pointer in that case or a zero-sized object. In case it does return a NULL pointer though it causes us to think we have run into an out-of-memory situation, and thus we return an error. Fix this by only allocating arrays when they have at least one entry. Reported-by: Randall S. Becker Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index ae274cd51c..f51d3ec9d9 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -254,9 +254,9 @@ static int reftable_stack_reload_once(struct reftable_stack *st, int reuse_open) { size_t cur_len = !st->merged ? 0 : st->merged->readers_len; - struct reftable_reader **cur; + struct reftable_reader **cur = NULL; struct reftable_reader **reused = NULL; - struct reftable_reader **new_readers; + struct reftable_reader **new_readers = NULL; size_t reused_len = 0, reused_alloc = 0, names_len; size_t new_readers_len = 0; struct reftable_merged_table *new_merged = NULL; @@ -264,18 +264,22 @@ static int reftable_stack_reload_once(struct reftable_stack *st, int err = 0; size_t i; - cur = stack_copy_readers(st, cur_len); - if (!cur) { - err = REFTABLE_OUT_OF_MEMORY_ERROR; - goto done; + if (cur_len) { + cur = stack_copy_readers(st, cur_len); + if (!cur) { + err = REFTABLE_OUT_OF_MEMORY_ERROR; + goto done; + } } names_len = names_length(names); - new_readers = reftable_calloc(names_len, sizeof(*new_readers)); - if (!new_readers) { - err = REFTABLE_OUT_OF_MEMORY_ERROR; - goto done; + if (names_len) { + new_readers = reftable_calloc(names_len, sizeof(*new_readers)); + if (!new_readers) { + err = REFTABLE_OUT_OF_MEMORY_ERROR; + goto done; + } } while (*names) { From d7282891f542b58410a209230009182b9057af79 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 22 Dec 2024 08:24:31 +0100 Subject: [PATCH 4/4] reftable/basics: return NULL on zero-sized allocations In the preceding commits we have fixed a couple of issues when allocating zero-sized objects. These issues were masked by implementation-defined behaviour. Quoting malloc(3p): If size is 0, either: * A null pointer shall be returned and errno may be set to an implementation-defined value, or * A pointer to the allocated space shall be returned. The application shall ensure that the pointer is not used to access an object. So it is perfectly valid that implementations of this function may or may not return a NULL pointer in such a case. Adapt both `reftable_malloc()` and `reftable_realloc()` so that they return NULL pointers on zero-sized allocations. This should remove any implementation-defined behaviour in our allocators and thus allows us to detect such platform-specific issues more easily going forward. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/basics.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/reftable/basics.c b/reftable/basics.c index 9a949e5cf8..eab5553d93 100644 --- a/reftable/basics.c +++ b/reftable/basics.c @@ -16,6 +16,8 @@ static void (*reftable_free_ptr)(void *); void *reftable_malloc(size_t sz) { + if (!sz) + return NULL; if (reftable_malloc_ptr) return (*reftable_malloc_ptr)(sz); return malloc(sz); @@ -23,6 +25,11 @@ void *reftable_malloc(size_t sz) void *reftable_realloc(void *p, size_t sz) { + if (!sz) { + reftable_free(p); + return NULL; + } + if (reftable_realloc_ptr) return (*reftable_realloc_ptr)(p, sz); return realloc(p, sz);