From 2758cc76e553e89445b5b16b8f7083b44bf98f60 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Mon, 28 Jul 2014 18:58:43 +0000 Subject: datastores: Audit ast_channel_datastore_remove usage. Audit of v1.8 usage of ast_channel_datastore_remove() for datastore memory leaks. * Fixed leaks in app_speech_utils and func_frame_trace. * Fixed app_speech_utils not locking the channel when accessing the channel datastore list. Review: https://reviewboard.asterisk.org/r/3859/ Audit of v11 usage of ast_channel_datastore_remove() for datastore memory leaks. * Fixed leak in func_jitterbuffer. (Was not in v12) Review: https://reviewboard.asterisk.org/r/3860/ Audit of v12 usage of ast_channel_datastore_remove() for datastore memory leaks. * Fixed leaks in abstract_jb. * Fixed leak in ast_channel_unsuppress(). Used by ARI mute control and res_mutestream. * Fixed ref leak in ast_channel_suppress(). Used by ARI mute control and res_mutestream. Review: https://reviewboard.asterisk.org/r/3861/ ........ Merged revisions 419684 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 419685 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 419686 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@419688 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- apps/app_speech_utils.c | 56 +++++++++++++++++++++++++++++++----------------- funcs/func_frame_trace.c | 1 + main/abstract_jb.c | 4 ++-- main/channel.c | 9 +++----- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/apps/app_speech_utils.c b/apps/app_speech_utils.c index 4706d5350..d94603837 100644 --- a/apps/app_speech_utils.c +++ b/apps/app_speech_utils.c @@ -290,7 +290,9 @@ static struct ast_speech *find_speech(struct ast_channel *chan) return NULL; } + ast_channel_lock(chan); datastore = ast_channel_datastore_find(chan, &speech_datastore, NULL); + ast_channel_unlock(chan); if (datastore == NULL) { return NULL; } @@ -299,6 +301,35 @@ static struct ast_speech *find_speech(struct ast_channel *chan) return speech; } +/*! + * \internal + * \brief Destroy the speech datastore on the given channel. + * + * \param chan Channel to destroy speech datastore. + * + * \retval 0 on success. + * \retval -1 not found. + */ +static int speech_datastore_destroy(struct ast_channel *chan) +{ + struct ast_datastore *datastore; + int res; + + ast_channel_lock(chan); + datastore = ast_channel_datastore_find(chan, &speech_datastore, NULL); + if (datastore) { + ast_channel_datastore_remove(chan, datastore); + } + ast_channel_unlock(chan); + if (datastore) { + ast_datastore_free(datastore); + res = 0; + } else { + res = -1; + } + return res; +} + /* Helper function to find a specific speech recognition result by number and nbest alternative */ static struct ast_speech_result *find_result(struct ast_speech_result *results, char *result_num) { @@ -532,7 +563,9 @@ static int speech_create(struct ast_channel *chan, const char *data) } pbx_builtin_setvar_helper(chan, "ERROR", NULL); datastore->data = speech; + ast_channel_lock(chan); ast_channel_datastore_add(chan, datastore); + ast_channel_unlock(chan); return 0; } @@ -675,7 +708,6 @@ static int speech_background(struct ast_channel *chan, const char *data) RAII_VAR(struct ast_format *, oldreadformat, NULL, ao2_cleanup); char dtmf[AST_MAX_EXTENSION] = ""; struct timeval start = { 0, 0 }, current; - struct ast_datastore *datastore = NULL; char *parse, *filename_tmp = NULL, *filename = NULL, tmp[2] = "", dtmf_terminator = '#'; const char *tmp2 = NULL; struct ast_flags options = { 0 }; @@ -904,11 +936,7 @@ static int speech_background(struct ast_channel *chan, const char *data) /* See if it was because they hung up */ if (done == 3) { - /* Destroy speech structure */ - ast_speech_destroy(speech); - datastore = ast_channel_datastore_find(chan, &speech_datastore, NULL); - if (datastore != NULL) - ast_channel_datastore_remove(chan, datastore); + speech_datastore_destroy(chan); } else { /* Channel is okay so restore read format */ ast_set_read_format(chan, oldreadformat); @@ -921,22 +949,10 @@ static int speech_background(struct ast_channel *chan, const char *data) /*! \brief SpeechDestroy() Dialplan Application */ static int speech_destroy(struct ast_channel *chan, const char *data) { - int res = 0; - struct ast_speech *speech = find_speech(chan); - struct ast_datastore *datastore = NULL; - - if (speech == NULL) + if (!chan) { return -1; - - /* Destroy speech structure */ - ast_speech_destroy(speech); - - datastore = ast_channel_datastore_find(chan, &speech_datastore, NULL); - if (datastore != NULL) { - ast_channel_datastore_remove(chan, datastore); } - - return res; + return speech_datastore_destroy(chan); } static int unload_module(void) diff --git a/funcs/func_frame_trace.c b/funcs/func_frame_trace.c index dea21c252..3ad048d28 100644 --- a/funcs/func_frame_trace.c +++ b/funcs/func_frame_trace.c @@ -185,6 +185,7 @@ static int frame_trace_helper(struct ast_channel *chan, const char *cmd, char *d id = datastore->data; ast_framehook_detach(chan, *id); ast_channel_datastore_remove(chan, datastore); + ast_datastore_free(datastore); } if (!(datastore = ast_datastore_alloc(&frame_trace_datastore, NULL))) { diff --git a/main/abstract_jb.c b/main/abstract_jb.c index 85c188e88..e4c3c76ed 100644 --- a/main/abstract_jb.c +++ b/main/abstract_jb.c @@ -1055,6 +1055,7 @@ void ast_jb_create_framehook(struct ast_channel *chan, struct ast_jb_conf *jb_co id = datastore->data; ast_framehook_detach(chan, *id); ast_channel_datastore_remove(chan, datastore); + ast_datastore_free(datastore); } ast_channel_unlock(chan); return; @@ -1087,6 +1088,7 @@ void ast_jb_create_framehook(struct ast_channel *chan, struct ast_jb_conf *jb_co id = datastore->data; ast_framehook_detach(chan, *id); ast_channel_datastore_remove(chan, datastore); + ast_datastore_free(datastore); } if (!(datastore = ast_datastore_alloc(&jb_datastore, NULL))) { @@ -1112,6 +1114,4 @@ void ast_jb_create_framehook(struct ast_channel *chan, struct ast_jb_conf *jb_co framedata = NULL; } ast_channel_unlock(chan); - - return; } diff --git a/main/channel.c b/main/channel.c index 4ce4e916f..6a252a699 100644 --- a/main/channel.c +++ b/main/channel.c @@ -10415,10 +10415,7 @@ int ast_channel_suppress(struct ast_channel *chan, unsigned int direction, enum if ((datastore = ast_channel_datastore_find(chan, datastore_info, NULL))) { suppress = datastore->data; - ao2_ref(suppress, +1); - suppress->direction |= direction; - return 0; } @@ -10450,13 +10447,12 @@ int ast_channel_suppress(struct ast_channel *chan, unsigned int direction, enum return -1; } + /* and another ref for the datastore */ + ao2_ref(suppress, +1); datastore->data = suppress; ast_channel_datastore_add(chan, datastore); - /* and another ref for the datastore */ - ao2_ref(suppress, +1); - return 0; } @@ -10484,6 +10480,7 @@ int ast_channel_unsuppress(struct ast_channel *chan, unsigned int direction, enu /* Nothing left to suppress. Bye! */ ast_framehook_detach(chan, suppress->framehook_id); ast_channel_datastore_remove(chan, datastore); + ast_datastore_free(datastore); } return 0; -- cgit v1.2.3