[asterisk-commits] res fax.c: Fix deadlock potential in FAXOPT(faxdetect) frame... (asterisk[13])

[SVN commits to the Asterisk project]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: res_fax.c: Fix deadlock potential in FAXOPT(faxdetect) framehook.
......................................................................


res_fax.c: Fix deadlock potential in FAXOPT(faxdetect) framehook.

The fax_detect_framehook() 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.

* Made only detach the framehook if we detected a fax and not on other
possible frames.

ASTERISK-26216
Reported by: Richard Mudgett

Change-Id: I99da35c26d1cd802626ffb4c1b4eb5b015581b6d
---
M res/res_fax.c
1 file changed, 13 insertions(+), 7 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_fax.c b/res/res_fax.c
index ad6e238..ba4fc46 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -3701,30 +3701,36 @@
 	}
 
 	if (result) {
-		const char *target_context = S_OR(ast_channel_macrocontext(chan), ast_channel_context(chan));
+		const char *target_context;
+
 		switch (result) {
 		case 'f':
 		case 't':
+			target_context = S_OR(ast_channel_macrocontext(chan), ast_channel_context(chan));
+
 			ast_channel_unlock(chan);
+			ast_frfree(f);
+			f = &ast_null_frame;
 			if (ast_exists_extension(chan, target_context, "fax", 1,
 			    S_COR(ast_channel_caller(chan)->id.number.valid, ast_channel_caller(chan)->id.number.str, NULL))) {
-				ast_channel_lock(chan);
 				ast_verb(2, "Redirecting '%s' to fax extension due to %s detection\n",
 					ast_channel_name(chan), (result == 'f') ? "CNG" : "T38");
 				pbx_builtin_setvar_helper(chan, "FAXEXTEN", ast_channel_exten(chan));
 				if (ast_async_goto(chan, target_context, "fax", 1)) {
 					ast_log(LOG_NOTICE, "Failed to async goto '%s' into fax of '%s'\n", ast_channel_name(chan), target_context);
 				}
-				ast_frfree(f);
-				f = &ast_null_frame;
 			} else {
-				ast_channel_lock(chan);
 				ast_log(LOG_NOTICE, "FAX %s detected but no fax extension in context (%s)\n",
 					(result == 'f') ? "CNG" : "T38", target_context);
 			}
+			ast_channel_lock(chan);
+
+			ast_framehook_detach(chan, details->faxdetect_id);
+			details->faxdetect_id = -1;
+			break;
+		default:
+			break;
 		}
-		ast_framehook_detach(chan, details->faxdetect_id);
-		details->faxdetect_id = -1;
 	}
 
 	return f;

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I99da35c26d1cd802626ffb4c1b4eb5b015581b6d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>