res_fax: Fix deadlock setting FAXMODE channel variable.

ASTERISK-25980 added the FAXMODE channel variable to res_fax.c.
Unfortunately, it also introduced a deadlock potential because
set_channel_variables() which sets FAXMODE can be called during a
masquerade.  The ast_channel_get_t38_state() which gets the value used to
set FAXMODE cannot be called with the channel locked.  As a result, local
channels can deadlock because of how they must acquire the locks necessary
to operate.

The intent of FAXMODE is for dialplan to know how a fax was transferred
after the fax completes.  However, the previous patch sets FAXMODE to the
channel's current T.38 state AFTER the fax has completed and where T.38
may have already disconnected.

* Set FAXMODE based upon T.38 negotiations exchanged either with the fax
applications or the fax framehooks.

ASTERISK-26203
Reported by: Etienne Lessard

ASTERISK-24822
Reported by: David Brillert

ASTERISK-22732
Reported by: Richard Mudgett

Change-Id: Id525747254b64c1efe8b1b5973d52ff9719c2ae1

diff --git a/include/asterisk/res_fax.h b/include/asterisk/res_fax.h
index 5119bfa6c48ffabdfa28df90500c0ed3a094e4a3..e88d8002f0166d6abff8f879f7f66c50fba40bae 100644 (file)
--- a/include/asterisk/res_fax.h
+++ b/include/asterisk/res_fax.h
@@ -20,16 +20,16 @@
 #ifndef _ASTERISK_RES_FAX_H
 #define _ASTERISK_RES_FAX_H
 
-#include <asterisk.h>
-#include <asterisk/lock.h>
-#include <asterisk/linkedlists.h>
-#include <asterisk/module.h>
-#include <asterisk/utils.h>
-#include <asterisk/options.h>
-#include <asterisk/frame.h>
-#include <asterisk/cli.h>
-#include <asterisk/stringfields.h>
-#include <asterisk/manager.h>
+#include "asterisk.h"
+#include "asterisk/lock.h"
+#include "asterisk/linkedlists.h"
+#include "asterisk/module.h"
+#include "asterisk/utils.h"
+#include "asterisk/options.h"
+#include "asterisk/frame.h"
+#include "asterisk/cli.h"
+#include "asterisk/stringfields.h"
+#include "asterisk/manager.h"
 
 /*! \brief capabilities for res_fax to locate a fax technology module */
 enum ast_fax_capabilities {
@@ -187,6 +187,8 @@ struct ast_fax_session_details {
        int faxdetect_timeout;
        /*! flags used for fax detection */
        int faxdetect_flags;
+       /*! Non-zero if T.38 is negotiated */
+       int is_t38_negotiated;
 };
 
 struct ast_fax_tech;

diff --git a/res/res_fax.c b/res/res_fax.c
index 67b3af1feb86effe279a697a00850a90add028ab..b7400fa7211cbe74a2a414ca31bb69a39a08cbd8 100644 (file)
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -641,6 +641,7 @@ static void fixup_callback(void *data, struct ast_channel *old_chan, struct ast_
                struct ast_fax_session_details *new_details = find_or_create_details(new_chan);
 
                ast_framehook_detach(old_chan, old_details->gateway_id);
+               new_details->is_t38_negotiated = old_details->is_t38_negotiated;
                fax_gateway_attach(new_chan, new_details);
                ao2_cleanup(new_details);
        }
@@ -1439,6 +1440,7 @@ static int report_fax_status(struct ast_channel *chan, struct ast_fax_session_de
 static void set_channel_variables(struct ast_channel *chan, struct ast_fax_session_details *details)
 {
        char buf[10];
+
        pbx_builtin_setvar_helper(chan, "FAXSTATUS", S_OR(details->result, NULL));
        pbx_builtin_setvar_helper(chan, "FAXERROR", S_OR(details->error, NULL));
        pbx_builtin_setvar_helper(chan, "FAXSTATUSSTRING", S_OR(details->resultstr, NULL));
@@ -1447,7 +1449,7 @@ static void set_channel_variables(struct ast_channel *chan, struct ast_fax_sessi
        pbx_builtin_setvar_helper(chan, "FAXBITRATE", S_OR(details->transfer_rate, NULL));
        pbx_builtin_setvar_helper(chan, "FAXRESOLUTION", S_OR(details->resolution, NULL));
 
-       if (ast_channel_get_t38_state(chan) == T38_STATE_NEGOTIATED) {
+       if (details->is_t38_negotiated) {
                pbx_builtin_setvar_helper(chan, "FAXMODE", "T38");
        } else {
                pbx_builtin_setvar_helper(chan, "FAXMODE", "audio");
@@ -1656,6 +1658,7 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
        ast_string_field_set(details, result, "");
        ast_string_field_set(details, resultstr, "");
        ast_string_field_set(details, error, "");
+       details->is_t38_negotiated = t38negotiated;
        set_channel_variables(chan, details);
 
        if (fax->tech->start_session(fax) < 0) {
@@ -1706,12 +1709,18 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
                                         * do T.38 as well
                                         */
                                        t38_parameters_fax_to_ast(&t38_parameters, &details->our_t38_parameters);
-                                       t38_parameters.request_response = (details->caps & AST_FAX_TECH_T38) ? AST_T38_NEGOTIATED : AST_T38_REFUSED;
+                                       if (details->caps & AST_FAX_TECH_T38) {
+                                               details->is_t38_negotiated = 1;
+                                               t38_parameters.request_response = AST_T38_NEGOTIATED;
+                                       } else {
+                                               t38_parameters.request_response = AST_T38_REFUSED;
+                                       }
                                        ast_indicate_data(chan, AST_CONTROL_T38_PARAMETERS, &t38_parameters, sizeof(t38_parameters));
                                        break;
                                case AST_T38_NEGOTIATED:
                                        t38_parameters_ast_to_fax(&details->their_t38_parameters, parameters);
                                        t38negotiated = 1;
+                                       details->is_t38_negotiated = 1;
                                        break;
                                default:
                                        break;
@@ -2902,6 +2911,7 @@ static int fax_gateway_start(struct fax_gateway *gateway, struct ast_fax_session
                ast_string_field_set(details, result, "FAILED");
                ast_string_field_set(details, resultstr, "error starting gateway session");
                ast_string_field_set(details, error, "INIT_ERROR");
+               details->is_t38_negotiated = 0;
                set_channel_variables(chan, details);
                report_fax_status(chan, details, "No Available Resource");
                ast_log(LOG_ERROR, "Can't create a FAX session, gateway attempt failed.\n");
@@ -2922,6 +2932,7 @@ static int fax_gateway_start(struct fax_gateway *gateway, struct ast_fax_session
                ast_string_field_set(details, result, "FAILED");
                ast_string_field_set(details, resultstr, "error starting gateway session");
                ast_string_field_set(details, error, "INIT_ERROR");
+               details->is_t38_negotiated = 0;
                set_channel_variables(chan, details);
                return -1;
        }
@@ -2967,6 +2978,7 @@ static struct ast_frame *fax_gateway_request_t38(struct fax_gateway *gateway, st
 
        gateway->t38_state = T38_STATE_NEGOTIATING;
        gateway->timeout_start = ast_tvnow();
+       details->is_t38_negotiated = 0;
        details->gateway_timeout = FAX_GATEWAY_TIMEOUT;
 
        ast_debug(1, "requesting T.38 for gateway session for %s\n", ast_channel_name(chan));
@@ -3064,6 +3076,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                        t38_parameters_ast_to_fax(&details->their_t38_parameters, control_params);
                        gateway->t38_state = T38_STATE_UNKNOWN;
                        gateway->timeout_start = ast_tvnow();
+                       details->is_t38_negotiated = 0;
                        details->gateway_timeout = FAX_GATEWAY_TIMEOUT;
                        ao2_ref(details, -1);
                        return f;
@@ -3079,12 +3092,14 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                        if (fax_gateway_start(gateway, details, chan)) {
                                ast_log(LOG_ERROR, "error starting T.38 gateway for T.38 channel %s and G.711 channel %s\n", ast_channel_name(active), ast_channel_name(other));
                                gateway->t38_state = T38_STATE_REJECTED;
+                               details->is_t38_negotiated = 0;
                                control_params->request_response = AST_T38_REFUSED;
 
                                ast_framehook_detach(chan, details->gateway_id);
                                details->gateway_id = -1;
                        } else {
                                gateway->t38_state = T38_STATE_NEGOTIATED;
+                               details->is_t38_negotiated = chan == active;
                                control_params->request_response = AST_T38_NEGOTIATED;
                                report_fax_status(chan, details, "T.38 Negotiated");
                        }
@@ -3102,6 +3117,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                        t38_parameters_ast_to_fax(&details->their_t38_parameters, control_params);
                        gateway->t38_state = T38_STATE_UNKNOWN;
                        gateway->timeout_start = ast_tvnow();
+                       details->is_t38_negotiated = 0;
                        details->gateway_timeout = FAX_GATEWAY_TIMEOUT;
 
                        ast_debug(1, "%s is attempting to negotiate T.38 after we already sent a negotiation request based on v21 preamble detection\n", ast_channel_name(active));
@@ -3124,6 +3140,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                        ast_string_field_set(details, result, "SUCCESS");
                        ast_string_field_set(details, resultstr, "no gateway necessary");
                        ast_string_field_set(details, error, "NATIVE_T38");
+                       details->is_t38_negotiated = 1;
                        set_channel_variables(chan, details);
 
                        ast_debug(1, "%s is attempting to negotiate T.38 after we already negotiated T.38 with %s, disabling the gateway\n", ast_channel_name(active), ast_channel_name(other));
@@ -3138,6 +3155,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                && control_params->request_response == AST_T38_REFUSED) {
 
                ast_debug(1, "unable to negotiate T.38 on %s for fax gateway\n", ast_channel_name(active));
+               details->is_t38_negotiated = 0;
 
                /* our request to negotiate T.38 was refused, if the other
                 * channel supports T.38, they might still reinvite and save
@@ -3166,11 +3184,13 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                if (fax_gateway_start(gateway, details, chan)) {
                        ast_log(LOG_ERROR, "error starting T.38 gateway for T.38 channel %s and G.711 channel %s\n", ast_channel_name(active), ast_channel_name(other));
                        gateway->t38_state = T38_STATE_NEGOTIATING;
+                       details->is_t38_negotiated = 0;
                        control_params->request_response = AST_T38_REQUEST_TERMINATE;
 
                        fax_gateway_indicate_t38(chan, active, control_params);
                } else {
                        gateway->t38_state = T38_STATE_NEGOTIATED;
+                       details->is_t38_negotiated = chan == active;
                        report_fax_status(chan, details, "T.38 Negotiated");
                }
 
@@ -3186,14 +3206,16 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                t38_parameters_fax_to_ast(control_params, &details->our_t38_parameters);
 
                if (fax_gateway_start(gateway, details, chan)) {
-                       ast_log(LOG_ERROR, "error starting T.38 gateway for T.38 channel %s and G.711 channel %s\n", ast_channel_name(active), ast_channel_name(other));
+                       ast_log(LOG_ERROR, "error starting T.38 gateway for T.38 channel %s and G.711 channel %s\n", ast_channel_name(other), ast_channel_name(active));
                        gateway->t38_state = T38_STATE_REJECTED;
+                       details->is_t38_negotiated = 0;
                        control_params->request_response = AST_T38_REFUSED;
 
                        ast_framehook_detach(chan, details->gateway_id);
                        details->gateway_id = -1;
                } else {
                        gateway->t38_state = T38_STATE_NEGOTIATED;
+                       details->is_t38_negotiated = chan == other;
                        control_params->request_response = AST_T38_NEGOTIATED;
                }
 
@@ -3209,6 +3231,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                details->gateway_id = -1;
 
                gateway->t38_state = T38_STATE_REJECTED;
+               details->is_t38_negotiated = 0;
                control_params->request_response = AST_T38_TERMINATED;
 
                fax_gateway_indicate_t38(chan, active, control_params);
@@ -3224,6 +3247,7 @@ static struct ast_frame *fax_gateway_detect_t38(struct fax_gateway *gateway, str
                ast_string_field_set(details, result, "SUCCESS");
                ast_string_field_set(details, resultstr, "no gateway necessary");
                ast_string_field_set(details, error, "NATIVE_T38");
+               details->is_t38_negotiated = 1;
                set_channel_variables(chan, details);
 
                ao2_ref(details, -1);
@@ -3351,6 +3375,7 @@ static struct ast_frame *fax_gateway_framehook(struct ast_channel *chan, struct
                        ast_string_field_set(details, result, "FAILED");
                        ast_string_field_set(details, resultstr, "neither channel supports T.38");
                        ast_string_field_set(details, error, "T38_NEG_ERROR");
+                       details->is_t38_negotiated = 0;
                        set_channel_variables(chan, details);
                        return f;
                }
@@ -3395,6 +3420,7 @@ static struct ast_frame *fax_gateway_framehook(struct ast_channel *chan, struct
                        ast_string_field_set(details, result, "FAILED");
                        ast_string_field_build(details, resultstr, "no fax activity after %d ms", details->gateway_timeout);
                        ast_string_field_set(details, error, "TIMEOUT");
+                       details->is_t38_negotiated = 0;
                        set_channel_variables(chan, details);
                        return f;
                }
@@ -3525,6 +3551,7 @@ static int fax_gateway_attach(struct ast_channel *chan, struct ast_fax_session_d
                ast_string_field_set(details, result, "FAILED");
                ast_string_field_set(details, resultstr, "error initializing gateway session");
                ast_string_field_set(details, error, "INIT_ERROR");
+               details->is_t38_negotiated = 0;
                set_channel_variables(chan, details);
                report_fax_status(chan, details, "No Available Resource");
                return -1;
@@ -3540,6 +3567,7 @@ static int fax_gateway_attach(struct ast_channel *chan, struct ast_fax_session_d
                ast_string_field_set(details, result, "FAILED");
                ast_string_field_set(details, resultstr, "error attaching gateway to channel");
                ast_string_field_set(details, error, "INIT_ERROR");
+               details->is_t38_negotiated = 0;
                set_channel_variables(chan, details);
                return -1;
        }