[PATCH net 0/3] net: ethernet: disable async work before teardown

[PATCH net 0/3] net: ethernet: disable async work before teardown

[BeaCox]

Three Ethernet drivers free the netdev while an asynchronously queued
work item can still dereference it.  The queue sites and the handlers
differ, but the lifetime rule is the same: stop the producer, drain the
work, then free the object.

Patch 1 fixes atl1.  reset_dev_task is queued from the interrupt handler
for link-down and DMA errors and from ndo_tx_timeout(); link_chg_task is
queued from the interrupt handler when ISR_GPHY reports a link event.
Both works are disabled and the PHY configuration timer is deleted
before unregister_netdev().

Patch 2 fixes enc28j60.  tx_work is queued from ndo_start_xmit(),
setrx_work from ndo_set_rx_mode(), and restart_work from ndo_tx_timeout().
The interface is unregistered first, then the three works are disabled
before free_irq() and free_netdev().

Patch 3 fixes xgmac.  tx_timeout_work is queued from ndo_tx_timeout()
and from the DMA error interrupt.  The work starts disabled in probe,
is enabled by xgmac_open()/xgmac_resume(), disabled in xgmac_stop()
before the DMA rings are freed and in xgmac_suspend() before NAPI and
interrupts are torn down, and disabled in xgmac_remove() before the
IRQs and MMIO are released.  The patch does not reorder the existing
free_irq()/unregister_netdev() calls in xgmac_remove().

The works are disabled with disable_work_sync() rather than cancelled:
it drains a pending or running instance and also rejects later
queue_work() calls from interrupts or timers that are still in flight.
This API was added in v6.10, so the stable tag is 6.10+; older trees
need the cancel_work_sync() form of the same change.

The three patches are independent and can be applied separately.  They
are grouped only because they fix the same teardown pattern.

Testing: the in-tree driver code was run under KASAN on QEMU/KVM (x86-64,
2 vCPUs).  A SCHED_FIFO task on one CPU queues the work and keeps the
per-CPU system workqueue from running while a second CPU unbinds the
device; the vulnerable kernels report a slab-use-after-free in
assign_work() and the fixed kernels complete with the work still
pending at remove entry.  The three patched drivers also build as
modules with W=1.  There is no physical hardware in the test setup: the
platform/SPI teardown paths are exercised with test devices, and the
atl1 probe path is not exercised.

BeaCox (3):
  net: atl1: disable work before freeing the netdev
  net: enc28j60: disable work before freeing the netdev
  net: xgmac: disable timeout work before teardown

 drivers/net/ethernet/atheros/atlx/atl1.c  | 12 +++++++++
 drivers/net/ethernet/calxeda/xgmac.c      | 30 +++++++++++++++++++++++
 drivers/net/ethernet/microchip/enc28j60.c |  9 +++++++
 3 files changed, 51 insertions(+)

-- 
2.43.0

[PATCH net 1/3] net: atl1: disable work before freeing the netdev

[BeaCox]

atl1_remove() frees the netdev without disabling reset_dev_task or
link_chg_task.  reset_dev_task is queued from the interrupt handler for
link-down and DMA errors and from atlx_tx_timeout(); link_chg_task is
queued from the interrupt handler when ISR_GPHY reports a link event.
Both callbacks dereference the adapter and the netdev, so a work that is
still pending or running after free_netdev() touches freed memory.

The original driver never cancelled these works, and the current
reset_dev_task was introduced when the two reset tasks were merged in
03662e41c7cf ("atl1: fix kernel panic in case of DMA errors").

Disable both works synchronously before unregistering the interface,
then delete the PHY configuration timer.  disable_work_sync() drains a
queued or running instance and rejects a later queue_work(), which a
plain cancel_work_sync() would not.  Deleting the timer after the works
have stopped also prevents a running link work from rearming it.
atl2_remove() cancels both works and deletes the timer before
unregistering for the same reason.

Fixes: f3cc28c79760 ("Add Attansic L1 ethernet driver.")
Cc: Chris Snook <chris.snook@gmail.com>
Cc: stable@vger.kernel.org # 6.10+
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 2883e9462f..2bd35c3cfc 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -3132,6 +3132,18 @@ static void atl1_remove(struct pci_dev *pdev)
 
 	adapter = netdev_priv(netdev);
 
+	/*
+	 * reset_dev_task is queued from the interrupt handler and from the
+	 * tx watchdog; link_chg_task is queued from the interrupt handler
+	 * when ISR_GPHY reports a link event.  Disable both before
+	 * unregistering the interface so that neither a pending nor a late
+	 * queue attempt can run after free_netdev().
+	 */
+	disable_work_sync(&adapter->reset_dev_task);
+	disable_work_sync(&adapter->link_chg_task);
+	timer_delete_sync(&adapter->phy_config_timer);
+	adapter->phy_timer_pending = false;
+
 	/*
 	 * Some atl1 boards lack persistent storage for their MAC, and get it
 	 * from the BIOS during POST.  If we've been messing with the MAC
-- 
2.43.0

[PATCH net 2/3] net: enc28j60: disable work before freeing the netdev

[BeaCox]

enc28j60_remove() unregisters the interface, frees the IRQ and frees the
netdev, but tx_work, setrx_work and restart_work can still be pending or
running.  tx_work is queued from enc28j60_send_packet(), setrx_work from
enc28j60_set_multicast_list(), and restart_work from
enc28j60_tx_timeout().  The handlers access the private data, the SPI
device and the netdev, all of which are freed with the netdev.

Unregister the interface first so the xmit and set_rx_mode paths can no
longer queue work, then disable the three works synchronously before
freeing the IRQ and the netdev.  disable_work_sync() drains a queued or
running instance and rejects a later queue_work(), which a plain
cancel_work_sync() would not.

Fixes: 3ec9c11da033 ("add driver for enc28j60 ethernet chip")
Cc: stable@vger.kernel.org # 6.10+
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index d6c9491537..bb29a94a1e 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -1600,7 +1600,16 @@ static void enc28j60_remove(struct spi_device *spi)
 {
 	struct enc28j60_net *priv = spi_get_drvdata(spi);
 
+	/*
+	 * Stop the interface first so the xmit and set_rx_mode paths can no
+	 * longer queue new work.  Then drain the works that were already
+	 * queued; the IRQ handler does not queue work, so the IRQ can be
+	 * released after the drain and before free_netdev().
+	 */
 	unregister_netdev(priv->netdev);
+	disable_work_sync(&priv->tx_work);
+	disable_work_sync(&priv->setrx_work);
+	disable_work_sync(&priv->restart_work);
 	free_irq(spi->irq, priv);
 	free_netdev(priv->netdev);
 }
-- 
2.43.0

[PATCH net 3/3] net: xgmac: disable timeout work before teardown

[BeaCox]

xgmac_remove() frees the IRQ lines and unmaps the registers before
freeing the netdev, but tx_timeout_work can still be pending or running.
The work is queued from ndo_tx_timeout() and from the DMA error
interrupt path, and its handler disables NAPI, programs the DMA
registers and walks the TX ring.

xgmac_stop() has the same lifetime problem: it frees the DMA descriptor
rings while a queued timeout work can still run.

Disable the work in probe after INIT_WORK() so open/resume can enable it,
disable it synchronously in xgmac_stop() before the rings are freed and
in xgmac_suspend() before NAPI/interrupts are torn down, enable it in
xgmac_open() and xgmac_resume(), and disable it in xgmac_remove() before
the IRQs and MMIO are released.  The enable/disable pairing avoids the
underflow warning that enable_work() emits when the work was never
disabled.  disable_work_sync() also rejects queue_work() from a late
interrupt, which cancellation alone would not.

The patch does not change the existing free_irq()/unregister_netdev()
ordering in xgmac_remove(); it only prevents the timeout work from
outliving the netdev.

Fixes: 8746f671ef04 ("net: calxedaxgmac: fix race between xgmac_tx_complete and xgmac_tx_err")
Cc: stable@vger.kernel.org # 6.10+
Signed-off-by: BeaCox <0xbeac@gmail.com>
---
 drivers/net/ethernet/calxeda/xgmac.c | 30 ++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index a2410fba6b..9b7aee56f2 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1023,6 +1023,9 @@ static int xgmac_open(struct net_device *dev)
 	if (ret < 0)
 		return ret;
 
+	/* xgmac_stop() disables this work before freeing the rings. */
+	enable_work(&priv->tx_timeout_work);
+
 	/* Enable the MAC Rx/Tx */
 	xgmac_mac_enable(ioaddr);
 
@@ -1046,6 +1049,13 @@ static int xgmac_stop(struct net_device *dev)
 {
 	struct xgmac_priv *priv = netdev_priv(dev);
 
+	/*
+	 * The timeout work programs the DMA rings that are freed below, and
+	 * it toggles NAPI.  Disable it synchronously before touching either;
+	 * xgmac_open() and xgmac_resume() enable it again.
+	 */
+	disable_work_sync(&priv->tx_timeout_work);
+
 	if (readl(priv->base + XGMAC_DMA_INTR_ENA))
 		napi_disable(&priv->napi);
 
@@ -1713,6 +1723,12 @@ static int xgmac_probe(struct platform_device *pdev)
 	ndev->ethtool_ops = &xgmac_ethtool_ops;
 	spin_lock_init(&priv->stats_lock);
 	INIT_WORK(&priv->tx_timeout_work, xgmac_tx_timeout_work);
+	/*
+	 * The work is enabled while the interface is up.  Start it disabled
+	 * so the enable_work() in xgmac_open()/xgmac_resume() has a matching
+	 * disable and never underflows.
+	 */
+	disable_work(&priv->tx_timeout_work);
 
 	priv->device = &pdev->dev;
 	priv->dev = ndev;
@@ -1823,6 +1839,13 @@ static void xgmac_remove(struct platform_device *pdev)
 	struct xgmac_priv *priv = netdev_priv(ndev);
 	struct resource *res;
 
+	/*
+	 * The timeout work can be queued from ndo_tx_timeout() and from the
+	 * DMA error interrupt.  Disable it before the IRQs are released and
+	 * the registers are unmapped.
+	 */
+	disable_work_sync(&priv->tx_timeout_work);
+
 	xgmac_mac_disable(priv->base);
 
 	/* Free the IRQ lines */
@@ -1862,6 +1885,12 @@ static int xgmac_suspend(struct device *dev)
 		return 0;
 
 	netif_device_detach(ndev);
+	/*
+	 * The timeout work touches the DMA registers and toggles NAPI.
+	 * Disable it before NAPI and the interrupts are torn down;
+	 * xgmac_resume() enables it again.
+	 */
+	disable_work_sync(&priv->tx_timeout_work);
 	napi_disable(&priv->napi);
 	writel(0, priv->base + XGMAC_DMA_INTR_ENA);
 
@@ -1891,6 +1920,7 @@ static int xgmac_resume(struct device *dev)
 
 	/* Enable the MAC and DMA */
 	xgmac_mac_enable(ioaddr);
+	enable_work(&priv->tx_timeout_work);
 	writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS);
 	writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA);
 
-- 
2.43.0

Re: [PATCH net 0/3] net: ethernet: disable async work before teardown

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net 0/3] net: ethernet: disable async work before teardown

[BeaCox]

Hi,

The issues were found by manual code inspection while auditing
Ethernet remove paths for async work that can outlive the netdev (the
same teardown pattern as a missing cancel/disable_work_sync before
free_netdev). They were not hit in production and are not from syzbot.

Testing is as described in the cover letter: KASAN on QEMU/KVM
(x86-64, 2 vCPUs). There is no physical atl1 / enc28j60 / xgmac
hardware in the test setup; platform/SPI teardown was exercised with
test devices, and the atl1 probe path was not exercised.

If a later revision is needed for other reasons I will fold this into
the commit messages.

Thanks,
BeaCox

On Wed, Nov 4, 2026 at 6:38 AM netdev-bot+sinfo@kernel.org wrote:

> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net 0/3] net: ethernet: disable async work before teardown

[Andrew Lunn]

On Wed, Nov 04, 2026 at 02:32:42PM +0800, BeaCox wrote:
> Three Ethernet drivers free the netdev while an asynchronously queued
> work item can still dereference it.  The queue sites and the handlers
> differ, but the lifetime rule is the same: stop the producer, drain the
> work, then free the object.

No need for a wall of text nobody is going to read. The commit message
should be terse. Please tell your LLM that.

Do any of these problems bother anybody?

If not, please target net-next.


    Andrew

---
pw-bot: cr