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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 0336e2a342a1854940637429b3c4ab04a6e33523..68400e73e20413cf08a15047afdb9316f27f3947 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -8605,29 +8605,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);
                }
        }