From 9f0634a4a735ab74a1cb48668dd54166ddcf1584 Mon Sep 17 00:00:00 2001 From: Jiyong Min Date: Tue, 1 Jun 2021 13:45:54 +0900 Subject: [PATCH] Bug fix. read memory after free - 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 | 92 +++++++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/svc/media_controller_svc.c b/svc/media_controller_svc.c index aeafc47..d0ac6e4 100644 --- a/svc/media_controller_svc.c +++ b/svc/media_controller_svc.c @@ -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; -- 2.7.4