[PATCH] wifi: brcmfmac: drain bus_reset work on device removal
From: Fan Wu <hidden>
Date: 2026-07-09 10:18:24
Also in:
linux-wireless, lkml, stable
Subsystem:
broadcom brcm80211 ieee802.11 wireless drivers, the rest · Maintainers:
Arend van Spriel, Linus Torvalds
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.
Cancellation cannot live in brcmf_detach() or brcmf_free(): the work
callback reaches teardown through the bus .reset op (PCIe
brcmf_pcie_reset -> brcmf_detach; SDIO brcmf_sdio_bus_reset ->
brcmf_sdiod_remove -> brcmf_free), so cancelling there would wait for
the running work and deadlock. Arming and the drain must also be
mutually exclusive: a debugfs writer can otherwise schedule bus_reset
after the drain and before the debugfs file is removed in
brcmf_cfg80211_detach(), re-opening the window.
Add a per-bus mutex and route all arming through
brcmf_bus_schedule_reset(), which under the lock skips when the bus is
marked removing. Each bus remove entry calls
brcmf_bus_cancel_reset_work(), which under the same lock sets removing
and cancels the work. Where applicable the remove entry first stops
the firmware-crash producer: on PCIe mask the mailbox and
synchronize_irq; on SDIO unregister the bus interrupt and cancel the
data worker, which also reports firmware halts through
brcmf_fw_crashed(). The mutex is initialized at bus allocation so it
is ready before any firmware-probe or removal path can reach it. The
SDIO suspend power-off path frees drvr through the same
brcmf_sdiod_remove() and takes the same lock; resume re-allows the work
only on a successful re-probe.
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(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index ac02244a6..c4bb32aec 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c@@ -1043,6 +1043,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, bus_if = kzalloc(sizeof(struct brcmf_bus), GFP_KERNEL); if (!bus_if) return -ENOMEM; + mutex_init(&bus_if->bus_reset_lock); sdiodev = kzalloc(sizeof(struct brcmf_sdio_dev), GFP_KERNEL); if (!sdiodev) { kfree(bus_if);
@@ -1102,6 +1103,14 @@ static void brcmf_ops_sdio_remove(struct sdio_func *func) if (func->num != 1) return; + /* Drain bus_reset before the shared brcmf_sdiod_remove() + * teardown, which the SDIO reset callback also reaches. The + * data worker can arm bus_reset via brcmf_fw_crashed(); cancel + * it first. + */ + brcmf_sdio_cancel_datawork(sdiodev->bus); + brcmf_bus_cancel_reset_work(bus_if); + /* only proceed with rest of cleanup if func 1 */ brcmf_sdiod_remove(sdiodev);
@@ -1163,6 +1172,8 @@ static int brcmf_ops_sdio_suspend(struct device *dev) } else { /* power will be cut so remove device, probe again in resume */ brcmf_sdiod_intr_unregister(sdiodev); + brcmf_sdio_cancel_datawork(sdiodev->bus); + brcmf_bus_cancel_reset_work(bus_if); ret = brcmf_sdiod_remove(sdiodev); if (ret) brcmf_err("Failed to remove device on suspend\n");
@@ -1188,6 +1199,8 @@ static int brcmf_ops_sdio_resume(struct device *dev) ret = brcmf_sdiod_probe(sdiodev); if (ret) brcmf_err("Failed to probe device on resume\n"); + else + brcmf_bus_allow_reset_work(bus_if); } else { if (sdiodev->wowl_enabled && sdiodev->settings->bus.sdio.oob_irq_supported)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
index 3f5da3bb6..b606094af 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h@@ -6,6 +6,7 @@ #ifndef BRCMFMAC_BUS_H #define BRCMFMAC_BUS_H +#include <linux/mutex.h> #include "debug.h" /* IDs of the 6 default common rings of msgbuf protocol */
@@ -149,11 +150,16 @@ struct brcmf_bus { u32 chiprev; bool always_use_fws_queue; bool wowl_supported; + bool removing; /* device removal in progress; quiesce async work */ + struct mutex bus_reset_lock; const struct brcmf_bus_ops *ops; struct brcmf_bus_msgbuf *msgbuf; }; +void brcmf_bus_cancel_reset_work(struct brcmf_bus *bus_if); +void brcmf_bus_allow_reset_work(struct brcmf_bus *bus_if); + /* * callback wrappers */
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); +} + +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); +} + +void brcmf_bus_allow_reset_work(struct brcmf_bus *bus_if) +{ + mutex_lock(&bus_if->bus_reset_lock); + bus_if->removing = false; + mutex_unlock(&bus_if->bus_reset_lock); +} + static void brcmf_core_bus_reset(struct work_struct *work) { struct brcmf_pub *drvr = container_of(work, struct brcmf_pub,
@@ -1184,7 +1213,7 @@ static ssize_t bus_reset_write(struct file *file, const char __user *user_buf, if (value != 1) return -EINVAL; - schedule_work(&drvr->bus_reset); + brcmf_bus_schedule_reset(drvr->bus_if); return count; }
@@ -1408,7 +1437,7 @@ void brcmf_fw_crashed(struct device *dev) brcmf_dev_coredump(dev); - schedule_work(&drvr->bus_reset); + brcmf_bus_schedule_reset(bus_if); } void brcmf_detach(struct device *dev)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 8b149996f..3c6775166 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c@@ -1914,6 +1914,7 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) ret = -ENOMEM; goto fail; } + mutex_init(&bus->bus_reset_lock); bus->msgbuf = kzalloc(sizeof(*bus->msgbuf), GFP_KERNEL); if (!bus->msgbuf) { ret = -ENOMEM;
@@ -1985,6 +1986,11 @@ brcmf_pcie_remove(struct pci_dev *pdev) if (devinfo->ci) brcmf_pcie_intr_disable(devinfo); + if (devinfo->irq_allocated) + synchronize_irq(pdev->irq); + + brcmf_bus_cancel_reset_work(bus); + brcmf_detach(&pdev->dev); brcmf_free(&pdev->dev);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 8effeb7a7..31e37b0d4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c@@ -4541,6 +4541,12 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev) return NULL; } +void brcmf_sdio_cancel_datawork(struct brcmf_sdio *bus) +{ + if (bus) + cancel_work_sync(&bus->datawork); +} + /* Detach and free everything */ void brcmf_sdio_remove(struct brcmf_sdio *bus) {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 15d2c02fa..3c68ebf8e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h@@ -373,6 +373,7 @@ int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev); struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev); void brcmf_sdio_remove(struct brcmf_sdio *bus); void brcmf_sdio_isr(struct brcmf_sdio *bus, bool in_isr); +void brcmf_sdio_cancel_datawork(struct brcmf_sdio *bus); void brcmf_sdio_wd_timer(struct brcmf_sdio *bus, bool active); void brcmf_sdio_wowl_config(struct device *dev, bool enabled);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 9fb68c2dc..97d65ba36 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c@@ -1271,6 +1271,7 @@ static int brcmf_usb_probe_cb(struct brcmf_usbdev_info *devinfo) ret = -ENOMEM; goto fail; } + mutex_init(&bus->bus_reset_lock); bus->dev = dev; bus_pub->bus = bus;
@@ -1336,6 +1337,8 @@ brcmf_usb_disconnect_cb(struct brcmf_usbdev_info *devinfo) return; brcmf_dbg(USB, "Enter, bus_pub %p\n", devinfo); + brcmf_bus_cancel_reset_work(devinfo->bus_pub.bus); + brcmf_detach(devinfo->dev); brcmf_free(devinfo->dev); kfree(devinfo->bus_pub.bus);
--
2.34.1