[Asterisk-code-review] chan_pjsip: Incorporate channel reference count into transfer_refer(). (asterisk[18])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15196 )

Change subject: chan_pjsip: Incorporate channel reference count into transfer_refer().
......................................................................

chan_pjsip: Incorporate channel reference count into transfer_refer().

Add channel reference count for PJSIP REFER. The call could be terminated
prior to the result of the transfer. In that scenario, when the SUBSCRIBE/NOTIFY
occurred several minutes later, it would attempt to access a session which was
no longer valid.  Terminate event subscription if pjsip_xfer_initiate() or
pjsip_xfer_send_request() fails in transfer_refer().

ASTERISK-29201 #close
Reported-by: Dan Cropp

Change-Id: I3fd92fd14b4e3844d3d7b0f60fe417a4df5f2435
---
M channels/chan_pjsip.c
1 file changed, 26 insertions(+), 18 deletions(-)

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



diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index b6944db..bb04d73 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -1922,8 +1922,7 @@
  */
 static void xfer_client_on_evsub_state(pjsip_evsub *sub, pjsip_event *event)
 {
-	struct ast_sip_session *session;
-	struct ast_channel *chan = NULL;
+	struct ast_channel *chan;
 	enum ast_control_transfer message = AST_TRANSFER_SUCCESS;
 	int res = 0;
 
@@ -1931,12 +1930,7 @@
 		return;
 	}
 
-	session = pjsip_evsub_get_mod_data(sub, refer_callback_module.id);
-	if (!session) {
-		return;
-	}
-
-	chan = session->channel;
+	chan = pjsip_evsub_get_mod_data(sub, refer_callback_module.id);
 	if (!chan) {
 		return;
 	}
@@ -1962,7 +1956,9 @@
 			 */
 			if (refer_sub && !pj_stricmp2(&refer_sub->hvalue, "false")) {
 				/* Since no subscription is desired, assume that call has been transferred successfully. */
+				/* Channel reference will be released at end of function */
 				/* Terminate subscription. */
+				pjsip_evsub_set_mod_data(sub, refer_callback_module.id, NULL);
 				pjsip_evsub_terminate(sub, PJ_TRUE);
 				res = -1;
 			}
@@ -2027,7 +2023,8 @@
 	}
 
 	if (res) {
-		ast_queue_control_data(session->channel, AST_CONTROL_TRANSFER, &message, sizeof(message));
+		ast_queue_control_data(chan, AST_CONTROL_TRANSFER, &message, sizeof(message));
+		ao2_ref(chan, -1);
 	}
 }
 
@@ -2040,28 +2037,29 @@
 	const char *ref_by_val;
 	char local_info[pj_strlen(&session->inv_session->dlg->local.info_str) + 1];
 	struct pjsip_evsub_user xfer_cb;
+	struct ast_channel *chan = session->channel;
 
 	pj_bzero(&xfer_cb, sizeof(xfer_cb));
 	xfer_cb.on_evsub_state = &xfer_client_on_evsub_state;
 
 	if (pjsip_xfer_create_uac(session->inv_session->dlg, &xfer_cb, &sub) != PJ_SUCCESS) {
 		message = AST_TRANSFER_FAILED;
-		ast_queue_control_data(session->channel, AST_CONTROL_TRANSFER, &message, sizeof(message));
+		ast_queue_control_data(chan, AST_CONTROL_TRANSFER, &message, sizeof(message));
 
 		return;
 	}
 
-	pjsip_evsub_set_mod_data(sub, refer_callback_module.id, session);
+	/* refer_callback_module requires a reference to chan
+	 * which will be released in xfer_client_on_evsub_state()
+	 * when the implicit REFER subscription terminates */
+	pjsip_evsub_set_mod_data(sub, refer_callback_module.id, chan);
+	ao2_ref(chan, +1);
 
 	if (pjsip_xfer_initiate(sub, pj_cstr(&tmp, target), &packet) != PJ_SUCCESS) {
-		message = AST_TRANSFER_FAILED;
-		ast_queue_control_data(session->channel, AST_CONTROL_TRANSFER, &message, sizeof(message));
-		pjsip_evsub_terminate(sub, PJ_FALSE);
-
-		return;
+		goto failure;
 	}
 
-	ref_by_val = pbx_builtin_getvar_helper(session->channel, "SIPREFERREDBYHDR");
+	ref_by_val = pbx_builtin_getvar_helper(chan, "SIPREFERREDBYHDR");
 	if (!ast_strlen_zero(ref_by_val)) {
 		ast_sip_add_header(packet, "Referred-By", ref_by_val);
 	} else {
@@ -2069,7 +2067,17 @@
 		ast_sip_add_header(packet, "Referred-By", local_info);
 	}
 
-	pjsip_xfer_send_request(sub, packet);
+	if (pjsip_xfer_send_request(sub, packet) == PJ_SUCCESS) {
+		return;
+	}
+
+failure:
+	message = AST_TRANSFER_FAILED;
+	ast_queue_control_data(chan, AST_CONTROL_TRANSFER, &message, sizeof(message));
+	pjsip_evsub_set_mod_data(sub, refer_callback_module.id, NULL);
+	pjsip_evsub_terminate(sub, PJ_FALSE);
+
+	ao2_ref(chan, -1);
 }
 
 static int transfer(void *data)

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15196
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I3fd92fd14b4e3844d3d7b0f60fe417a4df5f2435
Gerrit-Change-Number: 15196
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Cropp <dan at amtelco.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210106/db5ba4f8/attachment-0001.html>