Fix ASAN issue: heap-use-after-free

- The callback for sound play could be called after handle is freed.
- This patch allocates structures for the parameter of callback.
  It will not be freed if the callback is not called when handle is destroyed
  and prevents the heap-use-after-free issue as a result.

[Version] 0.10.285
[Issue Type] ASAN issue

Change-Id: Ie15ed724b47d9f4615d4a476420e02fbd203fb27
Signed-off-by: Jeongmo Yang <jm80.yang@samsung.com>

diff --git a/packaging/libmm-camcorder.spec b/packaging/libmm-camcorder.spec
index e9fcbf1..625951c 100755 (executable)
--- a/packaging/libmm-camcorder.spec
+++ b/packaging/libmm-camcorder.spec
@@ -1,6 +1,6 @@
 Name:       libmm-camcorder
 Summary:    Camera and recorder library
-Version:    0.10.284
+Version:    0.10.285
 Release:    0
 Group:      Multimedia/Libraries
 License:    Apache-2.0

diff --git a/src/include/mm_camcorder_internal.h b/src/include/mm_camcorder_internal.h
index 09d4d4e..b984024 100644 (file)
--- a/src/include/mm_camcorder_internal.h
+++ b/src/include/mm_camcorder_internal.h
@@ -851,8 +851,8 @@ typedef struct mmf_camcorder {
 
        /* gdbus */
        GDBusConnection *gdbus_conn;                            /**< gdbus connection */
-       _MMCamcorderGDbusCbInfo gdbus_info_sound;               /**< Information for the gdbus cb of sound play */
-       _MMCamcorderGDbusCbInfo gdbus_info_solo_sound;          /**< Information for the gdbus cb of solo sound play */
+       _MMCamcorderGDbusCbInfo *gdbus_info_sound;              /**< Information for the gdbus cb of sound play */
+       _MMCamcorderGDbusCbInfo *gdbus_info_solo_sound;         /**< Information for the gdbus cb of solo sound play */
 
        /* DPM(device policy manager) */
        device_policy_manager_h dpm_handle;                     /**< DPM handle */

diff --git a/src/include/mm_camcorder_util.h b/src/include/mm_camcorder_util.h
index b322f68..7e7eb49 100644 (file)
--- a/src/include/mm_camcorder_util.h
+++ b/src/include/mm_camcorder_util.h
@@ -213,7 +213,7 @@ typedef struct {
        int param;
        int is_playing;
        guint subscribe_id;
-       void *mm_handle;
+       gboolean free_in_cb;
 } _MMCamcorderGDbusCbInfo;
 
 /**
@@ -349,6 +349,9 @@ int _mmcamcorder_get_camera_id(int device_type, int *camera_id);
 int _mmcamcorder_get_device_led_brightness(GDBusConnection *conn, int *brightness);
 
 /* sound play via dbus*/
+_MMCamcorderGDbusCbInfo *_mmcamcorder_gdbus_info_new(void);
+void _mmcamcorder_gdbus_info_free(_MMCamcorderGDbusCbInfo *info);
+void _mmcamcorder_gdbus_info_check_free(_MMCamcorderGDbusCbInfo *info);
 int _mmcamcorder_send_sound_play_message(GDBusConnection *conn, _MMCamcorderGDbusCbInfo *gdbus_info,
        const char *sample_name, const char *stream_role, const char *volume_gain, int sync_play);
 

diff --git a/src/mm_camcorder_internal.c b/src/mm_camcorder_internal.c
index 0069000..eb34cec 100644 (file)
--- a/src/mm_camcorder_internal.c
+++ b/src/mm_camcorder_internal.c
@@ -155,13 +155,8 @@ static gint __mmcamcorder_init_handle(mmf_camcorder_t **hcamcorder, int device_t
        new_handle->task_thread_state = _MMCAMCORDER_TASK_THREAD_STATE_NONE;
 
        if (device_type != MM_VIDEO_DEVICE_NONE) {
-               new_handle->gdbus_info_sound.mm_handle = new_handle;
-               g_mutex_init(&new_handle->gdbus_info_sound.sync_mutex);
-               g_cond_init(&new_handle->gdbus_info_sound.sync_cond);
-
-               new_handle->gdbus_info_solo_sound.mm_handle = new_handle;
-               g_mutex_init(&new_handle->gdbus_info_solo_sound.sync_mutex);
-               g_cond_init(&new_handle->gdbus_info_solo_sound.sync_cond);
+               new_handle->gdbus_info_sound = _mmcamcorder_gdbus_info_new();
+               new_handle->gdbus_info_solo_sound = _mmcamcorder_gdbus_info_new();
        }
 
        /* create task thread */
@@ -264,10 +259,8 @@ static void __mmcamcorder_deinit_handle(mmf_camcorder_t *hcamcorder)
        g_mutex_clear(&hcamcorder->restart_preview_lock);
 
        if (hcamcorder->device_type != MM_VIDEO_DEVICE_NONE) {
-               g_mutex_clear(&hcamcorder->gdbus_info_sound.sync_mutex);
-               g_cond_clear(&hcamcorder->gdbus_info_sound.sync_cond);
-               g_mutex_clear(&hcamcorder->gdbus_info_solo_sound.sync_mutex);
-               g_cond_clear(&hcamcorder->gdbus_info_solo_sound.sync_cond);
+               _mmcamcorder_gdbus_info_check_free(hcamcorder->gdbus_info_sound);
+               _mmcamcorder_gdbus_info_check_free(hcamcorder->gdbus_info_solo_sound);
        }
 
        g_mutex_clear(&hcamcorder->task_thread_lock);
@@ -1417,23 +1410,6 @@ int _mmcamcorder_stop(MMHandleType handle)
 
        _mmcamcorder_set_state(handle, MM_CAMCORDER_STATE_READY);
 
-       if (hcamcorder->type != MM_CAMCORDER_MODE_AUDIO) {
-               /* unsubscribe remained unsubscribed signal */
-               g_mutex_lock(&hcamcorder->gdbus_info_sound.sync_mutex);
-               if (hcamcorder->gdbus_info_sound.subscribe_id > 0) {
-                       MMCAM_LOG_WARNING("subscribe_id[%u] is remained. remove it.", hcamcorder->gdbus_info_sound.subscribe_id);
-                       g_dbus_connection_signal_unsubscribe(hcamcorder->gdbus_conn, hcamcorder->gdbus_info_sound.subscribe_id);
-               }
-               g_mutex_unlock(&hcamcorder->gdbus_info_sound.sync_mutex);
-
-               g_mutex_lock(&hcamcorder->gdbus_info_solo_sound.sync_mutex);
-               if (hcamcorder->gdbus_info_solo_sound.subscribe_id > 0) {
-                       MMCAM_LOG_WARNING("subscribe_id[%u] is remained. remove it.", hcamcorder->gdbus_info_solo_sound.subscribe_id);
-                       g_dbus_connection_signal_unsubscribe(hcamcorder->gdbus_conn, hcamcorder->gdbus_info_solo_sound.subscribe_id);
-               }
-               g_mutex_unlock(&hcamcorder->gdbus_info_solo_sound.sync_mutex);
-       }
-
        _MMCAMCORDER_UNLOCK_CMD(hcamcorder);
 
        return MM_ERROR_NONE;

diff --git a/src/mm_camcorder_sound.c b/src/mm_camcorder_sound.c
index 4b2f3d5..96301ae 100644 (file)
--- a/src/mm_camcorder_sound.c
+++ b/src/mm_camcorder_sound.c
@@ -26,12 +26,9 @@
 #include "mm_camcorder_sound.h"
 
 /*---------------------------------------------------------------------------------------
-|    GLOBAL VARIABLE DEFINITIONS for internal                                          |
----------------------------------------------------------------------------------------*/
-
-/*---------------------------------------------------------------------------------------
 |    LOCAL VARIABLE DEFINITIONS for internal                                           |
 ---------------------------------------------------------------------------------------*/
+#define _MMCAMCORDER_SOLO_PLAY_WAIT_TIMEOUT (G_TIME_SPAN_SECOND * 3)
 
 
 /*---------------------------------------------------------------------------------------
@@ -126,7 +123,7 @@ gboolean _mmcamcorder_sound_play(MMHandleType handle, const char *sample_name, g
        MMCAM_LOG_INFO("Play start - sample name [%s]", sample_name);
 
        _mmcamcorder_send_sound_play_message(hcamcorder->gdbus_conn,
-               &hcamcorder->gdbus_info_sound, sample_name, "system", volume_gain, sync_play);
+               hcamcorder->gdbus_info_sound, sample_name, "system", volume_gain, sync_play);
 
        g_mutex_unlock(&info->open_mutex);
 
@@ -199,7 +196,7 @@ int _mmcamcorder_sound_solo_play(MMHandleType handle, const char *sample_name, g
        if (hcamcorder->shutter_sound_policy == VCONFKEY_CAMERA_SHUTTER_SOUND_POLICY_ON ||
                hcamcorder->sub_context->info_image->sound_status) {
                _mmcamcorder_send_sound_play_message(hcamcorder->gdbus_conn,
-                       &hcamcorder->gdbus_info_solo_sound, sample_name, "system", "shutter1", sync_play);
+                       hcamcorder->gdbus_info_solo_sound, sample_name, "system", "shutter1", sync_play);
        } else {
                MMCAM_LOG_WARNING("skip shutter sound : sound policy %d, sound status %d",
                        hcamcorder->shutter_sound_policy, hcamcorder->sub_context->info_image->sound_status);
@@ -220,17 +217,17 @@ void _mmcamcorder_sound_solo_play_wait(MMHandleType handle)
        MMCAM_LOG_INFO("START");
 
        /* check playing sound count */
-       g_mutex_lock(&hcamcorder->gdbus_info_solo_sound.sync_mutex);
+       g_mutex_lock(&hcamcorder->gdbus_info_solo_sound->sync_mutex);
 
-       if (hcamcorder->gdbus_info_solo_sound.is_playing) {
+       if (hcamcorder->gdbus_info_solo_sound->is_playing) {
                gint64 end_time = 0;
 
                MMCAM_LOG_INFO("Wait for signal");
 
-               end_time = g_get_monotonic_time() + G_TIME_SPAN_SECOND;
+               end_time = g_get_monotonic_time() + _MMCAMCORDER_SOLO_PLAY_WAIT_TIMEOUT;
 
-               if (g_cond_wait_until(&hcamcorder->gdbus_info_solo_sound.sync_cond,
-                       &hcamcorder->gdbus_info_solo_sound.sync_mutex, end_time)) {
+               if (g_cond_wait_until(&hcamcorder->gdbus_info_solo_sound->sync_cond,
+                       &hcamcorder->gdbus_info_solo_sound->sync_mutex, end_time)) {
                        MMCAM_LOG_INFO("signal received.");
                } else {
                        MMCAM_LOG_WARNING("capture sound play timeout.");
@@ -239,7 +236,7 @@ void _mmcamcorder_sound_solo_play_wait(MMHandleType handle)
                MMCAM_LOG_WARNING("no playing sound");
        }
 
-       g_mutex_unlock(&hcamcorder->gdbus_info_solo_sound.sync_mutex);
+       g_mutex_unlock(&hcamcorder->gdbus_info_solo_sound->sync_mutex);
 
        MMCAM_LOG_INFO("DONE");
 

diff --git a/src/mm_camcorder_util.c b/src/mm_camcorder_util.c
index 42afe2d..3614ffe 100644 (file)
--- a/src/mm_camcorder_util.c
+++ b/src/mm_camcorder_util.c
@@ -166,37 +166,44 @@ static void __gdbus_stream_eos_cb(GDBusConnection *connection,
 {
        int played_idx = 0;
        gboolean stopped_by_user = FALSE;
-       _MMCamcorderGDbusCbInfo *gdbus_info = NULL;
-       mmf_camcorder_t *hcamcorder = NULL;
-
-       MMCAM_LOG_WARNING("entered");
+       gboolean do_free = FALSE;
+       _MMCamcorderGDbusCbInfo *info = NULL;
 
        if (!param || !user_data) {
                MMCAM_LOG_ERROR("invalid parameter %p %p", param, user_data);
                return;
        }
 
-       gdbus_info = (_MMCamcorderGDbusCbInfo *)user_data;
-       hcamcorder = (mmf_camcorder_t *)gdbus_info->mm_handle;
+       info = (_MMCamcorderGDbusCbInfo *)user_data;
+
+       MMCAM_LOG_WARNING("entered[gdbus_info:%p]", info);
 
        g_variant_get(param, "(ib)", &played_idx, &stopped_by_user);
 
-       g_mutex_lock(&gdbus_info->sync_mutex);
+       g_mutex_lock(&info->sync_mutex);
 
-       MMCAM_LOG_WARNING("gdbus_info->param %d, played_idx %d, stopped_by_user %d, handle %p",
-               gdbus_info->param, played_idx, stopped_by_user, hcamcorder);
+       MMCAM_LOG_WARNING("gdbus_info->param %d, played_idx %d, stopped_by_user %d",
+               info->param, played_idx, stopped_by_user);
 
-       if (gdbus_info->param == played_idx) {
-               g_dbus_connection_signal_unsubscribe(connection, gdbus_info->subscribe_id);
+       if (info->param == played_idx) {
+               g_dbus_connection_signal_unsubscribe(connection, info->subscribe_id);
 
-               gdbus_info->is_playing = FALSE;
-               gdbus_info->subscribe_id = 0;
-               gdbus_info->param = 0;
+               info->is_playing = FALSE;
+               info->subscribe_id = 0;
+               info->param = 0;
 
-               g_cond_signal(&gdbus_info->sync_cond);
+               g_cond_signal(&info->sync_cond);
        }
 
-       g_mutex_unlock(&gdbus_info->sync_mutex);
+       if (info->free_in_cb)
+               do_free = TRUE;
+
+       g_mutex_unlock(&info->sync_mutex);
+
+       if (do_free) {
+               MMCAM_LOG_WARNING("free gdbus_info[%p]", info);
+               _mmcamcorder_gdbus_info_free(info);
+       }
 
        MMCAM_LOG_WARNING("done");
 
@@ -943,6 +950,60 @@ int _mmcamcorder_get_device_led_brightness(GDBusConnection *conn, int *brightnes
 }
 
 
+_MMCamcorderGDbusCbInfo *_mmcamcorder_gdbus_info_new(void)
+{
+       _MMCamcorderGDbusCbInfo *new_info = g_new0(_MMCamcorderGDbusCbInfo, 1);
+
+       g_mutex_init(&new_info->sync_mutex);
+       g_cond_init(&new_info->sync_cond);
+
+       MMCAM_LOG_INFO("new gdbus_info[%p]", new_info);
+
+       return new_info;
+}
+
+
+void _mmcamcorder_gdbus_info_free(_MMCamcorderGDbusCbInfo *info)
+{
+       if (!info) {
+               MMCAM_LOG_WARNING("NULL info");
+               return;
+       }
+
+       MMCAM_LOG_INFO("free gdbus_info[%p]", info);
+
+       g_mutex_clear(&info->sync_mutex);
+       g_cond_clear(&info->sync_cond);
+
+       g_free(info);
+}
+
+
+void _mmcamcorder_gdbus_info_check_free(_MMCamcorderGDbusCbInfo *info)
+{
+       gboolean do_free = FALSE;
+
+       if (!info) {
+               MMCAM_LOG_WARNING("NULL info");
+               return;
+       }
+
+       g_mutex_lock(&info->sync_mutex);
+
+       if (info->subscribe_id > 0) {
+               MMCAM_LOG_WARNING("gdbus_info[%p] will be freed in sound EOS cb", info);
+               info->free_in_cb = TRUE;
+       } else {
+               do_free = TRUE;
+       }
+
+       g_mutex_unlock(&info->sync_mutex);
+
+       if (do_free)
+               _mmcamcorder_gdbus_info_free(info);
+}
+
+
 int _mmcamcorder_send_sound_play_message(GDBusConnection *conn, _MMCamcorderGDbusCbInfo *gdbus_info,
        const char *sample_name, const char *stream_role, const char *volume_gain, int sync_play)
 {