From 20a893fd0683de64353ee2c111aa9c61a867886d Mon Sep 17 00:00:00 2001 From: Andrey Volk Date: Fri, 6 Sep 2019 17:02:52 +0400 Subject: [PATCH] FS-12038: [mod_sofia, core] Fix potential leak and race in chat_hash, add switch_core_hash_insert_auto_free(). --- src/include/switch_core.h | 10 ++++++++++ src/mod/endpoints/mod_sofia/mod_sofia.c | 11 ++--------- src/mod/endpoints/mod_sofia/sofia.c | 7 ++++--- src/mod/endpoints/mod_sofia/sofia_presence.c | 2 +- src/switch_core_hash.c | 9 +++++++++ 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/include/switch_core.h b/src/include/switch_core.h index 8dcea8acf3..bc7b70318b 100644 --- a/src/include/switch_core.h +++ b/src/include/switch_core.h @@ -1424,6 +1424,16 @@ SWITCH_DECLARE(switch_status_t) switch_core_hash_init_case(_Out_ switch_hash_t * */ SWITCH_DECLARE(switch_status_t) switch_core_hash_destroy(_Inout_ switch_hash_t **hash); +/*! + \brief Insert data into a hash and set flags so the value is automatically freed on delete + \param hash the hash to add data to + \param key the name of the key to add the data to + \param data the data to add + \return SWITCH_STATUS_SUCCESS if the data is added + \note the string key must be a constant or a dynamic string +*/ +SWITCH_DECLARE(switch_status_t) switch_core_hash_insert_auto_free(switch_hash_t *hash, const char *key, const void *data); + /*! \brief Insert data into a hash \param hash the hash to add data to diff --git a/src/mod/endpoints/mod_sofia/mod_sofia.c b/src/mod/endpoints/mod_sofia/mod_sofia.c index 848f1e6924..72ac5b01b9 100644 --- a/src/mod/endpoints/mod_sofia/mod_sofia.c +++ b/src/mod/endpoints/mod_sofia/mod_sofia.c @@ -360,7 +360,6 @@ switch_status_t sofia_on_destroy(switch_core_session_t *session) { private_object_t *tech_pvt = (private_object_t *) switch_core_session_get_private(session); switch_channel_t *channel = switch_core_session_get_channel(session); - char *uuid; switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_DEBUG, "%s SOFIA DESTROY\n", switch_channel_get_name(channel)); @@ -376,13 +375,7 @@ switch_status_t sofia_on_destroy(switch_core_session_t *session) } if (!zstr(tech_pvt->call_id)) { - switch_mutex_lock(tech_pvt->profile->flag_mutex); - if ((uuid = switch_core_hash_find(tech_pvt->profile->chat_hash, tech_pvt->call_id))) { - free(uuid); - uuid = NULL; - switch_core_hash_delete(tech_pvt->profile->chat_hash, tech_pvt->call_id); - } - switch_mutex_unlock(tech_pvt->profile->flag_mutex); + switch_core_hash_delete_locked(tech_pvt->profile->chat_hash, tech_pvt->call_id, tech_pvt->profile->flag_mutex); } @@ -461,7 +454,7 @@ switch_status_t sofia_on_hangup(switch_core_session_t *session) switch_channel_get_name(channel), switch_channel_cause2str(cause)); if (tech_pvt->hash_key && !sofia_test_pflag(tech_pvt->profile, PFLAG_DESTROY)) { - switch_core_hash_delete(tech_pvt->profile->chat_hash, tech_pvt->hash_key); + switch_core_hash_delete_locked(tech_pvt->profile->chat_hash, tech_pvt->hash_key, tech_pvt->profile->flag_mutex); } if (session && tech_pvt->profile->pres_type) { diff --git a/src/mod/endpoints/mod_sofia/sofia.c b/src/mod/endpoints/mod_sofia/sofia.c index 0b0b1b82a0..5c3a781153 100644 --- a/src/mod/endpoints/mod_sofia/sofia.c +++ b/src/mod/endpoints/mod_sofia/sofia.c @@ -2430,7 +2430,7 @@ void sofia_event_callback(nua_event_t event, tech_pvt->nh = NULL; sofia_set_flag(tech_pvt, TFLAG_BYE); switch_mutex_lock(profile->flag_mutex); - switch_core_hash_insert(profile->chat_hash, tech_pvt->call_id, strdup(switch_core_session_get_uuid(session))); + switch_core_hash_insert_auto_free(profile->chat_hash, tech_pvt->call_id, strdup(switch_core_session_get_uuid(session))); switch_mutex_unlock(profile->flag_mutex); nua_handle_destroy(nh); } else { @@ -2519,10 +2519,11 @@ void sofia_event_callback(nua_event_t event, if (sip->sip_call_id && sip->sip_call_id->i_id) { - char *uuid; + char *uuid = NULL, *tmp; switch_mutex_lock(profile->flag_mutex); - if ((uuid = (char *) switch_core_hash_find(profile->chat_hash, sip->sip_call_id->i_id))) { + if ((tmp = (char *) switch_core_hash_find(profile->chat_hash, sip->sip_call_id->i_id))) { + uuid = strdup(tmp); switch_core_hash_delete(profile->chat_hash, sip->sip_call_id->i_id); } switch_mutex_unlock(profile->flag_mutex); diff --git a/src/mod/endpoints/mod_sofia/sofia_presence.c b/src/mod/endpoints/mod_sofia/sofia_presence.c index de3d360703..6e5d15b381 100644 --- a/src/mod/endpoints/mod_sofia/sofia_presence.c +++ b/src/mod/endpoints/mod_sofia/sofia_presence.c @@ -5000,7 +5000,7 @@ void sofia_presence_handle_sip_i_message(int status, abort(); } - if (sofia_test_pflag(profile, PFLAG_IN_DIALOG_CHAT) && (tech_pvt = (private_object_t *) switch_core_hash_find(profile->chat_hash, hash_key))) { + if (sofia_test_pflag(profile, PFLAG_IN_DIALOG_CHAT) && (tech_pvt = (private_object_t *) switch_core_hash_find_locked(profile->chat_hash, hash_key, profile->flag_mutex))) { switch_core_session_queue_event(tech_pvt->session, &event); } else { switch_core_chat_send(proto, event); diff --git a/src/switch_core_hash.c b/src/switch_core_hash.c index a81747bb9f..1655aac726 100644 --- a/src/switch_core_hash.c +++ b/src/switch_core_hash.c @@ -55,6 +55,15 @@ SWITCH_DECLARE(switch_status_t) switch_core_hash_destroy(switch_hash_t **hash) return SWITCH_STATUS_SUCCESS; } +SWITCH_DECLARE(switch_status_t) switch_core_hash_insert_auto_free(switch_hash_t *hash, const char *key, const void *data) +{ + int r = 0; + + r = switch_hashtable_insert_destructor(hash, strdup(key), (void *)data, HASHTABLE_FLAG_FREE_KEY | HASHTABLE_FLAG_FREE_VALUE | HASHTABLE_DUP_CHECK, NULL); + + return r ? SWITCH_STATUS_SUCCESS : SWITCH_STATUS_FALSE; +} + SWITCH_DECLARE(switch_status_t) switch_core_hash_insert_destructor(switch_hash_t *hash, const char *key, const void *data, hashtable_destructor_t destructor) { int r = 0;