]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
dict-client: Fix potential iterator double-free
authorTimo Sirainen <timo.sirainen@open-xchange.com>
Tue, 26 May 2020 16:09:38 +0000 (19:09 +0300)
committeraki.tuomi <aki.tuomi@open-xchange.com>
Wed, 24 Jun 2020 17:04:44 +0000 (17:04 +0000)
client_dict_iterate_free() didn't really work properly, because of:

ctx->finished = TRUE;
client_dict_iter_api_callback(ctx, cmd, extra_args);
client_dict_iterate_free(ctx);

Here finished=TRUE is set first (and it needs to be set first). Afterwards
client_dict_iter_api_callback() internally calls
client_dict_iterate_deinit(), which can end up freeing the iterator.

src/lib-dict/dict-client.c

index 9b158c8cfc6126229e74d12af58fe62f0c0616e1..67505fcf8274536a7a0e2546ab7983e3c2c9bcdd 100644 (file)
@@ -100,6 +100,7 @@ struct client_dict_iterate_context {
        char *error;
        const char **paths;
        enum dict_iterate_flags flags;
+       int refcount;
 
        pool_t results_pool;
        ARRAY(struct client_dict_iter_result) results;
@@ -1023,9 +1024,10 @@ static int client_dict_lookup(struct dict *_dict, pool_t pool, const char *key,
        i_unreached();
 }
 
-static void client_dict_iterate_free(struct client_dict_iterate_context *ctx)
+static void client_dict_iterate_unref(struct client_dict_iterate_context *ctx)
 {
-       if (!ctx->deinit || !ctx->finished)
+       i_assert(ctx->refcount > 0);
+       if (--ctx->refcount > 0)
                return;
        i_free(ctx->error);
        i_free(ctx);
@@ -1092,9 +1094,10 @@ client_dict_iter_async_callback(struct client_dict_cmd *cmd,
        } else switch (reply) {
        case DICT_PROTOCOL_REPLY_ITER_FINISHED:
                /* end of iteration */
+               i_assert(!ctx->finished);
                ctx->finished = TRUE;
                client_dict_iter_api_callback(ctx, cmd, extra_args);
-               client_dict_iterate_free(ctx);
+               client_dict_iterate_unref(ctx);
                return;
        case DICT_PROTOCOL_REPLY_OK:
                /* key \t value */
@@ -1118,9 +1121,10 @@ client_dict_iter_async_callback(struct client_dict_cmd *cmd,
        if (error != NULL) {
                if (ctx->error == NULL)
                        ctx->error = i_strdup(error);
+               i_assert(!ctx->finished);
                ctx->finished = TRUE;
                client_dict_iter_api_callback(ctx, cmd, extra_args);
-               client_dict_iterate_free(ctx);
+               client_dict_iterate_unref(ctx);
                return;
        }
        cmd->unfinished = TRUE;
@@ -1148,6 +1152,7 @@ client_dict_iterate_init(struct dict *_dict, const char *const *paths,
        ctx->results_pool = pool_alloconly_create("client dict iteration", 512);
        ctx->flags = flags;
        ctx->paths = p_strarray_dup(system_pool, paths);
+       ctx->refcount = 1;
        i_array_init(&ctx->results, 64);
        return &ctx->ctx;
 }
@@ -1174,6 +1179,7 @@ client_dict_iterate_cmd_send(struct client_dict_iterate_context *ctx)
        cmd->callback = client_dict_iter_async_callback;
        cmd->retry_errors = TRUE;
 
+       ctx->refcount++;
        client_dict_cmd_send(dict, &cmd, NULL);
 }
 
@@ -1224,13 +1230,14 @@ static int client_dict_iterate_deinit(struct dict_iterate_context *_ctx,
                (struct client_dict_iterate_context *)_ctx;
        int ret = ctx->error != NULL ? -1 : 0;
 
+       i_assert(!ctx->deinit);
        ctx->deinit = TRUE;
 
        *error_r = t_strdup(ctx->error);
        array_free(&ctx->results);
        pool_unref(&ctx->results_pool);
        i_free(ctx->paths);
-       client_dict_iterate_free(ctx);
+       client_dict_iterate_unref(ctx);
 
        client_dict_add_timeout(dict);
        return ret;