diff options
author | Inkyun Kil <inkyun.kil@samsung.com> | 2024-07-05 09:19:03 +0900 |
---|---|---|
committer | Inkyun Kil <inkyun.kil@samsung.com> | 2024-07-05 13:06:58 +0900 |
commit | dff6f32f3e31caaa80d4ca07140f70c2acf9db88 (patch) | |
tree | ddf2c400c5127589203f7f4ecacba905f43d4442 | |
parent | 98692197cdceb609713d9be20f33880e9681a716 (diff) | |
download | message-port-dff6f32f3e31caaa80d4ca07140f70c2acf9db88.tar.gz message-port-dff6f32f3e31caaa80d4ca07140f70c2acf9db88.tar.bz2 message-port-dff6f32f3e31caaa80d4ca07140f70c2acf9db88.zip |
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>
-rw-r--r-- | src/message_port_remote.c | 155 |
1 files changed, 87 insertions, 68 deletions
diff --git a/src/message_port_remote.c b/src/message_port_remote.c index 31463fc..18458d1 100644 --- 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, |