From 88fb841a5da8943f10679fc1ffbf595d7351cb49 Mon Sep 17 00:00:00 2001 From: Anthony Minessale Date: Thu, 9 Mar 2017 13:22:11 -0600 Subject: [PATCH] FS-10118: [freeswitch-core] Race conditions from lack of error checking in switch_core_session_read_lock #resolve Conflicts: src/switch_core_media.c --- .../applications/mod_dptools/mod_dptools.c | 14 +++++++--- src/switch_core_media.c | 27 +++++++++++++++---- src/switch_cpp.cpp | 3 +-- src/switch_ivr_async.c | 7 ++--- src/switch_ivr_bridge.c | 12 +++++++-- src/switch_ivr_originate.c | 8 ++++-- 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/mod/applications/mod_dptools/mod_dptools.c b/src/mod/applications/mod_dptools/mod_dptools.c index d126101e78..48e18b3934 100644 --- a/src/mod/applications/mod_dptools/mod_dptools.c +++ b/src/mod/applications/mod_dptools/mod_dptools.c @@ -3199,14 +3199,22 @@ struct camping_stake { static void *SWITCH_THREAD_FUNC camp_music_thread(switch_thread_t *thread, void *obj) { struct camping_stake *stake = (struct camping_stake *) obj; - switch_core_session_t *session = stake->session; - switch_channel_t *channel = switch_core_session_get_channel(stake->session); + switch_core_session_t *session; + switch_channel_t *channel; const char *moh = stake->moh, *greet = NULL; switch_input_args_t args = { 0 }; char dbuf[2] = ""; switch_status_t status = SWITCH_STATUS_FALSE; const char *stop; + session = stake->session; + + if (switch_core_session_read_lock(session) != SWITCH_STATUS_SUCCESS) { + return NULL; + } + + channel = switch_core_session_get_channel(stake->session); + if ((stop = switch_channel_get_variable(channel, "campon_stop_key"))) { *dbuf = *stop; } @@ -3215,8 +3223,6 @@ static void *SWITCH_THREAD_FUNC camp_music_thread(switch_thread_t *thread, void args.buf = dbuf; args.buflen = sizeof(dbuf); - switch_core_session_read_lock(session); - /* don't set this to a local_stream:// or you will not be happy */ if ((greet = switch_channel_get_variable(channel, "campon_announce_sound"))) { status = switch_ivr_play_file(session, NULL, greet, &args); diff --git a/src/switch_core_media.c b/src/switch_core_media.c index 966bc491fc..727005b00b 100644 --- a/src/switch_core_media.c +++ b/src/switch_core_media.c @@ -84,6 +84,7 @@ struct media_helper { switch_mutex_t *file_read_mutex; switch_mutex_t *file_write_mutex; int up; + int ready; }; typedef enum { @@ -5733,8 +5734,8 @@ SWITCH_DECLARE(void) switch_core_autobind_cpu(void) static void *SWITCH_THREAD_FUNC video_helper_thread(switch_thread_t *thread, void *obj) { struct media_helper *mh = obj; - switch_core_session_t *session = mh->session; - switch_channel_t *channel = switch_core_session_get_channel(session); + switch_core_session_t *session; + switch_channel_t *channel; switch_status_t status; switch_frame_t *read_frame = NULL; switch_media_handle_t *smh; @@ -5748,10 +5749,21 @@ static void *SWITCH_THREAD_FUNC video_helper_thread(switch_thread_t *thread, voi int buflen = SWITCH_RTP_MAX_BUF_LEN; int blank_enabled = 1; + session = mh->session; + + if (switch_core_session_read_lock(session) != SWITCH_STATUS_SUCCESS) { + mh->ready = -1; + return NULL; + } + + mh->ready = 1; + if (!(smh = session->media_handle)) { return NULL; } + channel = switch_core_session_get_channel(session); + switch_core_autobind_cpu(); if ((var = switch_channel_get_variable(session->channel, "core_video_blank_image"))) { @@ -5774,8 +5786,6 @@ static void *SWITCH_THREAD_FUNC video_helper_thread(switch_thread_t *thread, voi v_engine = &smh->engines[SWITCH_MEDIA_TYPE_VIDEO]; v_engine->thread_id = switch_thread_self(); - switch_core_session_read_lock(session); - mh->up = 1; switch_mutex_lock(mh->cond_mutex); @@ -5931,7 +5941,14 @@ SWITCH_DECLARE(switch_status_t) switch_core_session_start_video_thread(switch_co switch_mutex_init(&v_engine->mh.file_write_mutex, SWITCH_MUTEX_NESTED, pool); switch_mutex_init(&smh->read_mutex[SWITCH_MEDIA_TYPE_VIDEO], SWITCH_MUTEX_NESTED, pool); switch_mutex_init(&smh->write_mutex[SWITCH_MEDIA_TYPE_VIDEO], SWITCH_MUTEX_NESTED, pool); - switch_thread_create(&v_engine->media_thread, thd_attr, video_helper_thread, &v_engine->mh, switch_core_session_get_pool(session)); + v_engine->mh.ready = 0; + + if (switch_thread_create(&v_engine->media_thread, thd_attr, video_helper_thread, &v_engine->mh, + switch_core_session_get_pool(session)) == SWITCH_STATUS_SUCCESS) { + while(!v_engine->mh.ready) { + switch_cond_next(); + } + } switch_mutex_unlock(smh->control_mutex); return SWITCH_STATUS_SUCCESS; diff --git a/src/switch_cpp.cpp b/src/switch_cpp.cpp index 5471eafcb7..95466c3a76 100644 --- a/src/switch_cpp.cpp +++ b/src/switch_cpp.cpp @@ -635,11 +635,10 @@ SWITCH_DECLARE_CONSTRUCTOR CoreSession::CoreSession(switch_core_session_t *new_s { init_vars(); - if (new_session) { + if (new_session && switch_core_session_read_lock_hangup(new_session) == SWITCH_STATUS_SUCCESS) { session = new_session; channel = switch_core_session_get_channel(session); allocated = 1; - switch_core_session_read_lock_hangup(session); uuid = strdup(switch_core_session_get_uuid(session)); } } diff --git a/src/switch_ivr_async.c b/src/switch_ivr_async.c index e52884fd99..1e87cc27eb 100644 --- a/src/switch_ivr_async.c +++ b/src/switch_ivr_async.c @@ -3909,9 +3909,10 @@ static void *SWITCH_THREAD_FUNC bcast_thread(switch_thread_t *thread, void *obj) return NULL; } - switch_core_session_read_lock(bch->session); - switch_ivr_broadcast(switch_core_session_get_uuid(bch->session), bch->app, bch->flags); - switch_core_session_rwunlock(bch->session); + if (switch_core_session_read_lock(bch->session) == SWITCH_STATUS_SUCCESS) { + switch_ivr_broadcast(switch_core_session_get_uuid(bch->session), bch->app, bch->flags); + switch_core_session_rwunlock(bch->session); + } return NULL; diff --git a/src/switch_ivr_bridge.c b/src/switch_ivr_bridge.c index f20cde5ef1..654aa492ac 100644 --- a/src/switch_ivr_bridge.c +++ b/src/switch_ivr_bridge.c @@ -58,8 +58,16 @@ static void video_bridge_thread(switch_core_session_t *session, void *obj) vh->up = 1; - switch_core_session_read_lock(vh->session_a); - switch_core_session_read_lock(vh->session_b); + if (switch_core_session_read_lock(vh->session_a) != SWITCH_STATUS_SUCCESS) { + vh->up = 0; + return; + } + + if (switch_core_session_read_lock(vh->session_b) != SWITCH_STATUS_SUCCESS) { + vh->up = 0; + switch_core_session_rwunlock(vh->session_a); + return; + } switch_core_session_request_video_refresh(vh->session_a); switch_core_session_request_video_refresh(vh->session_b); diff --git a/src/switch_ivr_originate.c b/src/switch_ivr_originate.c index 94002c4700..28f1f2bc06 100644 --- a/src/switch_ivr_originate.c +++ b/src/switch_ivr_originate.c @@ -1423,10 +1423,14 @@ static void *SWITCH_THREAD_FUNC enterprise_originate_ringback_thread(switch_thre { struct ent_originate_ringback *rb_data = (struct ent_originate_ringback *) obj; switch_core_session_t *session = rb_data->session; - switch_channel_t *channel = switch_core_session_get_channel(rb_data->session); + switch_channel_t *channel; switch_status_t status = SWITCH_STATUS_FALSE; - switch_core_session_read_lock(session); + if (switch_core_session_read_lock(session) != SWITCH_STATUS_SUCCESS) { + return NULL; + } + + channel = switch_core_session_get_channel(session); while (rb_data->running && switch_channel_ready(channel)) { switch_ivr_parse_all_messages(session);