From 0e1ba9a7783ea014a391ff26b93cba5e902a0e29 Mon Sep 17 00:00:00 2001 From: Kevin Harwell Date: Wed, 23 Dec 2020 13:06:19 -0600 Subject: [PATCH] app_mixmonitor: cleanup datastore when monitor thread fails to launch launch_monitor_thread is responsible for creating and initializing the mixmonitor, and dependent data structures. There was one off nominal path after the datastore gets created that triggers when the channel being monitored is hung up prior to monitor starting itself. If this happened the monitor thread would not "launch", and the mixmonitor object and associated objects are freed, including the underlying datastore data object. However, the datastore itself was not removed from the channel, so when the channel eventually gets destroyed it tries to access the previously freed datastore data and crashes. This patch removes and frees datastore object itself from the channel before freeing the mixmonitor object thus ensuring the channel does not call it when destroyed. ASTERISK-28947 #close Change-Id: Id4f9e958956d62473ed5ff06c98ae3436e839ff8 --- apps/app_mixmonitor.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c index bb1bc51d0c..239901f0ac 100644 --- a/apps/app_mixmonitor.c +++ b/apps/app_mixmonitor.c @@ -865,6 +865,24 @@ static int setup_mixmonitor_ds(struct mixmonitor *mixmonitor, struct ast_channel return 0; } +static void mixmonitor_ds_remove_and_free(struct ast_channel *chan, const char *datastore_id) +{ + struct ast_datastore *datastore; + + ast_channel_lock(chan); + + datastore = ast_channel_datastore_find(chan, &mixmonitor_ds_info, datastore_id); + + /* + * Currently the one place this function is called from guarantees a + * datastore is present, thus return checks can be avoided here. + */ + ast_channel_datastore_remove(chan, datastore); + ast_datastore_free(datastore); + + ast_channel_unlock(chan); +} + static int launch_monitor_thread(struct ast_channel *chan, const char *filename, unsigned int flags, int readvol, int writevol, const char *post_process, const char *filename_write, @@ -940,7 +958,6 @@ static int launch_monitor_thread(struct ast_channel *chan, const char *filename, pbx_builtin_setvar_helper(chan, uid_channel_var, datastore_id); } } - ast_free(datastore_id); mixmonitor->name = ast_strdup(ast_channel_name(chan)); @@ -990,12 +1007,16 @@ static int launch_monitor_thread(struct ast_channel *chan, const char *filename, if (startmon(chan, &mixmonitor->audiohook)) { ast_log(LOG_WARNING, "Unable to add '%s' spy to channel '%s'\n", mixmonitor_spy_type, ast_channel_name(chan)); + mixmonitor_ds_remove_and_free(chan, datastore_id); + ast_free(datastore_id); ast_autochan_destroy(mixmonitor->autochan); ast_audiohook_destroy(&mixmonitor->audiohook); mixmonitor_free(mixmonitor); return -1; } + ast_free(datastore_id); + /* reference be released at mixmonitor destruction */ mixmonitor->callid = ast_read_threadstorage_callid();