From 089ee0bf87229b2bc7e47bdaf10ad77efd424ca7 Mon Sep 17 00:00:00 2001 From: xuxinchun1003 Date: Wed, 7 Oct 2026 07:42:31 +0800 Subject: [PATCH 1/3] fix: NULL-guard bus teardown and clear drvdata after cleanup completes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-driven rework of the teardown lifetime (shenmintao/aic8800d80#103): - aicwf_bus_deinit(): keep the entry NULL guard (sequential double-deinit on probe-failure paths), but move dev_set_drvdata(dev, NULL) to the END of completed cleanup — after URB cancel, platform/rx/cmd teardown and the kthread_stop() join of bustx_thread. The previous placement (right after bus_stop) opened a real race: aicwf_usb_bus_txdata() re-reads drvdata and runs from bustx_thread, which is joined only later, and the TX-path state check is not a synchronization point. With the new ordering every cleanup callback and TX reader observes a non-NULL pointer throughout. - aicwf_usb_bus_stop(): NULL-check bus_if before dereferencing bus_if->bus_priv.usb (defense in depth; historical host oops RIP aicwf_usb_bus_stop+0x11 on probe-failure rollback). Verification (isolated VFIO lab, one 2357:0147 unit, sustained ping -f): a test-only fault-injection build (not included) reaches the repeated-deinit path and prints "already deinitialized, skip" with BUG=0; missing-firmware init failure runs one clean deinit; physical unplug x8, sysfs unbind and authorized cold cycles, disconnect during scanning and during connection establishment, ifdown/up x5, rmmod/insmod x3 -> BUG=0 throughout (only the pre-existing WARNs at rwnx_main.c:1529 and rwnx_cfg80211_disconnect:3328). --- drivers/aic8800/aic8800_fdrv/aicwf_txrxif.c | 23 +++++++++++++++++++++ drivers/aic8800/aic8800_fdrv/aicwf_usb.c | 8 ++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/aic8800/aic8800_fdrv/aicwf_txrxif.c b/drivers/aic8800/aic8800_fdrv/aicwf_txrxif.c index b848421..c15dcf6 100644 --- a/drivers/aic8800/aic8800_fdrv/aicwf_txrxif.c +++ b/drivers/aic8800/aic8800_fdrv/aicwf_txrxif.c @@ -210,6 +210,15 @@ void aicwf_bus_deinit(struct device *dev) AICWFDBG(LOGINFO, "%s Enter\r\n", __func__); bus_if = dev_get_drvdata(dev); + /* 入口判空：本函数在探测失败路径上会被调用**两次** + * aicwf_bus_init() 的 `fail:` 先调一次，随后 aicwf_usb_probe() 的 + * `out_free_bus:` 再调一次（用局部 bus_if 做 kfree，不依赖 drvdata）。 + * 第二次进入时 drvdata 已在**上一次调用的末尾**被清空 → 必然命中本判空。 + * 若不清空，第二次会对已完整拆解的对象重复执行整套 teardown。 */ + if (!bus_if) { + AICWFDBG(LOGINFO, "%s: already deinitialized, skip\n", __func__); + return; + } aicwf_bus_stop(bus_if); #ifdef AICWF_USB_SUPPORT @@ -260,6 +269,20 @@ void aicwf_bus_deinit(struct device *dev) tasklet_kill(&usb->xmit_tasklet); #endif + /* 清空 drvdata：全部拆解完成之后、对象由调用方释放之前。 + * （评审 shenmintao/aic8800d80#103 指出的顺序问题：此前清空点位于 + * bus_stop 之后、URB/platform/rx/cmd 拆解与 TX 线程 join 之前。） + * 1) 顺序双 deinit：第二次调用靠上方入口判空在本行之后必然 skip； + * 2) 过早清空会伤害仍在运行的读者——TX 链 + * usb_bustx_thread → aicwf_bus_txdata(usbdev->bus_if) → + * ops->txdata = aicwf_usb_bus_txdata()（usb.c 在该处重读 + * dev_get_drvdata）仅靠 state 检查做 check-then-act、无锁， + * 且该线程直到上方 kthread_stop(bustx_thread) 才被 join—— + * 必须先 join 再清； + * 3) cmd/rx/platform 拆解与 rwnx_close() 等读者在本行之前全程 + * 看到非空 drvdata，行为与未打补丁树一致。 */ + dev_set_drvdata(dev, NULL); + AICWFDBG(LOGINFO, "%s Exit \n", __func__); } diff --git a/drivers/aic8800/aic8800_fdrv/aicwf_usb.c b/drivers/aic8800/aic8800_fdrv/aicwf_usb.c index c25171b..16000d7 100644 --- a/drivers/aic8800/aic8800_fdrv/aicwf_usb.c +++ b/drivers/aic8800/aic8800_fdrv/aicwf_usb.c @@ -1946,9 +1946,15 @@ void aicwf_usb_cancel_all_urbs(struct aic_usb_dev *usb_dev){ static void aicwf_usb_bus_stop(struct device *dev) { struct aicwf_bus *bus_if = dev_get_drvdata(dev); - struct aic_usb_dev *usb_dev = bus_if->bus_priv.usb; + struct aic_usb_dev *usb_dev; AICWFDBG(LOGINFO, "%s\r\n", __func__); + /* 正常时序下 aicwf_bus_init() 已 set_drvdata、且 aicwf_bus_deinit() + * 调到本函数时 drvdata 尚未清空（清空在 deinit 末尾），故此处应非空； + * 判空为纵深防御：覆盖 probe 早期失败（set_drvdata 之前）等异常来源。 */ + if (bus_if == NULL) + return; + usb_dev = bus_if->bus_priv.usb; if (usb_dev == NULL) return; From 24229305112bed5d61363bb11b85dfc9a0b4fab9 Mon Sep 17 00:00:00 2001 From: xuxinchun1003 Date: Wed, 7 Oct 2026 07:42:31 +0800 Subject: [PATCH 2/3] fix: NULL-guard usbdev/bus_if in rwnx_close reset path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the vif_started==0 branch rwnx_close() evaluates usbdev->bus_if->state without checking usbdev. On the disconnect path aicwf_bus_deinit() runs before netdev close, so close observes usbdev==NULL and oopses (CR2=0x8, pinned by disassembly relocation rwnx_send_reset; field symptom: host hard-hang on physical unplug, reproduced x8 in the lab with this stack -> BUG=0 after this guard). In the unpatched tree the same expression reads a still-allocated DOWN-state object in the synchronous disconnect path — our earlier "latent UAF" phrasing is retracted (no freed-object access was observed or proven). Covers USB and SDIO variants. --- drivers/aic8800/aic8800_fdrv/rwnx_main.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/aic8800/aic8800_fdrv/rwnx_main.c b/drivers/aic8800/aic8800_fdrv/rwnx_main.c index 9180fe2..8881ccb 100644 --- a/drivers/aic8800/aic8800_fdrv/rwnx_main.c +++ b/drivers/aic8800/aic8800_fdrv/rwnx_main.c @@ -1666,9 +1666,16 @@ static int rwnx_close(struct net_device *dev) rwnx_ipc_tx_drain(rwnx_hw); #else #if defined(AICWF_USB_SUPPORT) - if (usbdev->bus_if->state != BUS_DOWN_ST && usbdev->state != USB_DOWN_ST) + /* 002 在 deinit 时摘 drvdata，disconnect→close 时序下本函数读到 + * usbdev=NULL：实测断开在此裸解引用 usbdev->bus_if 崩溃 + * （rwnx_close 偏移处 reloc=rwnx_send_reset，CR2=0x8）。 + * 上游 usbdev 为悬挂非空、此条件求值为假而侥幸跳过——本判空 + * 对两种情形给出同一语义：bus 已断 → 跳过 reset/重配。 */ + if (usbdev != NULL && usbdev->bus_if != NULL && + usbdev->bus_if->state != BUS_DOWN_ST && usbdev->state != USB_DOWN_ST) #else - if (sdiodev->bus_if->state != BUS_DOWN_ST) + if (sdiodev != NULL && sdiodev->bus_if != NULL && + sdiodev->bus_if->state != BUS_DOWN_ST) #endif { rwnx_send_reset(rwnx_hw); From 3facbaf006007c5efeffd5c28f1a472f330847cc Mon Sep 17 00:00:00 2001 From: xuxinchun1003 Date: Wed, 7 Oct 2026 07:54:47 +0800 Subject: [PATCH 3/3] fix: NULL-guard txmsg/txdata drvdata readers (defense in depth) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reader families do `bus_if = dev_get_drvdata(dev)` and immediately dereference bus_if->bus_priv.usb. With the ordering from the parent commit the corresponding threads (cmd tx, bustx) are quiesced before drvdata is cleared, so these guards should be unreachable in the current call graph — explicit belt-and-suspenders per the #103 reader audit, covering both CONFIG_USB_TX_AGGR txdata variants and txmsg against any future or premature caller. --- drivers/aic8800/aic8800_fdrv/aicwf_usb.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/aic8800/aic8800_fdrv/aicwf_usb.c b/drivers/aic8800/aic8800_fdrv/aicwf_usb.c index 16000d7..04e128a 100644 --- a/drivers/aic8800/aic8800_fdrv/aicwf_usb.c +++ b/drivers/aic8800/aic8800_fdrv/aicwf_usb.c @@ -1409,6 +1409,11 @@ static int aicwf_usb_bus_txmsg(struct device *dev, u8 *buf, u32 len) { int ret = 0; struct aicwf_bus *bus_if = dev_get_drvdata(dev); + /* Defense in depth (review #103): with the parent commit's ordering the + * cmd thread is quiesced before drvdata is cleared, so this should be + * unreachable — guard anyway against a future/premature caller. */ + if (bus_if == NULL) + return -ENODEV; struct aic_usb_dev *usb_dev = bus_if->bus_priv.usb; if (usb_dev->state != USB_UP_ST) @@ -1667,6 +1672,10 @@ static int aicwf_usb_bus_txdata(struct device *dev, struct sk_buff *pkt) uint prio; int ret = -EBADE; struct aicwf_bus *bus_if = dev_get_drvdata(dev); + /* Defense in depth (review #103): usb_bustx_thread is joined before + * drvdata is cleared; guard any other/premature caller. */ + if (bus_if == NULL) + return -ENODEV; struct aic_usb_dev *usbdev = bus_if->bus_priv.usb; //printk("%s\n", __func__); @@ -1703,6 +1712,10 @@ static int aicwf_usb_bus_txdata(struct device *dev, struct sk_buff *skb) int ret = 0; unsigned long flags; struct aicwf_bus *bus_if = dev_get_drvdata(dev); + /* Defense in depth (review #103): usb_bustx_thread is joined before + * drvdata is cleared; guard any other/premature caller. */ + if (bus_if == NULL) + return -ENODEV; struct aic_usb_dev *usb_dev = bus_if->bus_priv.usb; struct rwnx_txhdr *txhdr = (struct rwnx_txhdr *)skb->data; struct rwnx_hw *rwnx_hw = usb_dev->rwnx_hw;