From db4979fa79bc3f5d754aa76816a39869132b10ca Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Wed, 13 Jul 2016 18:49:08 -0500 Subject: chan_sip.c: Fix deadlock potential in fax redirection. The sip_read() 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: I6d3f5cccd4b77c3aa6ffc1a54c0f6bde61c9278e --- channels/chan_sip.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'channels') diff --git a/channels/chan_sip.c b/channels/chan_sip.c index b2522b6db..ab54c765b 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -8609,29 +8609,31 @@ static struct ast_frame *sip_read(struct ast_channel *ast) if (faxdetected && ast_test_flag(&p->flags[1], SIP_PAGE2_FAX_DETECT_CNG)) { if (strcmp(ast_channel_exten(ast), "fax")) { const char *target_context = S_OR(ast_channel_macrocontext(ast), ast_channel_context(ast)); - /* We need to unlock 'ast' here because + /* + * We need to unlock 'ast' 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. */ sip_pvt_unlock(p); ast_channel_unlock(ast); + ast_frfree(fr); + fr = &ast_null_frame; if (ast_exists_extension(ast, target_context, "fax", 1, S_COR(ast_channel_caller(ast)->id.number.valid, ast_channel_caller(ast)->id.number.str, NULL))) { - ast_channel_lock(ast); - sip_pvt_lock(p); ast_verb(2, "Redirecting '%s' to fax extension due to CNG detection\n", ast_channel_name(ast)); pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast_channel_exten(ast)); if (ast_async_goto(ast, target_context, "fax", 1)) { ast_log(LOG_NOTICE, "Failed to async goto '%s' into fax of '%s'\n", ast_channel_name(ast), target_context); } - ast_frfree(fr); - fr = &ast_null_frame; } else { - ast_channel_lock(ast); - sip_pvt_lock(p); ast_log(LOG_NOTICE, "FAX CNG detected but no fax extension\n"); } + ast_channel_lock(ast); + sip_pvt_lock(p); } } -- cgit v1.2.3