Fix deadlock issue

- An application can cause a deadlock if it uses a mutex lock within a callback.

Change-Id: I7d71961a2622dfd763120eacdd63c7e81afe897d
Signed-off-by: Inkyun Kil <inkyun.kil@samsung.com>

diff --git a/src/message_port_remote.c b/src/message_port_remote.c
index 31463fc57764832896e17ec11260fbb13329dbcc..18458d173dcb6380af4c86d4cc7647882c6cb53e 100644 (file)
--- a/src/message_port_remote.c
+++ b/src/message_port_remote.c
@@ -326,10 +326,13 @@ static gboolean __socket_request_handler(GIOChannel *gio,
        int fd = 0;
        message_port_callback_info_s *callback_info;
        message_port_pkt_s *pkt = NULL;
-       message_port_local_port_info_s *local_port_info;
        bundle *kb = NULL;
        bool ret = true;
        bool existed = true;
+       int copied_local_id = 0;
+       char *copied_remote_app_id = NULL;
+       void *user_data = NULL;
+       message_port_message_cb callback = NULL;
 
        callback_info = (message_port_callback_info_s *)data;
        if (callback_info == NULL)
@@ -339,21 +342,29 @@ static gboolean __socket_request_handler(GIOChannel *gio,
        if (__validate_callback_info(callback_info) == false) {
                ret = FALSE;
                existed = FALSE;
+               message_port_unlock_mutex();
                goto out;
        }
 
        if (__validate_local_info(callback_info->local_id) == false) {
                ret = FALSE;
+               message_port_unlock_mutex();
                goto out;
        }
 
-       local_port_info = callback_info->local_info;
-       if (local_port_info == NULL || local_port_info->callback == NULL) {
-               _LOGE("Failed to get callback info");
+       user_data = callback_info->local_info->user_data;
+       callback = callback_info->local_info->callback;
+       copied_local_id = callback_info->local_id;
+       copied_remote_app_id = strdup(callback_info->remote_app_id);
+       if (copied_remote_app_id == NULL) {
+               _LOGE("Failed to copy remote app id");
                ret = FALSE;
+               message_port_unlock_mutex();
                goto out;
        }
 
+       message_port_unlock_mutex();
+
        if (cond == G_IO_HUP) {
                _LOGI("socket G_IO_HUP");
                ret = FALSE;
@@ -382,13 +393,11 @@ static gboolean __socket_request_handler(GIOChannel *gio,
        }
 
        if (pkt->is_bidirection)
-               local_port_info->callback(callback_info->local_id,
-                               callback_info->remote_app_id, pkt->remote_port_name,
-                               pkt->is_trusted, kb, local_port_info->user_data);
+               callback(copied_local_id, copied_remote_app_id, pkt->remote_port_name,
+                               pkt->is_trusted, kb, user_data);
        else
-               local_port_info->callback(callback_info->local_id,
-                               callback_info->remote_app_id, NULL,
-                               pkt->is_trusted, kb, local_port_info->user_data);
+               callback(copied_local_id, copied_remote_app_id, NULL, pkt->is_trusted, kb,
+                               user_data);
 
        bundle_free(kb);
 
@@ -401,10 +410,14 @@ out:
                free(pkt);
        }
 
-       if (ret == FALSE && existed == TRUE)
+       if (ret == FALSE && existed == TRUE) {
+               message_port_lock_mutex();
                __callback_info_free_by_info(callback_info);
+               message_port_unlock_mutex();
+       }
 
-       message_port_unlock_mutex();
+       if (copied_remote_app_id)
+               free(copied_remote_app_id);
 
        return ret;
 }
@@ -488,7 +501,7 @@ static void __callback_info_update_user_data(int local_id, message_port_message_
 /* LCOV_EXCL_STOP */
 }
 
-static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invocation)
+static void __receive_message(GVariant *parameters, GDBusMethodInvocation *invocation)
 {
        char *local_port = NULL;
        char *local_appid = NULL;
@@ -504,6 +517,9 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc
        message_port_local_port_info_s *mi;
        int local_reg_id = 0;
        message_port_callback_info_s *callback_info = NULL;
+       void *user_data = NULL;
+       int copied_local_id = 0;
+       message_port_message_cb callback = NULL;
 
        char buf[1024];
        GDBusMessage *msg;
@@ -511,53 +527,64 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc
        int fd_len;
        int *returned_fds = NULL;
        int fd;
-       bool ret = false;
        char *key_appid;
 
-       message_port_lock_mutex();
        g_variant_get(parameters, "(&s&sbb&s&sbu&s)", &local_appid, &local_port, &local_trusted, &bi_dir,
                        &remote_appid, &remote_port, &remote_trusted, &len, &raw);
 
        if (!remote_port) {
                _LOGE("Invalid argument : remote_port is NULL");
-               goto out;
+               return;
        }
+
        if (!remote_appid) {
                _LOGE("Invalid argument : remote_appid is NULL");
-               goto out;
+               return;
+       }
+
+       if (strcmp(remote_appid, app_id) != 0) {
+               _LOGE("Invalid argument : remote_appid (%s)", remote_appid);
+               return;
+       }
+
+               if (!local_port) {
+               _LOGE("Invalid argument : local_port");
+               return;
        }
 
        if (!local_appid) {
                _LOGE("Invalid argument : local_appid");
-               goto out;
+               return;
        }
 
-       if (!is_local_port_registed(remote_port, remote_trusted, &local_reg_id, &mi)) {
-               _LOGE("Invalid argument : remote_port:(%s) trusted(%d)", remote_port, remote_trusted);
-               goto out;
+       if (!len) {
+               _LOGE("Invalid argument : data_len");
+               return;
        }
 
-       callback_info = __create_callback_info(mi, local_appid);
-       if (callback_info == NULL) {
-               goto out;
+       data = bundle_decode(raw, len);
+       if (!data) {
+               _LOGE("Invalid argument : message");
+               return;
        }
 
-       if (!local_port) {
-               _LOGE("Invalid argument : local_port");
-               goto out;
-       }
-       if (strcmp(remote_appid, app_id) != 0) {
-               _LOGE("Invalid argument : remote_appid (%s)", remote_appid);
-               goto out;
+       message_port_lock_mutex();
+       if (!is_local_port_registed(remote_port, remote_trusted, &local_reg_id, &mi)) {
+               _LOGE("Invalid argument : remote_port:(%s) trusted(%d)", remote_port, remote_trusted);
+               message_port_unlock_mutex();
+               return;
        }
-       if (strcmp(remote_port, callback_info->local_info->port_name) != 0) {
+
+       copied_local_id = mi->local_id;
+       user_data = mi->user_data;
+       callback = mi->callback;
+
+       if (strcmp(remote_port, mi->port_name) != 0) {
                _LOGE("Invalid argument : remote_port (%s)", remote_port);
-               goto out;
-       }
-       if (!len) {
-               _LOGE("Invalid argument : data_len");
-               goto out;
+               message_port_unlock_mutex();
+               return;
        }
+
        if (remote_trusted) {
 /* LCOV_EXCL_START */
                if (g_hash_table_lookup(__trusted_app_list_hash, (gpointer)local_appid) == NULL) {
@@ -570,30 +597,14 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc
                                } else {
                                        _LOGE("The application (%s) is not signed with the same certificate",
                                                        local_appid);
-                                       goto out;
+                                       message_port_unlock_mutex();
+                                       return;
                                }
                        }
                }
 /* LCOV_EXCL_STOP */
        }
 
-       data = bundle_decode(raw, len);
-       if (!data) {
-               _LOGE("Invalid argument : message");
-               goto out;
-       }
-
-       LOGD("call calback %s", local_appid);
-       if (bi_dir)
-               callback_info->local_info->callback(callback_info->local_info->local_id,
-                       local_appid, local_port, local_trusted, data, callback_info->local_info->user_data);
-       else
-               callback_info->local_info->callback(callback_info->local_info->local_id,
-                       local_appid, NULL, false, data, callback_info->local_info->user_data);
-       bundle_free(data);
-
-       ret = true;
-
        msg = g_dbus_method_invocation_get_message(invocation);
        fd_list = g_dbus_message_get_unix_fd_list(msg);
 
@@ -602,18 +613,25 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc
                returned_fds = g_unix_fd_list_steal_fds(fd_list, &fd_len);
                if (returned_fds == NULL) {
                        _LOGE("fail to get fds");
-                       ret = false;
-                       goto out;
+                       goto cb;
                }
                fd = returned_fds[0];
+               g_free(returned_fds);
 
                LOGI("g_unix_fd_list_get %d fd: [%d]", fd_len, fd);
                if (fd > 0) {
+                       callback_info = __create_callback_info(mi, local_appid);
+                       if (callback_info == NULL) {
+                               close(fd);
+                               goto cb;
+                       }
+
                        callback_info->gio_read = g_io_channel_unix_new(fd);
                        if (!callback_info->gio_read) {
                                _LOGE("Error is %s\n", strerror_r(errno, buf, sizeof(buf)));
-                               ret = false;
-                               goto out;
+                               __callback_info_free(callback_info);
+                               close(fd);
+                               goto cb;
                        }
 
                        callback_info->g_src_id = g_io_add_watch_full(
@@ -624,22 +642,23 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc
                                        NULL);
                        if (callback_info->g_src_id == 0) {
                                _LOGE("fail to add watch on socket");
-                               ret = false;
-                               goto out;
+                               __callback_info_free(callback_info);
+                               goto cb;
                        }
                        __callback_info_append(callback_info);
                }
        }
 
-out:
+cb:
        message_port_unlock_mutex();
-       if (ret == false)
-               __callback_info_free(callback_info);
 
-       if (returned_fds)
-               g_free(returned_fds);
-
-       return ret;
+       LOGD("call calback %s", local_appid);
+       if (bi_dir)
+               callback(copied_local_id, local_appid, local_port, local_trusted, data,
+                               user_data);
+       else
+               callback(copied_local_id, local_appid, NULL, false, data, user_data);
+       bundle_free(data);
 }
 
 static void __on_sender_name_appeared(GDBusConnection *connection,