diff mbox series

[3/4] reftable/stack: fix zero-sized allocation when there are no readers

Message ID 20241221-b4-pks-reftable-oom-fix-without-readers-v1-3-12db83a3267c@pks.im (mailing list archive)
State New
Headers show
Series reftable: fix out-of-memory errors on NonStop | expand

Commit Message

Patrick Steinhardt Dec. 21, 2024, 11:50 a.m. UTC
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.
Refactor the code so that we don't try to access those arrays in case
they are empty.

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)
diff mbox series

Patch

diff --git a/reftable/stack.c b/reftable/stack.c
index 6ca21965d8e1135d986043113d465abd14cd532c..8328bfc58e9c207983ce88355d6110c40be3fac1 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -270,40 +270,42 @@  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;
 	struct reftable_buf table_path = REFTABLE_BUF_INIT;
 	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) {
+	for (size_t i = 0; i < names_len; i++) {
 		struct reftable_reader *rd = NULL;
-		const char *name = *names++;
+		const char *name = names[i];
 
 		/* this is linear; we assume compaction keeps the number of
 		   tables under control so this is not quadratic. */
-		for (i = 0; reuse_open && i < cur_len; i++) {
-			if (cur[i] && 0 == strcmp(cur[i]->name, name)) {
-				rd = cur[i];
-				cur[i] = NULL;
+		for (size_t j = 0; reuse_open && j < cur_len; j++) {
+			if (cur[j] && 0 == strcmp(cur[j]->name, name)) {
+				rd = cur[j];
+				cur[j] = NULL;
 
 				/*
 				 * When reloading the stack fails, we end up
@@ -357,7 +359,7 @@  static int reftable_stack_reload_once(struct reftable_stack *st,
 	 * file of such an open reader wouldn't have been possible to be
 	 * unlinked by the compacting process.
 	 */
-	for (i = 0; i < cur_len; i++) {
+	for (size_t i = 0; i < cur_len; i++) {
 		if (cur[i]) {
 			const char *name = reader_name(cur[i]);
 
@@ -388,11 +390,11 @@  static int reftable_stack_reload_once(struct reftable_stack *st,
 	 * happen on the successful case, because on the unsuccessful one we
 	 * decrement their refcount via `new_readers`.
 	 */
-	for (i = 0; i < reused_len; i++)
+	for (size_t i = 0; i < reused_len; i++)
 		reftable_reader_decref(reused[i]);
 
 done:
-	for (i = 0; i < new_readers_len; i++)
+	for (size_t i = 0; i < new_readers_len; i++)
 		reftable_reader_decref(new_readers[i]);
 	reftable_free(new_readers);
 	reftable_free(reused);