wifi: iwlwifi: fix opmode start/stop race

There's a race when the device is unbound (maybe because the
module is unloaded) while the opmode start hasn't finished yet.
The complete(request_firmware_complete) after the opmode start
was meant (and commented accordingly) to prevent this problem,
but it's not sufficient when the opmode module is loaded after
the firmware load already completed, which happens regularly
now because firmware load doesn't require userspace, unlike
module load.

Fix this by using the existing opmode registration mutex to
protected the start/stop flows against each other properly.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Link: https://lore.kernel.org/r/20230926110319.85951554fed8.I62f20f40d79d0f136fa05e46d7fc16dc437fa3db@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 41ae4fae4b8a96dac1359eb48f4ccf14b529450a..c4e50f2046303432f9719f068c7f42dbc78f984b 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
 /*
- * Copyright (C) 2005-2014, 2018-2021 Intel Corporation
+ * Copyright (C) 2005-2014, 2018-2023 Intel Corporation
  * Copyright (C) 2013-2015 Intel Mobile Communications GmbH
  * Copyright (C) 2016-2017 Intel Deutschland GmbH
  */
@@ -1415,6 +1415,9 @@ _iwl_op_mode_start(struct iwl_drv *drv, struct iwlwifi_opmode_table *op)
        struct iwl_op_mode *op_mode = NULL;
        int retry, max_retry = !!iwlwifi_mod_params.fw_restart * IWL_MAX_INIT_RETRY;
 
+       /* also protects start/stop from racing against each other */
+       lockdep_assert_held(&iwlwifi_opmode_table_mtx);
+
        for (retry = 0; retry <= max_retry; retry++) {
 
 #ifdef CONFIG_IWLWIFI_DEBUGFS
@@ -1445,6 +1448,9 @@ _iwl_op_mode_start(struct iwl_drv *drv, struct iwlwifi_opmode_table *op)
 
 static void _iwl_op_mode_stop(struct iwl_drv *drv)
 {
+       /* also protects start/stop from racing against each other */
+       lockdep_assert_held(&iwlwifi_opmode_table_mtx);
+
        /* op_mode can be NULL if its start failed */
        if (drv->op_mode) {
                iwl_op_mode_stop(drv->op_mode);
@@ -1728,11 +1734,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
        }
        mutex_unlock(&iwlwifi_opmode_table_mtx);
 
-       /*
-        * Complete the firmware request last so that
-        * a driver unbind (stop) doesn't run while we
-        * are doing the start() above.
-        */
        complete(&drv->request_firmware_complete);
 
        /*
@@ -1837,11 +1838,12 @@ void iwl_drv_stop(struct iwl_drv *drv)
 {
        wait_for_completion(&drv->request_firmware_complete);
 
+       mutex_lock(&iwlwifi_opmode_table_mtx);
+
        _iwl_op_mode_stop(drv);
 
        iwl_dealloc_ucode(drv);
 
-       mutex_lock(&iwlwifi_opmode_table_mtx);
        /*
         * List is empty (this item wasn't added)
         * when firmware loading failed -- in that