Bug fix. read memory after free 85/259085/2 accepted/tizen/unified/20210611.013550 submit/tizen/20210610.063803
authorJiyong Min <jiyong.min@samsung.com>
Tue, 1 Jun 2021 04:45:54 +0000 (13:45 +0900)
committerJiyong Min <jiyong.min@samsung.com>
Tue, 1 Jun 2021 05:57:52 +0000 (14:57 +0900)
 - fix coverity issue
  'iter' pointer variable is used after free.
  After the 'iter' was freed using 'g_list_free_full', it is used in 'iter = g_list_next(iter)'.

Change-Id: I70115a6197b7d02ad9afb64376c920a3ea7fcb03

svc/media_controller_svc.c

index aeafc47..d0ac6e4 100644 (file)
@@ -74,6 +74,28 @@ static int __mc_sys_get_uid(uid_t *uid)
        return users;
 }
 
+static int __mc_compare_pid(gconstpointer a, gconstpointer b)
+{
+       mc_app_data_set_t *_app_data = (mc_app_data_set_t *)a;
+       int cmp_pid = GPOINTER_TO_INT(b);
+
+       mc_retvm_if(!_app_data, -1, "invalid _app_data");
+
+       mc_secure_debug("__mc_compare_pid : %d, %d", _app_data->pid, cmp_pid);
+
+       return (_app_data->pid - cmp_pid);
+}
+
+static int __mc_compare_cmd(gconstpointer a, gconstpointer b)
+{
+       char *_cmd_data = (char *)a;
+       char *cmp_cmd = (char *)b;
+
+       mc_secure_debug("__mc_compare_cmd : %s, %s", _cmd_data, cmp_cmd);
+
+       return g_strcmp0(_cmd_data, cmp_cmd);
+}
+
 static void __mc_destroy_queue(gpointer data)
 {
        mc_service_request *req = (mc_service_request *)data;
@@ -127,7 +149,7 @@ static void __mc_remove_cmd_to_send(gpointer data, gpointer user_data)
 {
        mc_app_data_set_t *_app_data = (mc_app_data_set_t *)data;
        mc_list_user_data *_user_data = (mc_list_user_data *)user_data;
-       GList *iter = NULL;
+       GList *found_cmd = NULL;
 
        mc_retm_if_failed(_app_data);
        mc_retm_if_failed(_user_data);
@@ -135,15 +157,12 @@ static void __mc_remove_cmd_to_send(gpointer data, gpointer user_data)
        mc_retm_if_failed(_user_data->message->msg_size);
        mc_retm_if_failed(MC_STRING_VALID(_user_data->message->msg));
 
-       for (iter = _app_data->cmds_to_send; iter; iter = g_list_next(iter)) {
-               if (!iter->data)
-                       continue;
-
-               if (!g_strcmp0(iter->data, _user_data->message->msg)) {
-                       _app_data->cmds_to_send = g_list_remove_link(_app_data->cmds_to_send, iter);
-                       g_list_free_full(iter, g_free);
-                       _user_data->result++;
-               }
+       while ((found_cmd = g_list_find_custom(_app_data->cmds_to_send, (gconstpointer)_user_data->message->msg,
+                       __mc_compare_cmd)) != NULL) {
+               mc_info("remove cmd: %s", (char *)(found_cmd->data));
+               _app_data->cmds_to_send = g_list_remove_link(_app_data->cmds_to_send, found_cmd);
+               g_list_free_full(found_cmd, g_free);
+               _user_data->result++;
        }
 }
 
@@ -397,7 +416,7 @@ static void __mc_service_remove_connection(GList **connected_list, mc_comm_msg_s
 
        mc_debug_fenter();
 
-       item_found = g_list_find_custom(*connected_list, (gpointer)request_msg, __find_func);
+       item_found = g_list_find_custom(*connected_list, (gconstpointer)request_msg, __find_func);
        mc_retm_if(!item_found, "Already disconnected pid[%d] priv_type[%d]", request_msg->pid, request_msg->priv_type);
 
        *connected_list = g_list_remove_link(*connected_list, item_found);
@@ -474,37 +493,42 @@ static int __mc_notify_server_updated(const char *server_name, mc_server_state_e
        return MEDIA_CONTROLLER_ERROR_NONE;
 }
 
-static int __mc_service_app_dead_handler(int pid, void *data)
+static void __mc_destroy_connected_apps_after_noti(gpointer data)
 {
-       mc_service_t *_service_data = (mc_service_t *)data;
-       GList *iter = NULL;
-       mc_app_data_set_t *_app_data = NULL;
+       mc_app_data_set_t *_app_data = (mc_app_data_set_t *)data;
 
-       mc_info("Received app_dead signal (pid : %d)", pid);
-       mc_retvm_if(!_service_data, AUL_R_ERROR, "data is null!");
-       mc_retvm_if(!_service_data->connected_apps, AUL_R_OK, "No connected application!");
+       mc_retm_if(!data, "invalid data!");
 
-       for (iter = _service_data->connected_apps; iter; iter = g_list_next(iter)) {
-               _app_data = (mc_app_data_set_t *)iter->data;
+       mc_info("app_dead(appid) : %s", _app_data->app_id);
 
-               if ((!_app_data) || (_app_data->pid != pid))
-                       continue;
+       /* Delete and update information of dead application on database */
+       if (MEDIA_CONTROLLER_ERROR_NONE != mc_db_remove_application(_app_data->uid, _app_data->app_id, _app_data->priv_type))
+               mc_error("Fail to remove dead application");
+
+       /* Sends notification for deactivated server via dbus */
+       if (_app_data->priv_type == MC_PRIV_TYPE_SERVER) {
+               if (MEDIA_CONTROLLER_ERROR_NONE != __mc_notify_server_updated(_app_data->app_id, MC_SERVER_STATE_DEACTIVATE))
+                       mc_error("Fail to notify deactivated server");
+       }
 
-               mc_secure_debug("app_dead(appid) : %s", _app_data->app_id);
+       mc_error("[No-error] decreased connection count [%d] pid[%d] priv_type[%d]",
+                               --g_connection_cnt, _app_data->pid, _app_data->priv_type);
+       __mc_destroy_connected_apps(_app_data);
+}
 
-               /* Delete and update information of dead application on database */
-               if (MEDIA_CONTROLLER_ERROR_NONE != mc_db_remove_application(_app_data->uid, _app_data->app_id, _app_data->priv_type))
-                       mc_error("Fail to remove dead application");
+static int __mc_service_app_dead_handler(int pid, void *data)
+{
+       mc_service_t *_service_data = (mc_service_t *)data;
+       GList *found_app = NULL;
 
-               /* Sends notification for deactivated server via dbus */
-               if (_app_data->priv_type == MC_PRIV_TYPE_SERVER) {
-                       if (MEDIA_CONTROLLER_ERROR_NONE != __mc_notify_server_updated(_app_data->app_id, MC_SERVER_STATE_DEACTIVATE))
-                               mc_error("Fail to notify deactivated server");
-               }
+       mc_info("Received app_dead signal (pid : %d)", pid);
+       mc_retvm_if(!_service_data, AUL_R_ERROR, "data is null!");
+       mc_retvm_if(!_service_data->connected_apps, AUL_R_OK, "No connected application!");
 
-               _service_data->connected_apps = g_list_remove_link(_service_data->connected_apps, iter);
-               g_list_free_full(iter, __mc_destroy_connected_apps);
-               mc_error("[No-error] decreased connection count [%d] pid[%d] priv_type[%d]", --g_connection_cnt, _app_data->pid, _app_data->priv_type);
+       while ((found_app = g_list_find_custom(_service_data->connected_apps, (gconstpointer)GINT_TO_POINTER(pid),
+                       __mc_compare_pid)) != NULL) {
+               _service_data->connected_apps = g_list_remove_link(_service_data->connected_apps, found_app);
+               g_list_free_full(found_app, __mc_destroy_connected_apps_after_noti);
        }
 
        return AUL_R_OK;