Re: [PATCH] wifi: brcmfmac: drain bus_reset work on device removal

On 10/07/2026 02:23, Eddie Phillips wrote:

On Thu,  9 Jul 2026 10:16:35 +0000 Fan Wu [off-list ref] wrote:

quoted

brcmf_fw_crashed() and the debugfs "reset" entry both schedule
drvr->bus_reset, whose callback recovers drvr through container_of()
and dereferences it.  The teardown paths free drvr (brcmf_free ->
wiphy_free) without draining the work, so a bus_reset callback pending
or running during removal can outlive drvr.

[...]

quoted

This issue was found by an in-house static analysis tool.

Fixes: 4684997d9eea ("brcmfmac: reset PCIe bus on a firmware crash")
Cc: stable@vger.kernel.org
Signed-off-by: Fan Wu <redacted>
Assisted-by: Codex:gpt-5.5
---
  .../broadcom/brcm80211/brcmfmac/bcmsdh.c      | 13 ++++++++
  .../broadcom/brcm80211/brcmfmac/bus.h         |  6 ++++
  .../broadcom/brcm80211/brcmfmac/core.c        | 33 +++++++++++++++++--
  .../broadcom/brcm80211/brcmfmac/pcie.c        |  6 ++++
  .../broadcom/brcm80211/brcmfmac/sdio.c        |  6 ++++
  .../broadcom/brcm80211/brcmfmac/sdio.h        |  1 +
  .../broadcom/brcm80211/brcmfmac/usb.c         |  3 ++
  7 files changed, 66 insertions(+), 2 deletions(-)

[...]

quoted

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index fed9cd5f2..b934feb9b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c

@@ -1164,6 +1164,35 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data)
  	return 0;
  }
  
+/* Serialize bus_reset arming (debugfs reset write, brcmf_fw_crashed) against the
+ * teardown drain: the remove path takes bus_reset_lock, sets ->removing and cancels
+ * the work under it, so a racing armer either schedules before the cancel (and is
+ * drained) or observes ->removing and desists.
+ */
+static void brcmf_bus_schedule_reset(struct brcmf_bus *bus_if)
+{
+	mutex_lock(&bus_if->bus_reset_lock);
+	if (bus_if->drvr && bus_if->drvr->bus_reset.func && !bus_if->removing)
+		schedule_work(&bus_if->drvr->bus_reset);
+	mutex_unlock(&bus_if->bus_reset_lock);
+}

Is this safe in a softIRQ context?
mutex_lock() sleeps until it can get the lock.

What softIRQ context? brcmf_fw_crashed() is called by PCIe (thread) and 
SDIO (worker).

quoted

+
+void brcmf_bus_cancel_reset_work(struct brcmf_bus *bus_if)
+{
+	mutex_lock(&bus_if->bus_reset_lock);
+	bus_if->removing = true;
+	if (bus_if->drvr)
+		cancel_work_sync(&bus_if->drvr->bus_reset);
+	mutex_unlock(&bus_if->bus_reset_lock);
+}

How about if brcmf_pcie_remove() calls brcmf_bus_cancel_reset_work(),
takes the lock and calls cancel_work_sync(), sleeps. If debugfs
path is already running, it can invoke the worker thread. Is there
potential that both try to reset?

What is "both" here?
Regards,
Arend