Mercurial > dovecot > core-2.2

changeset 21670:4ce2e5f4dafc

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-storage: Use refcounting for mail_storage_service_user doveadm import was freeing the user too early, which resulted mail_user._service_user pointing to freed memory. More importantly, after 34512eaad8b1b2f929e6d6e3a2f7252c29fba97b user->set was pointing to already freed memory.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Wed, 22 Feb 2017 13:28:43 +0200
parents 39b914bf2894
children b4dd0868ecc0
files src/lib-storage/mail-storage-service.c src/lib-storage/mail-storage-service.h src/lib-storage/mail-user.c
diffstat 3 files changed, 27 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-storage/mail-storage-service.c	Wed Feb 22 10:44:00 2017 +0200
+++ b/src/lib-storage/mail-storage-service.c	Wed Feb 22 13:28:43 2017 +0200
@@ -70,6 +70,8 @@
 
 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;
@@ -650,6 +652,7 @@
 	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);
@@ -1229,6 +1232,7 @@
 	}
 
 	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;
@@ -1476,7 +1480,7 @@
 
 	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;
@@ -1485,12 +1489,22 @@
 	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,
--- a/src/lib-storage/mail-storage-service.h	Wed Feb 22 10:44:00 2017 +0200
+++ b/src/lib-storage/mail-storage-service.h	Wed Feb 22 13:28:43 2017 +0200
@@ -112,7 +112,11 @@
 				     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
--- a/src/lib-storage/mail-user.c	Wed Feb 22 10:44:00 2017 +0200
+++ b/src/lib-storage/mail-user.c	Wed Feb 22 13:28:43 2017 +0200
@@ -36,6 +36,8 @@
 		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,
@@ -558,7 +560,10 @@
 
 	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,