diff options
author | Richard Mudgett <rmudgett@digium.com> | 2016-07-19 13:18:47 -0500 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2016-07-19 13:31:51 -0500 |
commit | 3d62f317dd0f1fb8a919c03861f1b6f50ce6c0d7 (patch) | |
tree | ee3cbf06528a73eb379d0d8a42f2a9bc11f5ac94 | |
parent | db4979fa79bc3f5d754aa76816a39869132b10ca (diff) |
chan_dahdi.c: Fix deadlock potential in fax redirection.
The dahdi_handle_dtmf() and my_handle_dtmf() have 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.
ASTERISK-26216 #close
Reported by: Richard Mudgett
Change-Id: I9144b84ade5f96690996624ec8a2d40c56af40aa
-rw-r--r-- | channels/chan_dahdi.c | 28 |
1 files changed, 16 insertions, 12 deletions
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c index e4b7c0ee8..308e0930b 100644 --- a/channels/chan_dahdi.c +++ b/channels/chan_dahdi.c @@ -1693,26 +1693,28 @@ static void my_handle_dtmf(void *pvt, struct ast_channel *ast, enum analog_sub a 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 ast_exists_extension has the + /* + * We need to unlock 'ast' here because ast_exists_extension has the * potential to start autoservice on the channel. Such action is prone - * to deadlock. + * to deadlock if the channel is locked. + * + * ast_async_goto() has its own restriction on not holding the + * channel lock. */ ast_mutex_unlock(&p->lock); ast_channel_unlock(ast); 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); - ast_mutex_lock(&p->lock); ast_verb(3, "Redirecting %s to fax extension\n", ast_channel_name(ast)); /* Save the DID/DNIS when we transfer the fax call to a "fax" extension */ pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast_channel_exten(ast)); if (ast_async_goto(ast, target_context, "fax", 1)) ast_log(LOG_WARNING, "Failed to async goto '%s' into fax of '%s'\n", ast_channel_name(ast), target_context); } else { - ast_channel_lock(ast); - ast_mutex_lock(&p->lock); ast_log(LOG_NOTICE, "Fax detected, but no fax extension\n"); } + ast_channel_lock(ast); + ast_mutex_lock(&p->lock); } else { ast_debug(1, "Already in a fax extension, not redirecting\n"); } @@ -7200,26 +7202,28 @@ static void dahdi_handle_dtmf(struct ast_channel *ast, int idx, struct ast_frame 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 ast_exists_extension has the + /* + * We need to unlock 'ast' here because ast_exists_extension has the * potential to start autoservice on the channel. Such action is prone - * to deadlock. + * to deadlock if the channel is locked. + * + * ast_async_goto() has its own restriction on not holding the + * channel lock. */ ast_mutex_unlock(&p->lock); ast_channel_unlock(ast); 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); - ast_mutex_lock(&p->lock); ast_verb(3, "Redirecting %s to fax extension\n", ast_channel_name(ast)); /* Save the DID/DNIS when we transfer the fax call to a "fax" extension */ pbx_builtin_setvar_helper(ast, "FAXEXTEN", ast_channel_exten(ast)); if (ast_async_goto(ast, target_context, "fax", 1)) ast_log(LOG_WARNING, "Failed to async goto '%s' into fax of '%s'\n", ast_channel_name(ast), target_context); } else { - ast_channel_lock(ast); - ast_mutex_lock(&p->lock); ast_log(LOG_NOTICE, "Fax detected, but no fax extension\n"); } + ast_channel_lock(ast); + ast_mutex_lock(&p->lock); } else { ast_debug(1, "Already in a fax extension, not redirecting\n"); } |