chan_pjsip: transfers with direct media reinvite has wrong address/port

During a transfer involving direct media a race occurs between when the
transferer channel is swapped out, initiating rtp changes/updates, and the
subsequent reinvites.

When Alice, after speaking with Charlie (Bob is on hold), connects Bob and
Charlie invites are sent to each in order to establish the call between them.
Bob is taken off hold and Charlie is told to have his media flow through
Asterisk. However, if before those invites go out the bridge updates Bob's
and/or Charlie's rtp information with direct media data (i.e. address, port)
then the invite(s) will contain the remote data in the SDP instead of the
Asterisk data.

The race occurs in the native bridge glue code when updating the peer. The
direct_media_address can get set twice before sending out the first invite
during call connection. This can happen because the checking/setting of the
direct_media_address happened in one thread while the sending of the invite(s)
happened in another thread.

This fix removes the race condition by moving the checking/setting of the
direct_media_address to be in the same thread as the sending of the invites(s).
This serializes the checking/setting and sending so they can no longer happen
out of order.

ASTERISK-25849 #close

Change-Id: Idfea590175e74f401929a601dba0c91ca1a7f873

1 files changed, 83 insertions, 35 deletions

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 32291e54c..729f453f9 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -223,17 +223,6 @@ static void chan_pjsip_get_codec(struct ast_channel *chan, struct ast_format_cap
 	ast_format_cap_append_from_cap(result, channel->session->endpoint->media.codecs, AST_MEDIA_TYPE_UNKNOWN);
 }
 
-static int send_direct_media_request(void *data)
-{
-	struct ast_sip_session *session = data;
-	int res;
-
-	res = ast_sip_session_refresh(session, NULL, NULL, NULL,
-		session->endpoint->media.direct_media.method, 1);
-	ao2_ref(session, -1);
-	return res;
-}
-
 /*! \brief Destructor function for \ref transport_info_data */
 static void transport_info_destroy(void *obj)
 {
@@ -302,6 +291,83 @@ static int check_for_rtp_changes(struct ast_channel *chan, struct ast_rtp_instan
 	return changed;
 }
 
+struct rtp_direct_media_data {
+	struct ast_channel *chan;
+	struct ast_rtp_instance *rtp;
+	struct ast_rtp_instance *vrtp;
+	struct ast_format_cap *cap;
+	struct ast_sip_session *session;
+};
+
+static void rtp_direct_media_data_destroy(void *data)
+{
+	struct rtp_direct_media_data *cdata = data;
+
+	ao2_cleanup(cdata->session);
+	ao2_cleanup(cdata->cap);
+	ao2_cleanup(cdata->vrtp);
+	ao2_cleanup(cdata->rtp);
+	ao2_cleanup(cdata->chan);
+}
+
+static struct rtp_direct_media_data *rtp_direct_media_data_create(
+	struct ast_channel *chan, struct ast_rtp_instance *rtp, struct ast_rtp_instance *vrtp,
+	const struct ast_format_cap *cap, struct ast_sip_session *session)
+{
+	struct rtp_direct_media_data *cdata = ao2_alloc(sizeof(*cdata), rtp_direct_media_data_destroy);
+
+	if (!cdata) {
+		return NULL;
+	}
+
+	cdata->chan = ao2_bump(chan);
+	cdata->rtp = ao2_bump(rtp);
+	cdata->vrtp = ao2_bump(vrtp);
+	cdata->cap = ao2_bump((struct ast_format_cap *)cap);
+	cdata->session = ao2_bump(session);
+
+	return cdata;
+}
+
+static int send_direct_media_request(void *data)
+{
+	struct rtp_direct_media_data *cdata = data;
+	struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(cdata->chan);
+	struct chan_pjsip_pvt *pvt = channel->pvt;
+	int changed = 0;
+	int res = 0;
+
+	if (pvt->media[SIP_MEDIA_AUDIO]) {
+		changed |= check_for_rtp_changes(
+			cdata->chan, cdata->rtp, pvt->media[SIP_MEDIA_AUDIO], 1);
+	}
+	if (pvt->media[SIP_MEDIA_VIDEO]) {
+		changed |= check_for_rtp_changes(
+			cdata->chan, cdata->vrtp, pvt->media[SIP_MEDIA_VIDEO], 3);
+	}
+
+	if (direct_media_mitigate_glare(cdata->session)) {
+		ast_debug(4, "Disregarding setting RTP on %s: mitigating re-INVITE glare\n", ast_channel_name(cdata->chan));
+		return 0;
+	}
+
+	if (cdata->cap && ast_format_cap_count(cdata->cap) &&
+	    !ast_format_cap_identical(cdata->session->direct_media_cap, cdata->cap)) {
+		ast_format_cap_remove_by_type(cdata->session->direct_media_cap, AST_MEDIA_TYPE_UNKNOWN);
+		ast_format_cap_append_from_cap(cdata->session->direct_media_cap, cdata->cap, AST_MEDIA_TYPE_UNKNOWN);
+		changed = 1;
+	}
+
+	if (changed) {
+		ast_debug(4, "RTP changed on %s; initiating direct media update\n", ast_channel_name(cdata->chan));
+		res = ast_sip_session_refresh(cdata->session, NULL, NULL, NULL,
+			cdata->session->endpoint->media.direct_media.method, 1);
+	}
+
+	ao2_ref(cdata, -1);
+	return res;
+}
+
 /*! \brief Function called by RTP engine to change where the remote party should send media */
 static int chan_pjsip_set_rtp_peer(struct ast_channel *chan,
 		struct ast_rtp_instance *rtp,
@@ -311,9 +377,8 @@ static int chan_pjsip_set_rtp_peer(struct ast_channel *chan,
 		int nat_active)
 {
 	struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(chan);
-	struct chan_pjsip_pvt *pvt = channel->pvt;
 	struct ast_sip_session *session = channel->session;
-	int changed = 0;
+	struct rtp_direct_media_data *cdata;
 
 	/* Don't try to do any direct media shenanigans on early bridges */
 	if ((rtp || vrtp || tpeer) && !ast_channel_is_bridged(chan)) {
@@ -326,31 +391,14 @@ static int chan_pjsip_set_rtp_peer(struct ast_channel *chan,
 		return 0;
 	}
 
-	if (pvt->media[SIP_MEDIA_AUDIO]) {
-		changed |= check_for_rtp_changes(chan, rtp, pvt->media[SIP_MEDIA_AUDIO], 1);
-	}
-	if (pvt->media[SIP_MEDIA_VIDEO]) {
-		changed |= check_for_rtp_changes(chan, vrtp, pvt->media[SIP_MEDIA_VIDEO], 3);
-	}
-
-	if (direct_media_mitigate_glare(session)) {
-		ast_debug(4, "Disregarding setting RTP on %s: mitigating re-INVITE glare\n", ast_channel_name(chan));
+	cdata = rtp_direct_media_data_create(chan, rtp, vrtp, cap, session);
+	if (!cdata) {
 		return 0;
 	}
 
-	if (cap && ast_format_cap_count(cap) && !ast_format_cap_identical(session->direct_media_cap, cap)) {
-		ast_format_cap_remove_by_type(session->direct_media_cap, AST_MEDIA_TYPE_UNKNOWN);
-		ast_format_cap_append_from_cap(session->direct_media_cap, cap, AST_MEDIA_TYPE_UNKNOWN);
-		changed = 1;
-	}
-
-	if (changed) {
-		ao2_ref(session, +1);
-
-		ast_debug(4, "RTP changed on %s; initiating direct media update\n", ast_channel_name(chan));
-		if (ast_sip_push_task(session->serializer, send_direct_media_request, session)) {
-			ao2_cleanup(session);
-		}
+	if (ast_sip_push_task(session->serializer, send_direct_media_request, cdata)) {
+		ast_log(LOG_ERROR, "Unable to send direct media request for channel %s\n", ast_channel_name(chan));
+		ao2_ref(cdata, -1);
 	}
 
 	return 0;