[Asterisk-code-review] bridge native rtp.c: Fix native rtp bridge data race. (asterisk[14])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review. ( https://gerrit.asterisk.org/4677 )

Change subject: bridge_native_rtp.c: Fix native rtp bridge data race.
......................................................................

bridge_native_rtp.c: Fix native rtp bridge data race.

native_rtp_bridge_compatible() didn't lock the bridge channels before
checking the channels for native bridging ability.  As a result, one of
the channel's native format capabilities structure got replaced out from
under the native bridge check.  Use of a stale pointer to freed memory
causes bad things to happen.

MALLOC_DEBUG, DO_CRASH, and the
tests/channels/pjsip/transfers/blind_transfer/caller_direct_media
testsuite test caught this.

* Add missing channel locking in native_rtp_bridge_compatible().

Change-Id: If25fdb3ac8e85563c4857fb8216b3d9dc3d0fa53
---
M bridges/bridge_native_rtp.c
1 file changed, 25 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/77/4677/1

diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c
index 6c7c1cd..6f9b8e4 100644
--- a/bridges/bridge_native_rtp.c
+++ b/bridges/bridge_native_rtp.c
@@ -312,10 +312,8 @@
 	return !ast_channel_has_hook_requiring_audio(chan);
 }
 
-static int native_rtp_bridge_compatible(struct ast_bridge *bridge)
+static int native_rtp_bridge_compatible_check(struct ast_bridge *bridge, struct ast_bridge_channel *bc0, struct ast_bridge_channel *bc1)
 {
-	struct ast_bridge_channel *bc0 = AST_LIST_FIRST(&bridge->channels);
-	struct ast_bridge_channel *bc1 = AST_LIST_LAST(&bridge->channels);
 	enum ast_rtp_glue_result native_type;
 	struct ast_rtp_glue *glue0, *glue1;
 	RAII_VAR(struct ast_rtp_instance *, instance0, NULL, ao2_cleanup);
@@ -325,13 +323,6 @@
 	RAII_VAR(struct ast_format_cap *, cap0, NULL, ao2_cleanup);
 	RAII_VAR(struct ast_format_cap *, cap1, NULL, ao2_cleanup);
 	int read_ptime0, read_ptime1, write_ptime0, write_ptime1;
-
-	/* We require two channels before even considering native bridging */
-	if (bridge->num_channels != 2) {
-		ast_debug(1, "Bridge '%s' can not use native RTP bridge as two channels are required\n",
-			bridge->uniqueid);
-		return 0;
-	}
 
 	if (!native_rtp_bridge_capable(bc0->chan)) {
 		ast_debug(1, "Bridge '%s' can not use native RTP bridge as channel '%s' has features which prevent it\n",
@@ -408,6 +399,30 @@
 	return 1;
 }
 
+static int native_rtp_bridge_compatible(struct ast_bridge *bridge)
+{
+	struct ast_bridge_channel *bc0;
+	struct ast_bridge_channel *bc1;
+	int is_compatible;
+
+	/* We require two channels before even considering native bridging */
+	if (bridge->num_channels != 2) {
+		ast_debug(1, "Bridge '%s' can not use native RTP bridge as two channels are required\n",
+			bridge->uniqueid);
+		return 0;
+	}
+
+	bc0 = AST_LIST_FIRST(&bridge->channels);
+	bc1 = AST_LIST_LAST(&bridge->channels);
+
+	ast_channel_lock_both(bc0->chan, bc1->chan);
+	is_compatible = native_rtp_bridge_compatible_check(bridge, bc0, bc1);
+	ast_channel_unlock(bc0->chan);
+	ast_channel_unlock(bc1->chan);
+
+	return is_compatible;
+}
+
 /*! \brief Helper function which adds frame hook to bridge channel */
 static int native_rtp_bridge_framehook_attach(struct ast_bridge_channel *bridge_channel)
 {

-- 
To view, visit https://gerrit.asterisk.org/4677
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If25fdb3ac8e85563c4857fb8216b3d9dc3d0fa53
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>