[Asterisk-code-review] Don't crash on authenticated reinvite after originated T.38 ... (asterisk[certified/13.1])

[Mark Michelson]

Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/238

Change subject: Don't crash on authenticated reinvite after originated T.38 FAX.
......................................................................

Don't crash on authenticated reinvite after originated T.38 FAX.

When Asterisk originates a channel to an application, the channel is
hung up once the application finishes executing. When the application
in question is SendFax, the Asterisk PJSIP code will attempt to reinvite
the T.38 session to audio after the FAX completes. The hangup of the
channel happens in the midst of this reinvite transaction. In most
circumstances, this works out okay because the BYE is delayed until the
reinvite transaction can complete.

However, if the reinvite that Asterisk sends receives a 401/407
response, then Asterisk's attempt to re-send the reinvite with
authentication will fail. This is because the session supplement in
res_pjsip_t38 makes the assumption that the channel on the session will
always be non-NULL. Since the channel has been hung up, though, the
channel is now NULL. Attempting to operate on the channel causes a
crash.

This patch fixes the issue by ensuring that the channel on the session
is not NULL before attempting to mess with the T.38 framehook.

This patch also contains some corrections for comments that were
incorrect and really confused me when I first started looking at the
code.

ASTERISK-25004 #close
Reported by Mark Michelson

Change-Id: Ic5a1230668369dda4bb13524098aed9306ab45a0
---
M res/res_pjsip_t38.c
1 file changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/38/238/1

diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index d4b384f..85d0ba8 100644
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -467,6 +467,11 @@
 		.chan_breakdown_cb = t38_masq,
 	};
 
+	/* If the channel's already gone, bail */
+	if (!session->channel) {
+		return;
+	}
+
 	/* Only attach the framehook if t38 is enabled for the endpoint */
 	if (!session->endpoint->media.t38.enabled) {
 		return;
@@ -501,14 +506,14 @@
 	ast_channel_unlock(session->channel);
 }
 
-/*! \brief Function called when an INVITE goes out */
+/*! \brief Function called when an INVITE arrives */
 static int t38_incoming_invite_request(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
 	t38_attach_framehook(session);
 	return 0;
 }
 
-/*! \brief Function called when an INVITE comes in */
+/*! \brief Function called when an INVITE is sent */
 static void t38_outgoing_invite_request(struct ast_sip_session *session, struct pjsip_tx_data *tdata)
 {
 	t38_attach_framehook(session);

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic5a1230668369dda4bb13524098aed9306ab45a0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>