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

diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index bb1bc51d0c8338428aaf8afbda4f79907b13a263..239901f0acb733f2ecd8f599952c0dfb22d2d3ad 100644 (file)
--- 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();