From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Tue, 27 Dec 2022 09:58:14 +0000 (-0500)
Subject: global: Avoid using data stack as memory pool for potentially large allocations
X-Git-Tag: 2.4.0~3200
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7027ff88e395edb6efc2a4a726f5a9ed280ccd58;p=thirdparty%2Fdovecot%2Fcore.git

global: Avoid using data stack as memory pool for potentially large allocations

The previous usage wasn't really bad, since the memory was freed soon
enough. However, these make it more difficult to find when data stack
really is growing excessively.
---

diff --git a/src/doveadm/doveadm-mail-fetch.c b/src/doveadm/doveadm-mail-fetch.c
index f051097488..f7ed0160a4 100644
--- a/src/doveadm/doveadm-mail-fetch.c
+++ b/src/doveadm/doveadm-mail-fetch.c
@@ -27,6 +27,7 @@ struct fetch_cmd_context {
 	struct doveadm_mail_cmd_context ctx;
 
 	struct mail *mail;
+	pool_t temp_pool;
 
 	ARRAY(struct fetch_field) fields;
 	ARRAY_TYPE(const_string) header_fields;
@@ -136,9 +137,10 @@ static int fetch_hdr(struct fetch_cmd_context *ctx)
 static int fetch_hdr_field(struct fetch_cmd_context *ctx)
 {
 	const char *const *value, *filter, *name = ctx->cur_field->name;
-	string_t *str = t_str_new(256);
+	string_t *str;
 	bool add_lf = FALSE;
 
+	p_clear(ctx->temp_pool);
 	filter = strchr(name, '.');
 	if (filter != NULL)
 		name = t_strdup_until(name, filter++);
@@ -151,6 +153,7 @@ static int fetch_hdr_field(struct fetch_cmd_context *ctx)
 			return -1;
 	}
 
+	str = str_new(ctx->temp_pool, 256);
 	for (; *value != NULL; value++) {
 		if (add_lf)
 			str_append_c(str, '\n');
@@ -165,7 +168,7 @@ static int fetch_hdr_field(struct fetch_cmd_context *ctx)
 		   strcmp(filter, "address_name.utf8") == 0) {
 		struct message_address *addr;
 
-		addr = message_address_parse(pool_datastack_create(),
+		addr = message_address_parse(ctx->temp_pool,
 					     str_data(str), str_len(str),
 					     UINT_MAX, 0);
 		str_truncate(str, 0);
@@ -667,16 +670,26 @@ static void cmd_fetch_init(struct doveadm_mail_cmd_context *_ctx)
 	if (!doveadm_cmd_param_array(cctx, "query", &query))
 		doveadm_mail_help_name("fetch");
 
+	ctx->temp_pool = pool_alloconly_create("doveadm fetch", 1024);
 	parse_fetch_fields(ctx, fields);
 	_ctx->search_args = doveadm_mail_build_search_args(query);
 }
 
+static void cmd_fetch_deinit(struct doveadm_mail_cmd_context *_ctx)
+{
+	struct fetch_cmd_context *ctx =
+		container_of(_ctx, struct fetch_cmd_context, ctx);
+
+	pool_unref(&ctx->temp_pool);
+}
+
 static struct doveadm_mail_cmd_context *cmd_fetch_alloc(void)
 {
 	struct fetch_cmd_context *ctx;
 
 	ctx = doveadm_mail_cmd_alloc(struct fetch_cmd_context);
 	ctx->ctx.v.init = cmd_fetch_init;
+	ctx->ctx.v.deinit = cmd_fetch_deinit;
 	ctx->ctx.v.run = cmd_fetch_run;
 	doveadm_print_init(DOVEADM_PRINT_TYPE_PAGER);
 	return &ctx->ctx;
diff --git a/src/imap/cmd-copy.c b/src/imap/cmd-copy.c
index e11d3adfdc..4a03af0fdb 100644
--- a/src/imap/cmd-copy.c
+++ b/src/imap/cmd-copy.c
@@ -126,7 +126,7 @@ static int fetch_and_copy(struct cmd_copy_context *copy_ctx,
 					 NULL, 0, NULL);
 	mail_search_args_unref(&search_args);
 
-	t_array_init(&src_uids, 64);
+	i_array_init(&src_uids, 64);
 	ret = 1;
 	while (mailbox_search_next(search_ctx, &mail) && ret > 0) {
 		if (mail->expunged) {
@@ -218,6 +218,7 @@ static int fetch_and_copy(struct cmd_copy_context *copy_ctx,
 		copy_update_trashed(client, copy_ctx->destbox, copy_ctx->copy_count);
 		pool_unref(&changes.pool);
 	}
+	array_free(&src_uids);
 
 	if (!copy_ctx->move ||
 	    copy_ctx->srcbox == copy_ctx->destbox) {
diff --git a/src/lib-mail/message-address.c b/src/lib-mail/message-address.c
index fb06afae7b..18ca24c904 100644
--- a/src/lib-mail/message-address.c
+++ b/src/lib-mail/message-address.c
@@ -454,7 +454,7 @@ message_address_parse_real(pool_t pool, const unsigned char *data, size_t size,
 	rfc822_parser_init(&ctx.parser, data, size, t_str_new(128));
 	ctx.parser.nul_replacement_str = RFC822_NUL_REPLACEMENT_STR;
 	ctx.pool = pool;
-	ctx.str = t_str_new(128);
+	ctx.str = str_new(default_pool, 128);
 	ctx.fill_missing = (flags & MESSAGE_ADDRESS_PARSE_FLAG_FILL_MISSING) != 0;
 	ctx.non_strict_dots = (flags & MESSAGE_ADDRESS_PARSE_FLAG_STRICT_DOTS) == 0;
 
@@ -464,6 +464,7 @@ message_address_parse_real(pool_t pool, const unsigned char *data, size_t size,
 		(void)parse_address_list(&ctx, max_addresses);
 	}
 	rfc822_parser_deinit(&ctx.parser);
+	str_free(&ctx.str);
 	return ctx.first_addr;
 }
 
@@ -479,11 +480,12 @@ message_address_parse_path_real(pool_t pool, const unsigned char *data,
 
 	rfc822_parser_init(&ctx.parser, data, size, NULL);
 	ctx.pool = pool;
-	ctx.str = t_str_new(128);
+	ctx.str = str_new(default_pool, 128);
 
 	ret = parse_path(&ctx);
 
 	rfc822_parser_deinit(&ctx.parser);
+	str_free(&ctx.str);
 	*addr_r = ctx.first_addr;
 	return (ret < 0 ? -1 : 0);
 }
diff --git a/src/lib-mail/message-search.c b/src/lib-mail/message-search.c
index e4fc9637fb..c1809c3477 100644
--- a/src/lib-mail/message-search.c
+++ b/src/lib-mail/message-search.c
@@ -191,10 +191,9 @@ void message_search_reset(struct message_search_context *ctx)
 	message_decoder_decode_reset(ctx->decoder);
 }
 
-static int
-message_search_msg_real(struct message_search_context *ctx,
-			struct istream *input, struct message_part *parts,
-			const char **error_r)
+int message_search_msg(struct message_search_context *ctx,
+		       struct istream *input, struct message_part *parts,
+		       const char **error_r)
 {
 	const struct message_parser_settings parser_set = {
 		.hdr_flags = MESSAGE_HEADER_PARSER_FLAG_CLEAN_ONELINE,
@@ -202,6 +201,7 @@ message_search_msg_real(struct message_search_context *ctx,
 	struct message_parser_ctx *parser_ctx;
 	struct message_block raw_block;
 	struct message_part *new_parts;
+	pool_t pool = NULL;
 	int ret;
 
 	message_search_reset(ctx);
@@ -210,8 +210,8 @@ message_search_msg_real(struct message_search_context *ctx,
 		parser_ctx = message_parser_init_from_parts(parts,
 						input, &parser_set);
 	} else {
-		parser_ctx = message_parser_init(pool_datastack_create(),
-						 input, &parser_set);
+		pool = pool_alloconly_create("message search parts", 1024);
+		parser_ctx = message_parser_init(pool, input, &parser_set);
 	}
 
 	while ((ret = message_parser_parse_next_block(parser_ctx,
@@ -230,17 +230,6 @@ message_search_msg_real(struct message_search_context *ctx,
 		/* broken parts */
 		ret = -1;
 	}
-	return ret;
-}
-
-int message_search_msg(struct message_search_context *ctx,
-		       struct istream *input, struct message_part *parts,
-		       const char **error_r)
-{
-	int ret;
-
-	T_BEGIN {
-		ret = message_search_msg_real(ctx, input, parts, error_r);
-	} T_END_PASS_STR_IF(ret < 0, error_r);
+	pool_unref(&pool);
 	return ret;
 }
diff --git a/src/lib-storage/index/index-mail-headers.c b/src/lib-storage/index/index-mail-headers.c
index 1423c54147..18c922239d 100644
--- a/src/lib-storage/index/index-mail-headers.c
+++ b/src/lib-storage/index/index-mail-headers.c
@@ -56,7 +56,8 @@ static void index_mail_parse_header_finish(struct index_mail *mail)
 	lines = array_get(&mail->header_lines, &count);
 	match = array_get(&mail->header_match, &match_count);
 	header = mail->header_data->data;
-	buf = t_buffer_create(256);
+	pool_t pool = pool_alloconly_create("index mail header", 512);
+	buf = buffer_create_dynamic(pool, 256);
 
 	/* go through all the header lines we found */
 	for (i = match_idx = 0; i < count; i = j) {
@@ -142,6 +143,7 @@ static void index_mail_parse_header_finish(struct index_mail *mail)
 
 	mail->data.dont_cache_field_idx = UINT_MAX;
 	index_mail_parse_header_deinit(mail);
+	pool_unref(&pool);
 }
 
 static unsigned int
@@ -799,13 +801,15 @@ index_mail_headers_decode(struct index_mail *mail, const char *const **_list,
 		count = max_count;
 	decoded_list = p_new(mail->mail.data_pool, const char *, count + 1);
 
-	str = t_str_new(512);
+	str = str_new(default_pool, 512);
 	for (i = 0; i < count; i++) {
 		str_truncate(str, 0);
 		input = list[i];
 		/* unfold all lines into a single line */
-		if (unfold_header(mail->mail.data_pool, &input) < 0)
+		if (unfold_header(mail->mail.data_pool, &input) < 0) {
+			str_free(&str);
 			return -1;
+		}
 
 		/* decode MIME encoded-words. decoding may also add new LFs. */
 		message_header_decode_utf8((const unsigned char *)input,
@@ -819,6 +823,7 @@ index_mail_headers_decode(struct index_mail *mail, const char *const **_list,
 		}
 		decoded_list[i] = input;
 	}
+	str_free(&str);
 	*_list = decoded_list;
 	return 0;
 }
diff --git a/src/lib-storage/index/index-mail.c b/src/lib-storage/index/index-mail.c
index 5a2d45398e..e653c08758 100644
--- a/src/lib-storage/index/index-mail.c
+++ b/src/lib-storage/index/index-mail.c
@@ -817,12 +817,12 @@ static void index_mail_body_parsed_cache_message_parts(struct index_mail *mail)
 		return;
 	}
 
-	T_BEGIN {
-		buffer = t_buffer_create(1024);
-		message_part_serialize(mail->data.parts, buffer);
-		index_mail_cache_add_idx(mail, cache_field,
-					 buffer->data, buffer->used);
-	} T_END;
+	pool_t pool = pool_alloconly_create("mail parts", 2048);
+	buffer = buffer_create_dynamic(pool, 1024);
+	message_part_serialize(mail->data.parts, buffer);
+	index_mail_cache_add_idx(mail, cache_field,
+				 buffer->data, buffer->used);
+	pool_unref(&pool);
 
 	data->messageparts_saved_to_cache = TRUE;
 }
diff --git a/src/lib-storage/index/index-search-private.h b/src/lib-storage/index/index-search-private.h
index d98cea4849..ad01a41a1c 100644
--- a/src/lib-storage/index/index-search-private.h
+++ b/src/lib-storage/index/index-search-private.h
@@ -22,6 +22,7 @@ struct index_search_context {
 	struct mail *cur_mail;
 	struct index_mail *cur_imail;
 	struct mail_thread_context *thread_ctx;
+	pool_t temp_pool;
 
 	struct timeval search_start_time, last_notify;
 	struct timeval last_nonblock_timeval;
diff --git a/src/lib-storage/index/index-search.c b/src/lib-storage/index/index-search.c
index 899df4c278..786d8eae1e 100644
--- a/src/lib-storage/index/index-search.c
+++ b/src/lib-storage/index/index-search.c
@@ -528,6 +528,23 @@ static void compress_lwsp(string_t *dest, const unsigned char *src,
 	}
 }
 
+static pool_t
+search_context_temp_pool(struct index_search_context *ctx, size_t size)
+{
+	if (ctx->temp_pool != NULL) {
+		p_clear(ctx->temp_pool);
+		return ctx->temp_pool;
+	}
+
+	if (size <= 1024)
+		return pool_datastack_create();
+	/* This could just as well be allocated from data stack, but it
+	   makes it more difficult to track bad data_stack_grow events.
+	   Use a temporary memory pool instead. */
+	ctx->temp_pool = pool_alloconly_create("search context temp", size);
+	return ctx->temp_pool;
+}
+
 static void search_header_arg(struct mail_search_arg *arg,
 			      struct search_header_context *ctx)
 {
@@ -601,25 +618,31 @@ static void search_header_arg(struct mail_search_arg *arg,
 		case SEARCH_HEADER:
 			/* simple match */
 			break;
-		case SEARCH_HEADER_ADDRESS:
+		case SEARCH_HEADER_ADDRESS: {
 			/* we have to match against normalized address */
-			addr = message_address_parse(pool_datastack_create(),
+			pool_t pool = search_context_temp_pool(ctx->index_ctx,
+						ctx->hdr->full_value_len);
+			addr = message_address_parse(pool,
 						     ctx->hdr->full_value,
 						     ctx->hdr->full_value_len,
 						     UINT_MAX,
 						     MESSAGE_ADDRESS_PARSE_FLAG_FILL_MISSING);
-			str = t_str_new(ctx->hdr->value_len);
+			str = str_new(pool, ctx->hdr->value_len);
 			message_address_write(str, addr);
 			hdr.value = hdr.full_value = str_data(str);
 			hdr.value_len = hdr.full_value_len = str_len(str);
 			break;
-		case SEARCH_HEADER_COMPRESS_LWSP:
+		}
+		case SEARCH_HEADER_COMPRESS_LWSP: {
 			/* convert LWSP to single spaces */
-			str = t_str_new(hdr.full_value_len);
+			pool_t pool = search_context_temp_pool(ctx->index_ctx,
+						hdr.full_value_len);
+			str = str_new(pool, hdr.full_value_len);
 			compress_lwsp(str, hdr.full_value, hdr.full_value_len);
 			hdr.value = hdr.full_value = str_data(str);
 			hdr.value_len = hdr.full_value_len = str_len(str);
 			break;
+		}
 		default:
 			i_unreached();
 		}
@@ -1392,6 +1415,7 @@ int index_storage_search_deinit(struct mail_search_context *_ctx)
 	if (ctx->failed)
 		mail_storage_last_error_pop(ctx->box->storage);
 	array_free(&ctx->mail_ctx.mails);
+	pool_unref(&ctx->temp_pool);
 	i_free(ctx);
 	return ret;
 }
diff --git a/src/lib-storage/index/index-storage.c b/src/lib-storage/index/index-storage.c
index ac96d89c7d..0182aac239 100644
--- a/src/lib-storage/index/index-storage.c
+++ b/src/lib-storage/index/index-storage.c
@@ -1083,12 +1083,13 @@ void index_copy_cache_fields(struct mail_save_context *ctx,
 					 &dest_metadata) < 0)
 			i_unreached();
 
-		buf = t_buffer_create(1024);
+		buf = buffer_create_dynamic(default_pool, 1024);
 		array_foreach(src_metadata.cache_fields, field) {
 			mail_copy_cache_field(ctx, src_mail, dest_seq,
 					      field->name, buf);
 		}
 		index_copy_vsize_extension(ctx, src_mail, dest_seq);
+		buffer_free(&buf);
 	} T_END;
 }
 
diff --git a/src/lib-storage/index/maildir/maildir-save.c b/src/lib-storage/index/maildir/maildir-save.c
index 586be5b2df..4e302bba32 100644
--- a/src/lib-storage/index/maildir/maildir-save.c
+++ b/src/lib-storage/index/maildir/maildir-save.c
@@ -873,7 +873,7 @@ maildir_save_move_files_to_newcur(struct maildir_save_context *ctx)
 	/* put files into an array sorted by the destination filename.
 	   this way we can easily check if there are duplicate destination
 	   filenames within this transaction. */
-	t_array_init(&files, ctx->files_count);
+	p_array_init(&files, ctx->pool, ctx->files_count);
 	for (mf = ctx->files; mf != NULL; mf = mf->next)
 		array_push_back(&files, &mf);
 	array_sort(&files, maildir_filename_dest_basename_cmp);
diff --git a/src/plugins/fts/fts-build-mail.c b/src/plugins/fts/fts-build-mail.c
index d9ca6ccbd8..6e048fc642 100644
--- a/src/plugins/fts/fts-build-mail.c
+++ b/src/plugins/fts/fts-build-mail.c
@@ -602,7 +602,8 @@ fts_build_mail_real(struct fts_backend_update_context *update_ctx,
 		ctx.pending_input = buffer_create_dynamic(default_pool, 128);
 
 	prev_part = NULL;
-	parser = message_parser_init(pool_datastack_create(), input, &parser_set);
+	pool_t parts_pool = pool_alloconly_create("fts message parts", 512);
+	parser = message_parser_init(parts_pool, input, &parser_set);
 
 	decoder = message_decoder_init(update_ctx->normalizer, 0);
 	for (;;) {
@@ -699,6 +700,7 @@ fts_build_mail_real(struct fts_backend_update_context *update_ctx,
 	i_free(ctx.content_disposition);
 	buffer_free(&ctx.word_buf);
 	buffer_free(&ctx.pending_input);
+	pool_unref(&parts_pool);
 	return ret < 0 ? -1 : 1;
 }