netdev - [PATCH] mwifiex: Fix use-after-free bug due to race condition between main thread thread and timer thread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 18 Feb 2023 15:59:56 +0800
From:   Zheng Wang <zyytlz.wz@....com>
To:     ganapathi017@...il.com
Cc:     alex000young@...il.com, amitkarwar@...il.com,
        sharvari.harisangam@....com, huxinming820@...il.com,
        kvalo@...nel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Zheng Wang <zyytlz.wz@....com>
Subject: [PATCH] mwifiex: Fix use-after-free bug due to race condition between main thread thread and timer thread

This is a potential race condition by executing the following order.

In summary, the adapter could be freed in timer function and be used after
that. The race condition needs 10s window which could be extended by the
paper : https://www.usenix.org/system/files/sec21-lee-yoochan.pdf

And the function in wakeup_timer_fn may have the same problem.

I dont't really know how to fix that, so I just removed the reset call,
which is totally wrong. If you know anything abouth the fix,
plz free to let me know.

Note that, this bug is found by static analysis, it could be wrong. We
could discuss that before writing the fix.

        CPU0                                                        CPU1
mwifiex_sdio_probe
mwifiex_add_card
mwifiex_init_hw_fw
request_firmware_nowait
  mwifiex_fw_dpc
    _mwifiex_fw_dpc
      mwifiex_init_fw
        mwifiex_main_process
          mwifiex_exec_next_cmd
            mwifiex_dnld_cmd_to_fw
              mod_timer(&adapter->cmd_timer,..)
                                                mwifiex_cmd_timeout_func
                                                  if_ops.card_reset(adapter)
                                                    mwifiex_sdio_card_reset
                                                      schedule_work(&card->work)
                                                        mwifiex_sdio_work
                                                          mwifiex_sdio_card_reset_work
                                                            mwifiex_reinit_sw
                                                              _mwifiex_fw_dpc
                                                                mwifiex_free_adapter
                                                                  mwifiex_unregister
                                                                    kfree(adapter)  //free adapter
                mwifiex_get_priv
                  // Use adapter

Signed-off-by: Zheng Wang <zyytlz.wz@....com>
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c | 2 --
 drivers/net/wireless/marvell/mwifiex/init.c   | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index d3339d67e7a0..688dd451aba9 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -1016,8 +1016,6 @@ mwifiex_cmd_timeout_func(struct timer_list *t)
 	if (adapter->if_ops.device_dump)
 		adapter->if_ops.device_dump(adapter);
 
-	if (adapter->if_ops.card_reset)
-		adapter->if_ops.card_reset(adapter);
 }
 
 void
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 7dddb4b5dea1..ff2d447c1de3 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -47,8 +47,6 @@ static void wakeup_timer_fn(struct timer_list *t)
 	adapter->hw_status = MWIFIEX_HW_STATUS_RESET;
 	mwifiex_cancel_all_pending_cmd(adapter);
 
-	if (adapter->if_ops.card_reset)
-		adapter->if_ops.card_reset(adapter);
 }
 
 static void fw_dump_work(struct work_struct *work)
-- 
2.25.1