From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Wed, 31 Mar 2021 13:04:47 +0000 (+0300)
Subject: lib: data-stack - Move frame memory allocation inside data stack block
X-Git-Tag: 2.3.16~241
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=62fa24f787616ef5b5f8b8ffd5d034778c35f1bb;p=thirdparty%2Fdovecot%2Fcore.git

lib: data-stack - Move frame memory allocation inside data stack block
---

diff --git a/src/lib/data-stack.c b/src/lib/data-stack.c
index 1839d04236..d7b9de0702 100644
--- a/src/lib/data-stack.c
+++ b/src/lib/data-stack.c
@@ -149,20 +149,18 @@ data_stack_frame_t t_push(const char *marker)
 	}
 
 	/* allocate new block */
-	frame = calloc(sizeof(*frame), 1);
-	if (frame == NULL)
-		i_fatal_status(FATAL_OUTOFMEM, "t_push(): Out of memory");
+	frame = t_buffer_get(sizeof(*frame));
 	frame->prev = current_frame;
 	current_frame = frame;
 
-	data_stack_last_buffer_reset(FALSE);
-
 	/* mark our current position */
 	current_frame->block = current_block;
 	current_frame->block_space_left = current_block->left;
 	current_frame->last_alloc_size = 0;
 	current_frame->marker = marker;
 
+	t_buffer_alloc(sizeof(*frame));
+
 #ifndef STATIC_CHECKER
 	return data_stack_frame_id++;
 #else
@@ -257,7 +255,7 @@ static void t_pop_verify(void)
 
 void t_pop_last_unsafe(void)
 {
-	struct stack_frame *frame;
+	size_t block_space_left;
 
 	if (unlikely(current_frame == NULL))
 		i_panic("t_pop() called with empty stack");
@@ -270,17 +268,22 @@ void t_pop_last_unsafe(void)
 	/* update the current block */
 	current_block = current_frame->block;
 	block_canary_check(current_block);
+
+	/* current_frame points inside the stack frame that will be freed.
+	   make sure it's not accessed after it's already freed/cleaned. */
+	block_space_left = current_frame->block_space_left;
+	current_frame = current_frame->prev;
+
 	if (clean_after_pop) {
 		size_t start_pos, end_pos;
 
-		start_pos = current_block->size -
-			current_frame->block_space_left;
+		start_pos = current_block->size - block_space_left;
 		end_pos = current_block->size - current_block->left_lowwater;
 		i_assert(end_pos >= start_pos);
 		memset(STACK_BLOCK_DATA(current_block) + start_pos, CLEAR_CHR,
 		       end_pos - start_pos);
 	}
-	current_block->left = current_frame->block_space_left;
+	current_block->left = block_space_left;
 	current_block->left_lowwater = current_block->left;
 
 	if (current_block->next != NULL) {
@@ -289,10 +292,6 @@ void t_pop_last_unsafe(void)
 		current_block->next = NULL;
 	}
 
-	frame = current_frame;
-	current_frame = frame->prev;
-	free(frame);
-
 	data_stack_frame_id--;
 }
 
@@ -435,8 +434,10 @@ static void *t_malloc_real(size_t size, bool permanent)
 #endif
 	data_stack_last_buffer_reset(TRUE);
 
-	/* used for t_try_realloc() */
-	current_frame->last_alloc_size = alloc_size;
+	if (permanent) {
+		/* used for t_try_realloc() */
+		current_frame->last_alloc_size = alloc_size;
+	}
 
 	if (current_block->left < alloc_size) {
 		struct stack_block *block;
diff --git a/src/lib/test-data-stack.c b/src/lib/test-data-stack.c
index f437c2ddf5..fa703b0d86 100644
--- a/src/lib/test-data-stack.c
+++ b/src/lib/test-data-stack.c
@@ -134,7 +134,10 @@ static void test_ds_buffers(void)
 	T_BEGIN {
 		size_t bigleft = t_get_bytes_available();
 		size_t i;
-		for (i = 1; i < bigleft-64; i += i_rand_limit(32)) T_BEGIN {
+		/* with DEBUG: the stack frame allocation takes 96 bytes
+		   and malloc takes extra 40 bytes + alignment, so don't let
+		   "i" be too high. */
+		for (i = 1; i < bigleft-96-40-16; i += i_rand_limit(32)) T_BEGIN {
 			unsigned char *p, *p2;
 			size_t left;
 			t_malloc_no0(i);