autochan/mixmonitor/chanspy: Fix unsafe channel locking and references.

Dereferencing struct ast_autochan.chan without first calling
ast_autochan_channel_lock() is unsafe because the pointer could change at
any time due to a masquerade.  Unfortunately, ast_autochan_channel_lock()
itself uses struct ast_autochan.chan unsafely and can result in a deadlock
if the original channel happens to get destroyed after a masquerade in
addition to the pointer getting changed.

The problem is more likely to happen with v11 and earlier because
masquerades are used to optimize out local channels on those versions.
However, it could still happen on newer versions if the channel is
executing a dialplan application when the channel is transferred or
redirected.  In this situation a masquerade still must be used.

* Added a lock to struct ast_autochan to safely be able to use
ast_autochan.chan while trying to get the channel lock in
ast_autochan_channel_lock().  The locking order is the channel lock then
the autochan lock.  Locking in the other direction requires deadlock
avoidance.

* Fix unsafe ast_autochan.chan usages in app_mixmonitor.c.

* Fix unsafe ast_autochan.chan usages in app_chanspy.c.

* app_chanspy.c: Removed unused autochan parameter from next_channel().

ASTERISK-26867

Change-Id: Id29dd22bc0f369b44e23ca423d2f3657187cc592

1 files changed, 12 insertions, 4 deletions

diff --git a/main/autochan.c b/main/autochan.c
index c7e5c0042..68aeaf804 100644
--- a/main/autochan.c
+++ b/main/autochan.c
@@ -46,15 +46,18 @@ struct ast_autochan *ast_autochan_setup(struct ast_channel *chan)
 	if (!(autochan = ast_calloc(1, sizeof(*autochan)))) {
 		return NULL;
 	}
+	ast_mutex_init(&autochan->lock);
 
 	autochan->chan = ast_channel_ref(chan);
 
-	ast_channel_lock(autochan->chan); /* autochan is still private, no need for ast_autochan_channel_lock() */
+	ast_debug(1, "Created autochan %p to hold channel %s (%p)\n",
+		autochan, ast_channel_name(chan), chan);
+
+	/* autochan is still private, no need for ast_autochan_channel_lock() */
+	ast_channel_lock(autochan->chan);
 	AST_LIST_INSERT_TAIL(ast_channel_autochans(autochan->chan), autochan, list);
 	ast_channel_unlock(autochan->chan);
 
-	ast_debug(1, "Created autochan %p to hold channel %s (%p)\n", autochan, ast_channel_name(chan), chan);
-
 	return autochan;
 }
 
@@ -75,6 +78,8 @@ void ast_autochan_destroy(struct ast_autochan *autochan)
 
 	autochan->chan = ast_channel_unref(autochan->chan);
 
+	ast_mutex_destroy(&autochan->lock);
+
 	ast_free(autochan);
 }
 
@@ -84,13 +89,16 @@ void ast_autochan_new_channel(struct ast_channel *old_chan, struct ast_channel *
 
 	AST_LIST_APPEND_LIST(ast_channel_autochans(new_chan), ast_channel_autochans(old_chan), list);
 
+	/* Deadlock avoidance is not needed since the channels are already locked. */
 	AST_LIST_TRAVERSE(ast_channel_autochans(new_chan), autochan, list) {
+		ast_mutex_lock(&autochan->lock);
 		if (autochan->chan == old_chan) {
-			autochan->chan = ast_channel_unref(old_chan);
 			autochan->chan = ast_channel_ref(new_chan);
+			ast_channel_unref(old_chan);
 
 			ast_debug(1, "Autochan %p used to hold channel %s (%p) but now holds channel %s (%p)\n",
 					autochan, ast_channel_name(old_chan), old_chan, ast_channel_name(new_chan), new_chan);
 		}
+		ast_mutex_unlock(&autochan->lock);
 	}
 }