[PATCH wireless 0/1] wifi: brcmfmac: fix oops on removal while wpa_supplicant exits

[PATCH wireless 0/1] wifi: brcmfmac: fix oops on removal while wpa_supplicant exits

[Michele Dionisio]

Unloading brcmfmac while wpa_supplicant is still exiting crashes the
kernel in _cfg80211_unregister_wdev(). The P2P device interface is
removed twice: once by NL80211_CMD_DEL_INTERFACE (brcmf_p2p_del_vif())
and once by brcmf_detach(), which reads ifp->vif before blocking on
rtnl_lock() and then uses it after the other path has freed it.

I hit this on an i.MX 8M Plus board with a CYW55513 (Sona IF513) on
SDIO, running the Ezurio backport of brcmfmac from v6.18.22 on a 5.4-rt
kernel. The code involved is unchanged in mainline. It reproduces with:

  kill $(pidof wpa_supplicant); rmmod brcmfmac_cyw brcmfmac

The patch takes RTNL and the wiphy mutex in brcmf_detach() before
removing the interface that has no netdev, and re-reads iflist under
them.

Testing: on mainline the patch is build tested only (W=1, no warnings).
I tested the same change on the board above, applied to the Ezurio
backport of brcmfmac (from v6.18.22) running on the 5.4-rt kernel: with
the patch the reproducer no longer crashes the kernel. I am not able to
test a mainline kernel on that board.

The analysis of the oops and the patch were done with the help of an
AI assistant (Claude), from the oops, the driver debug log (debug=0x406)
and the driver sources. I reviewed the change and I can answer
questions about it.

Michele Dionisio (1):
  wifi: brcmfmac: fix P2P device removal race in brcmf_detach()

 .../broadcom/brcm80211/brcmfmac/core.c        | 27 +++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.53.0

[PATCH wireless 1/1] wifi: brcmfmac: fix P2P device removal race in brcmf_detach()

[Michele Dionisio]

When the driver is removed while the user space process that created
the P2P device (e.g. wpa_supplicant) is still exiting, the P2P device
interface is removed twice and the kernel crashes.

The two paths are:

 - wpa_supplicant, on exit, sends NL80211_CMD_DEL_INTERFACE for the P2P
   device. nl80211_del_interface() holds RTNL and the wiphy mutex and
   calls brcmf_p2p_del_vif(), which disables discovery and waits up to
   1.5 s for BRCMF_E_IF_DEL. Then it calls brcmf_remove_interface(),
   which unregisters the wdev and frees the vif and the ifp.

 - brcmf_detach() sets the bus down, so BRCMF_E_IF_DEL never arrives,
   and calls brcmf_remove_interface(ifp, false) for the same ifp.
   brcmf_p2p_ifp_removed() reads ifp->vif and then blocks on
   rtnl_lock(). When brcmf_p2p_del_vif() releases RTNL, it calls
   cfg80211_unregister_wdev() on the wdev that was already unregistered
   and freed.

The second list_del_rcu() of wdev->list faults on LIST_POISON2:

  brcmfmac: brcmf_p2p_del_vif delete P2P vif
  brcmfmac: brcmf_p2p_deinit_discovery enter
  brcmfmac: brcmf_p2p_del_vif P2P: GO_NEG_PHASE status cleared
  brcmfmac: brcmf_detach Enter
  brcmfmac: brcmf_bus_change_state 1 -> 0
  brcmfmac: brcmf_remove_interface Enter, bsscfgidx=1, ifidx=0
  brcmfmac: brcmf_del_if Enter, bsscfgidx=1, ifidx=0
  brcmfmac: brcmf_p2p_ifp_removed P2P: device interface removed
  [1.5 s later, brcmf_p2p_del_vif() timed out]
  brcmfmac: brcmf_remove_interface Enter, bsscfgidx=1, ifidx=0
  brcmfmac: brcmf_del_if Enter, bsscfgidx=1, ifidx=0
  brcmfmac: brcmf_p2p_ifp_removed P2P: device interface removed
  Unable to handle kernel paging request at virtual address dead000000000122
  Internal error: Oops: 96000044 [#1] PREEMPT_RT SMP
  pc : _cfg80211_unregister_wdev+0x6c/0x264 [cfg80211]
  Call trace:
   _cfg80211_unregister_wdev+0x6c/0x264 [cfg80211]
   cfg80211_unregister_wdev+0x10/0x1c [cfg80211]
   brcmf_p2p_ifp_removed+0x84/0xb0 [brcmfmac]
   brcmf_remove_interface+0x1f4/0x254 [brcmfmac]
   brcmf_detach+0x88/0x180 [brcmfmac]
   brcmf_sdio_remove+0x90/0x7b0 [brcmfmac]
   brcmf_sdiod_remove+0x20/0xa0 [brcmfmac]
   brcmf_ops_sdio_remove+0xbc/0x12c [brcmfmac]
   sdio_bus_remove+0x38/0x144
   device_release_driver_internal+0x1e8/0x2c0
   device_release_driver+0x14/0x20
   brcmf_sdio_bus_remove+0x2c/0x40 [brcmfmac]
   brcmf_fwvid_unregister_vendor+0xd8/0x170 [brcmfmac]
   __exit_compat+0x18/0x418 [brcmfmac_cyw]
   __arm64_sys_delete_module+0x1ac/0x244

The log was taken with debug=0x406 on an i.MX 8M Plus board with a
CYW55513 (Sona IF513) on SDIO. The driver is the Ezurio backport of
brcmfmac from v6.18.22 on a 5.4-rt kernel. The code involved is the same
in mainline.

To reproduce:

  1. Start wpa_supplicant on wlan0 and let it associate. It creates the
     P2P device (iw dev shows "type P2P-device").
  2. Stop wpa_supplicant and remove the driver without waiting for
     wpa_supplicant to exit:

       kill $(pidof wpa_supplicant)
       rmmod brcmfmac_cyw brcmfmac

The crash is reproducible on that board.

For the interface without a netdev (the P2P device), take RTNL and the
wiphy mutex in brcmf_detach() before reading iflist, and remove it with
locked=true. If brcmf_p2p_del_vif() runs first, brcmf_detach() finds the
slot empty. If brcmf_detach() runs first, nl80211 does not find the wdev
any more. The interfaces with a netdev are removed as before, because
brcmf_del_if() takes RTNL on its own for the primary interface.

Fixes: 9831bcb987df ("brcmfmac: Deleting of p2p device is leaking memory.")
Cc: stable@vger.kernel.org
Assisted-by: claude-opus-5-5
Signed-off-by: Michele Dionisio <michele.dionisio@gmail.com>
---
 .../broadcom/brcm80211/brcmfmac/core.c        | 27 +++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index d2ae679856067..92ec269f43b23 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1489,8 +1489,31 @@ void brcmf_detach(struct device *dev)
 	brcmf_bus_change_state(bus_if, BRCMF_BUS_DOWN);
 	/* make sure primary interface removed last */
 	for (i = BRCMF_MAX_IFS - 1; i > -1; i--) {
-		if (drvr->iflist[i])
-			brcmf_remove_interface(drvr->iflist[i], false);
+		struct brcmf_if *ifp = drvr->iflist[i];
+
+		if (!ifp)
+			continue;
+
+		if (ifp->ndev) {
+			brcmf_remove_interface(ifp, false);
+			continue;
+		}
+
+		/* The P2P device interface has no netdev. Its removal can
+		 * race with NL80211_CMD_DEL_INTERFACE issued by a user space
+		 * process that is exiting (e.g. wpa_supplicant), which ends
+		 * in brcmf_p2p_del_vif() under RTNL and the wiphy mutex. If
+		 * both paths remove the interface, the wdev is unregistered
+		 * twice and the vif is used after being freed. Take the same
+		 * locks and re-read iflist, so only one path removes it.
+		 */
+		rtnl_lock();
+		wiphy_lock(drvr->wiphy);
+		ifp = drvr->iflist[i];
+		if (ifp)
+			brcmf_remove_interface(ifp, true);
+		wiphy_unlock(drvr->wiphy);
+		rtnl_unlock();
 	}
 	brcmf_bus_stop(drvr->bus_if);
 
-- 
2.53.0