[Asterisk-code-review] res pjsip refer.c: Fix attended transfer race condition crash. (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8377 )

Change subject: res_pjsip_refer.c: Fix attended transfer race condition crash.
......................................................................

res_pjsip_refer.c: Fix attended transfer race condition crash.

The transferrer's session channel was destroyed by the transferrer's
serializer thread in a race condition with the transfer target's
serializer thread during an attended transfer.  The transfer target's
serializer was attempting to clean up a deferred end status on behalf of
the transferrer's channel when it should have passed the action to the
transferrer's serializer.  When the transfer target's serializer lost the
race then both threads wind up trying to end the transferrer's session.

* Push the ast_sip_session_end_if_deferred() call onto the transferrer's
serializer to avoid a race condition that results in a crash.  The
session_end() function that could be called by
ast_sip_session_end_if_deferred() really must be executed by the
transferrer's serializer to avoid this kind of crash.

ASTERISK-27568

Change-Id: Iacda724e7cb24d7520e49b2fd7e504aa398d7238
---
M res/res_pjsip_refer.c
1 file changed, 24 insertions(+), 8 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/res/res_pjsip_refer.c b/res/res_pjsip_refer.c
index 5e0141b..7d892f6 100644
--- a/res/res_pjsip_refer.c
+++ b/res/res_pjsip_refer.c
@@ -468,10 +468,20 @@
 	return attended;
 }
 
-static int defer_termination_cancel(void *data)
+static int session_end_if_deferred_task(void *data)
 {
 	struct ast_sip_session *session = data;
 
+	ast_sip_session_end_if_deferred(session);
+	ao2_ref(session, -1);
+	return 0;
+}
+
+static int defer_termination_cancel_task(void *data)
+{
+	struct ast_sip_session *session = data;
+
+	ast_sip_session_end_if_deferred(session);
 	ast_sip_session_defer_termination_cancel(session);
 	ao2_ref(session, -1);
 	return 0;
@@ -513,6 +523,7 @@
 {
 	struct refer_attended *attended = data;
 	int response;
+	int (*task_cb)(void *data);
 
 	if (attended->transferer_second->channel) {
 		ast_debug(3, "Performing a REFER attended transfer - Transferer #1: %s Transferer #2: %s\n",
@@ -543,13 +554,18 @@
 		}
 	}
 
-	ast_sip_session_end_if_deferred(attended->transferer);
-	if (response != 200) {
-		if (!ast_sip_push_task(attended->transferer->serializer,
-			defer_termination_cancel, attended->transferer)) {
-			/* Gave the ref to the pushed task. */
-			attended->transferer = NULL;
-		}
+	if (response == 200) {
+		task_cb = session_end_if_deferred_task;
+	} else {
+		task_cb = defer_termination_cancel_task;
+	}
+	if (!ast_sip_push_task(attended->transferer->serializer,
+		task_cb, attended->transferer)) {
+		/* Gave the ref to the pushed task. */
+		attended->transferer = NULL;
+	} else {
+		/* Do this anyway even though it is the wrong serializer. */
+		ast_sip_session_end_if_deferred(attended->transferer);
 	}
 
 	ao2_ref(attended, -1);

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iacda724e7cb24d7520e49b2fd7e504aa398d7238
Gerrit-Change-Number: 8377
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180301/db2ce161/attachment.html>