[Asterisk-code-review] chan pjsip.c: Fix deadlock potential in fax redirection. (asterisk[14])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review.

  https://gerrit.asterisk.org/3297

Change subject: chan_pjsip.c: Fix deadlock potential in fax redirection.
......................................................................

chan_pjsip.c: Fix deadlock potential in fax redirection.

The chan_pjsip_cng_tone_detected() has the potential to deadlock if an
incoming fax happens during the Playback or similar application.

* Fixed the potential deadlock by not calling ast_async_goto() with the
channel lock held.

* Made always eat the fax detection frame whether there is a fax extension
or not.

ASTERISK-26216
Reported by: Richard Mudgett

Change-Id: I32aecbb4818af646dc5a619f0dc040e9b1f222e5
---
M channels/chan_pjsip.c
1 file changed, 7 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/3297/1

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 5ad1174..a0ad424 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -628,16 +628,19 @@
 
 	target_context = S_OR(ast_channel_macrocontext(session->channel), ast_channel_context(session->channel));
 
-	/* We need to unlock the channel here because ast_exists_extension has the
+	/*
+	 * We need to unlock the channel here because ast_exists_extension has the
 	 * potential to start and stop an autoservice on the channel. Such action
 	 * is prone to deadlock if the channel is locked.
+	 *
+	 * ast_async_goto() has its own restriction on not holding the channel lock.
 	 */
 	ast_channel_unlock(session->channel);
+	ast_frfree(f);
+	f = &ast_null_frame;
 	exists = ast_exists_extension(session->channel, target_context, "fax", 1,
 		S_COR(ast_channel_caller(session->channel)->id.number.valid,
 			ast_channel_caller(session->channel)->id.number.str, NULL));
-	ast_channel_lock(session->channel);
-
 	if (exists) {
 		ast_verb(2, "Redirecting '%s' to fax extension due to CNG detection\n",
 			ast_channel_name(session->channel));
@@ -646,12 +649,11 @@
 			ast_log(LOG_ERROR, "Failed to async goto '%s' into fax extension in '%s'\n",
 				ast_channel_name(session->channel), target_context);
 		}
-		ast_frfree(f);
-		f = &ast_null_frame;
 	} else {
 		ast_log(LOG_NOTICE, "FAX CNG detected on '%s' but no fax extension in '%s'\n",
 			ast_channel_name(session->channel), target_context);
 	}
+	ast_channel_lock(session->channel);
 
 	return f;
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32aecbb4818af646dc5a619f0dc040e9b1f222e5
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>