From: Vignesh Shanmuga Sunder Date: Wed, 4 Jan 2017 10:49:13 +0000 (+0530) Subject: Fix memory access issue in browser provider slots X-Git-Tag: submit/tizen/20170412.010038~6 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0901620ad36ba5315dfbb1647a67cf5f49f11fdf;p=platform%2Fframework%2Fweb%2Fbrowser-provider.git Fix memory access issue in browser provider slots [Model] COMMON [BinType] AP [Customer] OPEN [Issue#] P170103-03175 [Request] PLM [Occurrence Version] Latest [Problem] Crash observed when we try to open youtube page in browser (Once crash) [Cause & Measure] slot->client memory was accessed when it was getting free [Checking Method] N/A [Team] BrowserUI [Developer] Vignesh Shanmuga Sunder (vgn.sunder) [Solution company] Samsung [Change Type] Apply Patch Change-Id: I3384a1674f44417a76a34991c375e0a302894254 Signed-off-by: HyeKyoung Hwang --- diff --git a/provider/browser-provider-requests-manager.c b/provider/browser-provider-requests-manager.c index 8b05fe2..8912c5e 100755 --- a/provider/browser-provider-requests-manager.c +++ b/provider/browser-provider-requests-manager.c @@ -475,8 +475,7 @@ void *client_thread_idle(void *arg) FD_CLR(client->cmd_socket, &imask); FD_CLR(client->cmd_socket, &emask); } - bp_client_free(client); - slot->client = NULL; + bp_remove_client_from_slot(slot); return 0; } #endif @@ -660,8 +659,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) client->cmd_socket); if (client->cmd_socket >= 0) FD_CLR(client->cmd_socket, &listen_fdset); - bp_client_free(client); - privates->slots[oldest_index].client = NULL; + bp_remove_client_from_slot(&privates->slots[oldest_index]); i = oldest_index; } else { TRACE_ERROR("[CRITICAL] No space in slots"); @@ -720,8 +718,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("client type"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } else if (!strcmp(privates->slots[i].client->privilege_label, "inhouse")) { // inhouse APIs (tab & scrap) have no privilege label. @@ -734,8 +731,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("failed to create cynara client identification string"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } if(cynara_creds_socket_get_user(clientfd, USER_METHOD_UID, @@ -743,8 +739,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("failed to create cynara user identification string"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } pid_t cynara_pid; @@ -752,16 +747,14 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("failed to create PID string for client session"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } if((privates->slots[i].client->cynara_session = cynara_session_from_pid(cynara_pid)) == NULL) { TRACE_ERROR("failed to create client session"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } @@ -779,8 +772,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("client have no privilege permission"); bp_ipc_send_errorcode(clientfd, BP_ERROR_PERMISSION_DENY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } #endif @@ -797,8 +789,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) if (privates->slots[i].client->notify < 0) { TRACE_ERROR("failed to open fifo slot:%d", clientfd); bp_ipc_send_errorcode(clientfd, BP_ERROR_IO_ERROR); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } @@ -817,8 +808,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) TRACE_ERROR("[ERROR][main_thread] create\n"); bp_ipc_send_errorcode(clientfd, BP_ERROR_OUT_OF_MEMORY); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } pthread_detach(privates->slots[i].client->tid); @@ -842,8 +832,7 @@ void bp_thread_requests_manager(bp_privates_defs *privates) continue; if (privates->slots[i].client->cmd_socket < 0){ TRACE_ERROR("[CRITICAL] [i] Found Bad socket", i); - bp_client_free(privates->slots[i].client); - privates->slots[i].client = NULL; + bp_remove_client_from_slot(&privates->slots[i]); continue; } @@ -855,10 +844,9 @@ void bp_thread_requests_manager(bp_privates_defs *privates) if (__handle_client_request (privates->slots[i].client) == BP_ERROR_IO_ERROR) { TRACE_ERROR("disconnect client slot:%d sock:%d", i, - client->cmd_socket); - FD_CLR(client->cmd_socket, &listen_fdset); - bp_client_free(client); - privates->slots[i].client = NULL; + privates->slots[i].client->cmd_socket); + FD_CLR(privates->slots[i].client->cmd_socket, &listen_fdset); + bp_remove_client_from_slot(&privates->slots[i]); } } // FD_ISSET } // BP_MAX_CLIENT diff --git a/provider/browser-provider-slots.c b/provider/browser-provider-slots.c index bf09570..8427405 100755 --- a/provider/browser-provider-slots.c +++ b/provider/browser-provider-slots.c @@ -107,14 +107,24 @@ int bp_client_free(bp_client_defs *client) return 0; } +int bp_remove_client_from_slot(bp_client_slots_defs *slot) +{ + if (slot == NULL || slot->client == NULL) + return -1; + + // First set NULL and then free client + bp_client_defs *_client = slot->client; + slot->client = NULL; + return bp_client_free(_client); +} + void bp_client_slots_free(bp_client_slots_defs *slots, int size) { int i = 0; if (slots != NULL) { for (; i < size; i++) { - if (slots[i].client == NULL) - bp_client_free(slots[i].client); - slots[i].client = NULL; + if (slots[i].client != NULL) + bp_remove_client_from_slot(&slots[i]); pthread_mutex_destroy(&slots[i].mutex); } free(slots); diff --git a/provider/include/browser-provider-slots.h b/provider/include/browser-provider-slots.h index dda8dc8..ea61061 100755 --- a/provider/include/browser-provider-slots.h +++ b/provider/include/browser-provider-slots.h @@ -56,6 +56,7 @@ typedef struct { int bp_create_unique_id(void); bp_client_slots_defs *bp_client_slots_new(int size); int bp_client_free(bp_client_defs *client); +int bp_remove_client_from_slot(bp_client_slots_defs *slot); void bp_client_slots_free(bp_client_slots_defs *slots, int size); #endif