Skip to content

Commit

Permalink
lib-storage: Use refcounting for mail_storage_service_user
Browse files Browse the repository at this point in the history
doveadm import was freeing the user too early, which resulted
mail_user._service_user pointing to freed memory. More importantly,
after 34512ea user->set was pointing
to already freed memory.
  • Loading branch information
sirainen committed Feb 22, 2017
1 parent 7d58e42 commit 036e524
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
18 changes: 16 additions & 2 deletions src/lib-storage/mail-storage-service.c
Expand Up @@ -70,6 +70,8 @@ struct mail_storage_service_ctx {

struct mail_storage_service_user {
pool_t pool;
int refcount;

struct mail_storage_service_ctx *service_ctx;
struct mail_storage_service_input input;
enum mail_storage_service_flags flags;
Expand Down Expand Up @@ -650,6 +652,7 @@ mail_storage_service_init_post(struct mail_storage_service_ctx *ctx,
mail_user = mail_user_alloc_nodup_set(user->input.username,
user->user_info, user->user_set);
mail_user->_service_user = user;
mail_storage_service_user_ref(user);
mail_user_set_home(mail_user, *home == '\0' ? NULL : home);
mail_user_set_vars(mail_user, ctx->service->name,
&user->input.local_ip, &user->input.remote_ip);
Expand Down Expand Up @@ -1229,6 +1232,7 @@ mail_storage_service_lookup_real(struct mail_storage_service_ctx *ctx,
}

user = p_new(user_pool, struct mail_storage_service_user, 1);
user->refcount = 1;
user->service_ctx = ctx;
user->pool = user_pool;
user->input = *input;
Expand Down Expand Up @@ -1476,7 +1480,7 @@ int mail_storage_service_lookup_next(struct mail_storage_service_ctx *ctx,

ret = mail_storage_service_next(ctx, user, mail_user_r);
if (ret < 0) {
mail_storage_service_user_free(&user);
mail_storage_service_user_unref(&user);
*error_r = ret == -2 ? ERRSTR_INVALID_USER_SETTINGS :
MAIL_ERRSTR_CRITICAL_MSG;
return ret;
Expand All @@ -1485,12 +1489,22 @@ int mail_storage_service_lookup_next(struct mail_storage_service_ctx *ctx,
return 1;
}

void mail_storage_service_user_free(struct mail_storage_service_user **_user)
void mail_storage_service_user_ref(struct mail_storage_service_user *user)
{
i_assert(user->refcount > 0);
user->refcount++;
}

void mail_storage_service_user_unref(struct mail_storage_service_user **_user)
{
struct mail_storage_service_user *user = *_user;

*_user = NULL;

i_assert(user->refcount > 0);
if (--user->refcount > 0)
return;

if (user->ioloop_ctx != NULL) {
if ((user->flags & MAIL_STORAGE_SERVICE_FLAG_NO_LOG_INIT) == 0) {
io_loop_context_remove_callbacks(user->ioloop_ctx,
Expand Down
6 changes: 5 additions & 1 deletion src/lib-storage/mail-storage-service.h
Expand Up @@ -112,7 +112,11 @@ int mail_storage_service_lookup_next(struct mail_storage_service_ctx *ctx,
struct mail_storage_service_user **user_r,
struct mail_user **mail_user_r,
const char **error_r);
void mail_storage_service_user_free(struct mail_storage_service_user **user);
void mail_storage_service_user_ref(struct mail_storage_service_user *user);
void mail_storage_service_user_unref(struct mail_storage_service_user **user);
/* FIXME: for backwards compatibility - remove */
#define mail_storage_service_user_free(user) \
mail_storage_service_user_unref(user)
/* Initialize iterating through all users. */
void mail_storage_service_all_init(struct mail_storage_service_ctx *ctx);
/* Same as mail_storage_service_all_init(), but give a user mask hint to the
Expand Down
7 changes: 6 additions & 1 deletion src/lib-storage/mail-user.c
Expand Up @@ -36,6 +36,8 @@ static void mail_user_deinit_base(struct mail_user *user)
dict_deinit(&user->_attr_dict);
}
mail_namespaces_deinit(&user->namespaces);
if (user->_service_user != NULL)
mail_storage_service_user_unref(&user->_service_user);
}

static void mail_user_stats_fill_base(struct mail_user *user ATTR_UNUSED,
Expand Down Expand Up @@ -558,7 +560,10 @@ struct mail_user *mail_user_dup(struct mail_user *user)

user2 = mail_user_alloc(user->username, user->set_info,
user->unexpanded_set);
user2->_service_user = user->_service_user;
if (user2->_service_user != NULL) {
user2->_service_user = user->_service_user;
mail_storage_service_user_ref(user2->_service_user);
}
if (user->_home != NULL)
mail_user_set_home(user2, user->_home);
mail_user_set_vars(user2, user->service,
Expand Down

0 comments on commit 036e524

Please sign in to comment.