From fccd110b494a7e31f23d31d9e3bc3e986c9bb1a8 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Thu, 4 May 2017 17:31:47 +0300 Subject: [PATCH] lib-storage: Fix memory leak in mail_search_args_simplify() The leaks happened when search args were already initialized (which they usually were at this point) and some of the args were dropped entirely. Improved the unit test to initialize search args before calling the simplify, so valgrind would notice if there are any leaks. --- src/lib-storage/mail-search-args-simplify.c | 40 ++++++++++++------- .../test-mail-search-args-simplify.c | 9 +++++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/lib-storage/mail-search-args-simplify.c b/src/lib-storage/mail-search-args-simplify.c index 7c7a24e08c..154383d749 100644 --- a/src/lib-storage/mail-search-args-simplify.c +++ b/src/lib-storage/mail-search-args-simplify.c @@ -27,6 +27,7 @@ struct mail_search_simplify_ctx { struct mail_search_simplify_prev_arg *) prev_args; bool parent_and; bool removals; + bool initialized; }; static int @@ -96,6 +97,9 @@ mail_search_args_merge_mask(struct mail_search_simplify_ctx *ctx, *prev_argp = args; return FALSE; } + if (ctx->initialized) + mail_search_arg_deinit(*prev_argp); + if ((*prev_argp)->match_not != args->match_not) { /* a && !a = 0 */ (*prev_argp)->type = SEARCH_ALL; @@ -326,7 +330,8 @@ mail_search_args_have_equal(const struct mail_search_arg *args, } static bool -mail_search_args_remove_equal(struct mail_search_arg **argsp, +mail_search_args_remove_equal(struct mail_search_args *all_args, + struct mail_search_arg **argsp, const struct mail_search_arg *wanted_arg, bool check_subs) { @@ -335,12 +340,14 @@ mail_search_args_remove_equal(struct mail_search_arg **argsp, for (argp = argsp; (*argp) != NULL; ) { if (mail_search_arg_one_equals(*argp, wanted_arg)) { + if (all_args->init_refcount > 0) + mail_search_arg_deinit(*argp); *argp = (*argp)->next; found = TRUE; } else if (check_subs) { i_assert((*argp)->type == SEARCH_SUB || (*argp)->type == SEARCH_OR); - if (!mail_search_args_remove_equal(&(*argp)->value.subargs, wanted_arg, FALSE)) { + if (!mail_search_args_remove_equal(all_args, &(*argp)->value.subargs, wanted_arg, FALSE)) { /* we already verified that this should have existed. */ i_unreached(); @@ -384,7 +391,8 @@ mail_search_args_count(const struct mail_search_arg *args) } static bool -mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp, +mail_search_args_simplify_drop_redundant_args(struct mail_search_args *all_args, + struct mail_search_arg **argsp, bool and_arg) { struct mail_search_arg *arg, **argp, one_arg, *lowest_arg = NULL; @@ -419,6 +427,8 @@ mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp, if (*argp != lowest_arg && (*argp)->type == child_subargs_type && (*argp)->value.subargs != lowest_arg && mail_search_args_have_all_equal(*argp, lowest_arg)) { + if (all_args->init_refcount > 0) + mail_search_arg_deinit(*argp); *argp = (*argp)->next; ret = TRUE; } else { @@ -429,7 +439,8 @@ mail_search_args_simplify_drop_redundant_args(struct mail_search_arg **argsp, } static bool -mail_search_args_simplify_extract_common(struct mail_search_arg **argsp, +mail_search_args_simplify_extract_common(struct mail_search_args *all_args, + struct mail_search_arg **argsp, pool_t pool, bool and_arg) { /* Simple SUB example: @@ -483,7 +494,7 @@ mail_search_args_simplify_extract_common(struct mail_search_arg **argsp, continue; /* extract the arg and put it to common_args */ - mail_search_args_remove_equal(argsp, sub_arg, TRUE); + mail_search_args_remove_equal(all_args, argsp, sub_arg, TRUE); sub_arg->next = common_args; common_args = sub_arg; } @@ -511,7 +522,7 @@ mail_search_args_simplify_extract_common(struct mail_search_arg **argsp, } static bool -mail_search_args_simplify_sub(struct mailbox *box, pool_t pool, +mail_search_args_simplify_sub(struct mail_search_args *all_args, pool_t pool, struct mail_search_arg **argsp, bool parent_and) { struct mail_search_simplify_ctx ctx; @@ -519,6 +530,7 @@ mail_search_args_simplify_sub(struct mailbox *box, pool_t pool, bool merged; i_zero(&ctx); + ctx.initialized = all_args->init_refcount > 0; ctx.parent_and = parent_and; ctx.pool = pool_alloconly_create("mail search args simplify", 1024); hash_table_create(&ctx.prev_args, ctx.pool, 0, @@ -565,12 +577,12 @@ mail_search_args_simplify_sub(struct mailbox *box, pool_t pool, if (args->type != SEARCH_INTHREAD) { bool and_arg = args->type == SEARCH_SUB; - if (mail_search_args_simplify_drop_redundant_args(&args->value.subargs, and_arg)) + if (mail_search_args_simplify_drop_redundant_args(all_args, &args->value.subargs, and_arg)) ctx.removals = TRUE; - if (mail_search_args_simplify_extract_common(&args->value.subargs, pool, and_arg)) + if (mail_search_args_simplify_extract_common(all_args, &args->value.subargs, pool, and_arg)) ctx.removals = TRUE; } - if (mail_search_args_simplify_sub(box, pool, &args->value.subargs, + if (mail_search_args_simplify_sub(all_args, pool, &args->value.subargs, args->type != SEARCH_OR)) ctx.removals = TRUE; } @@ -759,20 +771,20 @@ void mail_search_args_simplify(struct mail_search_args *args) args->simplified = TRUE; - removals = mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE); + removals = mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE); if (mail_search_args_unnest_inthreads(args, &args->args, FALSE, TRUE)) { /* we may have added some extra SUBs that could be dropped */ - if (mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE)) + if (mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE)) removals = TRUE; } do { - if (mail_search_args_simplify_drop_redundant_args(&args->args, TRUE)) + if (mail_search_args_simplify_drop_redundant_args(args, &args->args, TRUE)) removals = TRUE; - if (mail_search_args_simplify_extract_common(&args->args, args->pool, TRUE)) + if (mail_search_args_simplify_extract_common(args, &args->args, args->pool, TRUE)) removals = TRUE; if (removals) - removals = mail_search_args_simplify_sub(args->box, args->pool, &args->args, TRUE); + removals = mail_search_args_simplify_sub(args, args->pool, &args->args, TRUE); /* do the flag merging into a single arg only at the end. up until then they're treated as any other search args, which simplifies their handling. after the flags merging is diff --git a/src/lib-storage/test-mail-search-args-simplify.c b/src/lib-storage/test-mail-search-args-simplify.c index b83697deac..4862be8316 100644 --- a/src/lib-storage/test-mail-search-args-simplify.c +++ b/src/lib-storage/test-mail-search-args-simplify.c @@ -3,6 +3,7 @@ #include "lib.h" #include "str.h" #include "test-common.h" +#include "mail-storage-private.h" #include "mail-search-build.h" #include "mail-search-parser.h" #include "mail-search.h" @@ -237,13 +238,18 @@ test_build_search_args(const char *args) static void test_mail_search_args_simplify(void) { struct mail_search_args *args; + struct mail_storage_settings set = { .mail_max_keyword_length = 100 }; + struct mail_storage storage = { .set = &set }; + struct mailbox box = { .opened = TRUE, .storage = &storage }; string_t *str = t_str_new(256); const char *error; unsigned int i; test_begin("mail search args simplify"); + box.index = mail_index_alloc(NULL, "dovecot.index."); for (i = 0; i < N_ELEMENTS(tests); i++) { args = test_build_search_args(tests[i].input); + mail_search_args_init(args, &box, FALSE, NULL); mail_search_args_simplify(args); str_truncate(str, 0); @@ -251,6 +257,7 @@ static void test_mail_search_args_simplify(void) test_assert_idx(strcmp(str_c(str), tests[i].output) == 0, i); mail_search_args_unref(&args); } + mail_index_free(&box.index); test_end(); } @@ -270,8 +277,10 @@ static void test_mail_search_args_simplify_empty_lists(void) int main(void) { static void (*const test_functions[])(void) = { + mail_storage_init, test_mail_search_args_simplify, test_mail_search_args_simplify_empty_lists, + mail_storage_deinit, NULL }; -- 2.47.3