summaryrefslogtreecommitdiff
path: root/channels
diff options
context:
space:
mode:
authorRichard Mudgett <rmudgett@digium.com>2016-07-19 13:18:47 -0500
committerRichard Mudgett <rmudgett@digium.com>2016-07-19 13:27:32 -0500
commitdcb8aa8c1c1207a6ee0c8f9f158154254ab63ec8 (patch)
tree3b9b56693be66cb6b99aac48e99e0920196b948e /channels
parentfa91cf3eeca1ee3b2bdb5e3d685de7100702ff21 (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
Diffstat (limited to 'channels')
-rw-r--r--channels/chan_dahdi.c28
1 files changed, 16 insertions, 12 deletions
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index e9d0b5bd3..9e0bed701 100644
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -1696,26 +1696,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");
}
@@ -7203,26 +7205,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");
}