From 668721eb4c29e012c35afa5eda5900c40a9542d1 Mon Sep 17 00:00:00 2001 From: Jeongmo Yang Date: Fri, 8 Feb 2019 14:19:18 +0900 Subject: [PATCH] Replace tbm_key by tbm_fd for buffer protection - Any other process can access buffer if it knows its tbm_key, but, there is no way to access if it's replaced by tbm_fd. [Version] 0.3.17 [Profile] Common [Issue Type] Update [Dependency module] mmsvc-recorder Change-Id: I7a9b78b758e148b138c30ecc654e41599b534b97 Signed-off-by: Jeongmo Yang --- include/recorder_private.h | 1 + packaging/capi-media-recorder.spec | 2 +- src/recorder.c | 176 ++++++++++++------------------------- 3 files changed, 56 insertions(+), 123 deletions(-) diff --git a/include/recorder_private.h b/include/recorder_private.h index 388b5a7..1c308e6 100644 --- a/include/recorder_private.h +++ b/include/recorder_private.h @@ -123,6 +123,7 @@ typedef struct _recorder_message_s { muse_recorder_api_e api; muse_recorder_event_e event; muse_recorder_event_class_e event_class; + int tfd; } recorder_message_s; typedef struct _recorder_idle_event_s { diff --git a/packaging/capi-media-recorder.spec b/packaging/capi-media-recorder.spec index e8ca824..115626f 100644 --- a/packaging/capi-media-recorder.spec +++ b/packaging/capi-media-recorder.spec @@ -1,6 +1,6 @@ Name: capi-media-recorder Summary: A Recorder API -Version: 0.3.16 +Version: 0.3.17 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/recorder.c b/src/recorder.c index 691f291..cc1357b 100644 --- a/src/recorder.c +++ b/src/recorder.c @@ -117,20 +117,20 @@ _DONE: } -static int _recorder_import_tbm_key(tbm_bufmgr bufmgr, unsigned int tbm_key, tbm_bo *bo, tbm_bo_handle *bo_handle) +static int _recorder_import_tbm_fd(tbm_bufmgr bufmgr, int fd, tbm_bo *bo, tbm_bo_handle *bo_handle) { tbm_bo tmp_bo = NULL; tbm_bo_handle tmp_bo_handle = {NULL, }; - if (bufmgr == NULL || bo == NULL || bo_handle == NULL || tbm_key == 0) { - LOGE("invalid parameter - bufmgr %p, bo %p, bo_handle %p, key %d", - bufmgr, bo, bo_handle, tbm_key); + if (bufmgr == NULL || bo == NULL || bo_handle == NULL || fd < 0) { + LOGE("invalid parameter - bufmgr %p, bo %p, bo_handle %p, fd %d", + bufmgr, bo, bo_handle, fd); return false; } - tmp_bo = tbm_bo_import(bufmgr, tbm_key); + tmp_bo = tbm_bo_import_fd(bufmgr, (tbm_fd)fd); if (tmp_bo == NULL) { - LOGE("bo import failed - bufmgr %p, key %d", bufmgr, tbm_key); + LOGE("bo import failed - bufmgr %p, fd %d", bufmgr, fd); return false; } @@ -163,7 +163,7 @@ static void _recorder_release_imported_bo(tbm_bo *bo) return; } -static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *recv_msg, muse_recorder_event_e event) +static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *recv_msg, muse_recorder_event_e event, int tfd) { if (recv_msg == NULL || event >= MUSE_RECORDER_EVENT_TYPE_NUM) { LOGE("invalid parameter - recorder msg %p, event %d", recv_msg, event); @@ -259,7 +259,7 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re break; case MUSE_RECORDER_EVENT_TYPE_AUDIO_STREAM: { - int tbm_key = 0; + int audio_fd = -1; int size = 0; int format = 0; int channel = 0; @@ -268,14 +268,15 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re tbm_bo_handle bo_handle = {.ptr = NULL}; char *send_msg = NULL; - muse_recorder_msg_get(tbm_key, recv_msg); - if (tbm_key == 0) { - LOGE("invalid key"); + if (tfd < 0) { + LOGE("invalid fd %d", tfd); break; } + muse_recorder_msg_get(audio_fd, recv_msg); + if (cb_info->user_cb[event]) { - if (_recorder_import_tbm_key(cb_info->bufmgr, tbm_key, &bo, &bo_handle)) { + if (_recorder_import_tbm_fd(cb_info->bufmgr, tfd, &bo, &bo_handle)) { muse_recorder_msg_get(size, recv_msg); muse_recorder_msg_get(format, recv_msg); muse_recorder_msg_get(channel, recv_msg); @@ -288,23 +289,32 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re /* release imported bo */ _recorder_release_imported_bo(&bo); } else { - LOGE("tbm key %d import failed", tbm_key); + LOGE("tbm fd %d import failed", tfd); } } /* return buffer */ send_msg = muse_core_msg_new(MUSE_RECORDER_API_RETURN_BUFFER, - MUSE_TYPE_INT, "tbm_key", tbm_key, NULL); + MUSE_TYPE_INT, "ret_fd", audio_fd, NULL); - if (muse_core_msg_send(cb_info->fd, send_msg) <= 0) - LOGE("sending message failed"); + if (send_msg) { + if (muse_core_msg_send(cb_info->fd, send_msg) <= 0) + LOGE("sending message failed"); - muse_core_msg_free(send_msg); + muse_core_msg_free(send_msg); + send_msg = NULL; + } else { + LOGE("failed to create send msg for fd %d", audio_fd); + } + + /* close imported fd */ + close(tfd); + tfd = -1; break; } case MUSE_RECORDER_EVENT_TYPE_MUXED_STREAM: { - int tbm_key = 0; + int muxed_fd = -1; int size = 0; int64_t offset = 0; @@ -312,14 +322,15 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re tbm_bo_handle bo_handle = {.ptr = NULL}; char *send_msg = NULL; - muse_recorder_msg_get(tbm_key, recv_msg); - if (tbm_key == 0) { - LOGE("invalid key"); + if (tfd < 0) { + LOGE("invalid fd %d", tfd); break; } + muse_recorder_msg_get(muxed_fd, recv_msg); + if (cb_info->user_cb[event]) { - if (_recorder_import_tbm_key(cb_info->bufmgr, tbm_key, &bo, &bo_handle)) { + if (_recorder_import_tbm_fd(cb_info->bufmgr, tfd, &bo, &bo_handle)) { muse_recorder_msg_get(size, recv_msg); muse_recorder_msg_get(offset, recv_msg); @@ -329,13 +340,13 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re /* release imported bo */ _recorder_release_imported_bo(&bo); } else { - LOGE("tbm key %d import failed", tbm_key); + LOGE("tbm fd %d import failed", tfd); } } /* return buffer */ send_msg = muse_core_msg_new(MUSE_RECORDER_API_RETURN_BUFFER, - MUSE_TYPE_INT, "tbm_key", tbm_key, NULL); + MUSE_TYPE_INT, "ret_fd", muxed_fd, NULL); if (send_msg) { if (muse_core_msg_send(cb_info->fd, send_msg) <= 0) LOGE("sending message failed"); @@ -343,8 +354,12 @@ static void _recorder_client_user_callback(recorder_cb_info_s *cb_info, char *re muse_core_msg_free(send_msg); send_msg = NULL; } else { - LOGE("failed to create send msg for key %d", tbm_key); + LOGE("failed to create send msg for fd %d", tfd); } + + /* close imported fd */ + close(tfd); + tfd = -1; break; } case MUSE_RECORDER_EVENT_TYPE_ERROR: @@ -449,7 +464,7 @@ static gboolean _recorder_idle_event_callback(gpointer data) g_mutex_unlock(&g_rec_idle_event_lock); /* user callback */ - _recorder_client_user_callback(cb_info, rec_idle_event->recv_msg, rec_idle_event->event); + _recorder_client_user_callback(cb_info, rec_idle_event->recv_msg, rec_idle_event->event, -1); IDLE_EVENT_CALLBACK_DONE: /* release event */ @@ -515,7 +530,7 @@ static void _recorder_deactivate_idle_event_all(recorder_cb_info_s *cb_info) } -static void __recorder_add_msg_to_queue(recorder_cb_info_s *cb_info, int api, int event, int event_class, char *msg) +static void __recorder_add_msg_to_queue(recorder_cb_info_s *cb_info, int api, int event, int event_class, char *msg, int tfd) { recorder_message_s *rec_msg = NULL; recorder_msg_handler_info_s *msg_handler_info = NULL; @@ -534,6 +549,7 @@ static void __recorder_add_msg_to_queue(recorder_cb_info_s *cb_info, int api, in rec_msg->api = api; rec_msg->event = event; rec_msg->event_class = event_class; + rec_msg->tfd = tfd; strncpy(rec_msg->recv_msg, msg, sizeof(rec_msg->recv_msg) - 1); @@ -692,7 +708,7 @@ static void __recorder_get_api_operation(int api, recorder_cb_info_s *cb_info, c } -static void __recorder_process_msg(recorder_cb_info_s *cb_info, char *msg) +static void __recorder_process_msg(recorder_cb_info_s *cb_info, char *msg, int tfd) { int ret = RECORDER_ERROR_NONE; int api = -1; @@ -772,7 +788,7 @@ static void __recorder_process_msg(recorder_cb_info_s *cb_info, char *msg) g_mutex_unlock(&cb_info->api_mutex[api]); } else if (api_class == MUSE_RECORDER_API_CLASS_THREAD_SUB || api == MUSE_RECORDER_CB_EVENT) { - __recorder_add_msg_to_queue(cb_info, api, event, event_class, msg); + __recorder_add_msg_to_queue(cb_info, api, event, event_class, msg, tfd); } else { LOGW("unknown recorder api %d and api_class %d", api, api_class); } @@ -848,7 +864,7 @@ static void *_recorder_msg_handler_func(gpointer data) } else if (api == MUSE_RECORDER_CB_EVENT) { switch (rec_msg->event_class) { case MUSE_RECORDER_EVENT_CLASS_THREAD_SUB: - _recorder_client_user_callback(cb_info, rec_msg->recv_msg, rec_msg->event); + _recorder_client_user_callback(cb_info, rec_msg->recv_msg, rec_msg->event, rec_msg->tfd); break; case MUSE_RECORDER_EVENT_CLASS_THREAD_MAIN: rec_idle_event = g_new0(recorder_idle_event_s, 1); @@ -910,14 +926,8 @@ static void *_recorder_msg_handler_func(gpointer data) static void *_recorder_msg_recv_func(gpointer data) { int recv_length = 0; - int single_length = 0; - int remained_length = 0; + int tfd[MUSE_NUM_FD] = {-1, -1, -1, -1}; char *recv_msg = NULL; - char *single_msg = NULL; - char *remained_msg = NULL; - int num_msg = 0; - int cur_pos = 0; - int prev_pos = 0; recorder_cb_info_s *cb_info = (recorder_cb_info_s *)data; if (cb_info == NULL) { @@ -927,16 +937,13 @@ static void *_recorder_msg_recv_func(gpointer data) LOGD("start"); - single_msg = (char *)malloc(sizeof(char) * MUSE_RECORDER_MSG_MAX_LENGTH); - if (single_msg == NULL) { - LOGE("single_msg malloc failed"); - goto CB_HANDLER_EXIT; - } - recv_msg = cb_info->recv_msg; while (g_atomic_int_get(&cb_info->msg_recv_running)) { - recv_length = muse_core_msg_recv(cb_info->fd, recv_msg, MUSE_MSG_MAX_LENGTH); + /* tfd[0] is only used. */ + tfd[0] = -1; + + recv_length = muse_core_msg_recv_fd(cb_info->fd, recv_msg, MUSE_MSG_MAX_LENGTH, tfd); if (recv_length <= 0) { cb_info->is_server_connected = FALSE; LOGE("receive msg failed - server disconnected"); @@ -945,75 +952,9 @@ static void *_recorder_msg_recv_func(gpointer data) recv_msg[recv_length] = '\0'; - cur_pos = 0; - prev_pos = 0; - num_msg = 0; - /*LOGD("recv msg : %s, length : %d", recv_msg, recv_length);*/ - /* Need to split the combined entering msgs */ - for (cur_pos = 0; cur_pos < recv_length; cur_pos++) { - if (recv_msg[cur_pos] == '}') { - single_length = cur_pos - prev_pos + 1; - - if (single_length < MUSE_RECORDER_MSG_MAX_LENGTH) { - /* check remained msg */ - if (remained_length > 0) { - if (remained_msg) { - strncpy(single_msg, remained_msg, remained_length); - strncpy(single_msg + remained_length, recv_msg + prev_pos, single_length); - single_msg[remained_length + single_length] = '\0'; - - free(remained_msg); - remained_msg = NULL; - } else { - strncpy(single_msg, recv_msg + prev_pos, single_length); - single_msg[single_length] = '\0'; - LOGE("lost msg [%s], skip...", single_msg); - } - - remained_length = 0; - } else { - strncpy(single_msg, recv_msg + prev_pos, single_length); - single_msg[single_length] = '\0'; - } - - if (single_msg[0] == '{') { - num_msg++; - /*LOGD("splitted msg : [%s], Index : %d", single_msg, num_msg);*/ - __recorder_process_msg(cb_info, single_msg); - } else { - LOGE("invalid msg [%s]", single_msg); - } - } else { - LOGE("too long message [len %d] skip...", single_length); - } - - prev_pos = cur_pos + 1; - } - } - - /* check incompleted message */ - if (recv_msg[recv_length - 1] != '}') { - remained_length = recv_length - prev_pos; - - LOGW("incompleted message [len %d]", remained_length); - - if (remained_msg) { - free(remained_msg); - remained_msg = NULL; - } - - remained_msg = (char *)malloc(remained_length + 1); - if (remained_msg) { - strncpy(remained_msg, recv_msg + prev_pos, remained_length); - remained_msg[remained_length] = '\0'; - } else { - LOGE("failed to alloc for remained msg"); - } - } else { - remained_length = 0; - } + __recorder_process_msg(cb_info, recv_msg, tfd[0]); } LOGD("client cb exit - server connected %d", cb_info->is_server_connected); @@ -1036,7 +977,8 @@ static void *_recorder_msg_recv_func(gpointer data) MUSE_RECORDER_CB_EVENT, MUSE_RECORDER_EVENT_TYPE_ERROR, MUSE_RECORDER_EVENT_CLASS_THREAD_MAIN, - error_msg); + error_msg, + -1); muse_core_msg_free(error_msg); error_msg = NULL; @@ -1045,16 +987,6 @@ static void *_recorder_msg_recv_func(gpointer data) } CB_HANDLER_EXIT: - if (single_msg) { - free(single_msg); - single_msg = NULL; - } - - if (remained_msg) { - free(remained_msg); - remained_msg = NULL; - } - return NULL; } -- 2.7.4