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 <cookie@samsung.com>

3 files changed, 28 insertions, 29 deletions

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