[PATCH v2] net: natsemi: ns83820: Fix use-after-free in ns83820_remove_one due to race condition

[PATCH v2] net: natsemi: ns83820: Fix use-after-free in ns83820_remove_one due to race condition

[Pei Xiao]

In ns83820_init_one, &dev->tq_refill is bound with queue_refill, and
ns83820_rx_kick can schedule this work on system_wq when it is called
from the IRQ handler ns83820_irq (via ns83820_do_isr) or from the rx
tasklet rx_action.

If we remove the device, ns83820_remove_one makes cleanup and the
memory allocated for dev with netdev_priv() is released by
free_netdev(), while the work mentioned above may still be pending or
running. The sequence of operations that may lead to a UAF bug is as
follows:

CPU0                                      CPU1

                                          | ns83820_irq
                                          | ns83820_do_isr
                                          | ns83820_rx_kick
                                          | schedule_work(&dev->tq_refill)
ns83820_remove_one                        |
ns83820_disable_interrupts(dev)           |
unregister_netdev(ndev)                   |
free_irq(dev->pci_dev->irq, ndev)         |
iounmap(dev->base)                        |
dma_free_coherent(...)                    |
free_netdev(ndev)                         |
// dev is freed                           |
                                          | queue_refill
                                          | // use dev (use-after-free)

free_irq() only prevents the IRQ handler from running again. An rx
tasklet that was already scheduled by a previous interrupt can still
run afterwards, and rx_action calls ns83820_rx_kick, which can
re-schedule tq_refill on system_wq. This leaves a window where
queue_refill can still run after free_netdev() has freed dev.

Fix it by stopping the sources that can schedule the work, in order:
unregister_netdev() and free_irq() stop the IRQ handler, tasklet_kill()
waits for the rx tasklet to finish, and cancel_work_sync() then drains
any work that was queued before proceeding with the remaining cleanup
in ns83820_remove_one.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: Codex:deepseek-v4-flash
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v2: add tasklet_kill 
---
 drivers/net/ethernet/natsemi/ns83820.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index cdbf82affa7b..022fe0d29cb8 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -2209,6 +2209,10 @@ static void ns83820_remove_one(struct pci_dev *pci_dev)
 
 	unregister_netdev(ndev);
 	free_irq(dev->pci_dev->irq, ndev);
+
+	tasklet_kill(&dev->rx_tasklet);
+	cancel_work_sync(&dev->tq_refill);
+
 	iounmap(dev->base);
 	dma_free_coherent(&dev->pci_dev->dev, 4 * DESC_SIZE * NR_TX_DESC,
 			  dev->tx_descs, dev->tx_phy_descs);
-- 
2.25.1

Re: [PATCH v2] net: natsemi: ns83820: Fix use-after-free in ns83820_remove_one due to race condition

[Andrew Lunn]

On Tue, Aug 11, 2026 at 10:51:47AM +0800, Pei Xiao wrote:
> In ns83820_init_one, &dev->tq_refill is bound with queue_refill, and
> ns83820_rx_kick can schedule this work on system_wq when it is called
> from the IRQ handler ns83820_irq (via ns83820_do_isr) or from the rx
> tasklet rx_action.
> 
> If we remove the device, ns83820_remove_one makes cleanup and the
> memory allocated for dev with netdev_priv() is released by
> free_netdev(), while the work mentioned above may still be pending or
> running. The sequence of operations that may lead to a UAF bug is as
> follows:
> 
> CPU0                                      CPU1
> 
>                                           | ns83820_irq
>                                           | ns83820_do_isr
>                                           | ns83820_rx_kick
>                                           | schedule_work(&dev->tq_refill)
> ns83820_remove_one                        |
> ns83820_disable_interrupts(dev)           |
> unregister_netdev(ndev)                   |
> free_irq(dev->pci_dev->irq, ndev)         |
> iounmap(dev->base)                        |
> dma_free_coherent(...)                    |
> free_netdev(ndev)                         |
> // dev is freed                           |
>                                           | queue_refill
>                                           | // use dev (use-after-free)
> 
> free_irq() only prevents the IRQ handler from running again. An rx
> tasklet that was already scheduled by a previous interrupt can still
> run afterwards, and rx_action calls ns83820_rx_kick, which can
> re-schedule tq_refill on system_wq. This leaves a window where
> queue_refill can still run after free_netdev() has freed dev.
> 
> Fix it by stopping the sources that can schedule the work, in order:
> unregister_netdev() and free_irq() stop the IRQ handler, tasklet_kill()
> waits for the rx tasklet to finish, and cancel_work_sync() then drains
> any work that was queued before proceeding with the remaining cleanup
> in ns83820_remove_one.

Is this change tested in any way?

   Andrew

Re: [PATCH v2] net: natsemi: ns83820: Fix use-after-free in ns83820_remove_one due to race condition

[Jakub Kicinski]

On Tue, 11 Aug 2026 10:51:47 +0800 Pei Xiao wrote:
> Subject: [PATCH v2] net: natsemi: ns83820: Fix use-after-free in ns83820_remove_one due to race condition

Please stop sending those pointless fixes to long unused drivers.