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

[Richard Mudgett]

Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/8375


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(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/75/8375/1

diff --git a/res/res_pjsip_refer.c b/res/res_pjsip_refer.c
index 8e179fc..964c231 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/8375
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iacda724e7cb24d7520e49b2fd7e504aa398d7238
Gerrit-Change-Number: 8375
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180222/00057eb2/attachment.html>