[Asterisk-code-review] res pjsip rfc3326.c: Fix crash when channel goes away. (asterisk[master])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: res_pjsip_rfc3326.c: Fix crash when channel goes away.
......................................................................


res_pjsip_rfc3326.c: Fix crash when channel goes away.

If an authenticated incoming caller does not respond to our 200 OK INVITE
response with an ACK then PJSIP will hangup the call.  Unfortunately,
there is a chance that the session's channel will go away between one use
of the channel pointer and another when building the BYE request because
the BYE is being built by the monitor thread and not the call's serializer
thread.

* Added a check to ensure that the thread trying to add the Reason header
is the call's serializer thread.  This ensures that the channel will not
go away on us.

Change-Id: I866388d2b97ea2032eaae3f3ab3f1ca6cbd2df89
---
M res/res_pjsip_rfc3326.c
1 file changed, 13 insertions(+), 4 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved



diff --git a/res/res_pjsip_rfc3326.c b/res/res_pjsip_rfc3326.c
index 6eb27ff..d49a170 100644
--- a/res/res_pjsip_rfc3326.c
+++ b/res/res_pjsip_rfc3326.c
@@ -32,6 +32,7 @@
 #include "asterisk/res_pjsip_session.h"
 #include "asterisk/module.h"
 #include "asterisk/causes.h"
+#include "asterisk/threadpool.h"
 
 static void rfc3326_use_reason_header(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
@@ -101,9 +102,15 @@
 
 static void rfc3326_outgoing_request(struct ast_sip_session *session, struct pjsip_tx_data *tdata)
 {
-	if ((pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_bye_method) &&
-	     pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_cancel_method)) ||
-	    !session->channel) {
+	if ((pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_bye_method)
+			&& pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_cancel_method))
+		|| !session->channel
+		/*
+		 * The session->channel has been seen to go away on us between
+		 * checks so we must also be running under the call's serializer
+		 * thread.
+		 */
+		|| session->serializer != ast_threadpool_serializer_get_current()) {
 		return;
 	}
 
@@ -114,7 +121,9 @@
 {
 	struct pjsip_status_line status = tdata->msg->line.status;
 
-	if ((status.code < 300) || !session->channel) {
+	if (status.code < 300
+		|| !session->channel
+		|| session->serializer != ast_threadpool_serializer_get_current()) {
 		return;
 	}
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I866388d2b97ea2032eaae3f3ab3f1ca6cbd2df89
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>