[PATCH net v3 0/2] net: stmmac: Fix XSK crashes on stm32mp2

[PATCH net v3 0/2] net: stmmac: Fix XSK crashes on stm32mp2

[Kurt Kanzenbach]

Hi,

while looking at the TBS thingy [1], I've noticed kernel crashes. That
happens when opening or closing an AF_XDP/ZC socket while parallel Tx
traffic is going on.

So far, I see two issues:

 - Patch #1: stmmac_xdp_release() seems to have the teardown order wrong
 - Patch #2: stmmac_xdp_(en|dis)able_pool() does not stop the Tx queue
   at all

The kernel crashes can be reproduced instantly on the stm32mp2 like this:

 - Run iperf3
 - Run application which opens an AF_XDP/ZC socket

[1] - https://lore.kernel.org/all/20260812-stm32mp2_txtime-v1-1-f9e2462cc85d@linutronix.de/

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
Changes in v3:
- Remove txq_trans_cond_update() (Maciej)
- Add queue stop check for XDP TX etc. (Nicolai)
- Add tags (Nicolai and Maxime)
- Link to v2: https://patch.msgid.link/20261005-stmmac_xsk_crashes-v2-0-46c60cba6421@linutronix.de

Changes in v2:
- Remove RFC. No comments.
- Link to v1: https://patch.msgid.link/20260915-stmmac_xsk_crashes-v1-0-e14553fcf553@linutronix.de

---
Kurt Kanzenbach (2):
      net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()
      net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c  | 13 +++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)
---
base-commit: 8e2fbe0e75d38690d551456658194c632933cb57
change-id: 20260914-stmmac_xsk_crashes-10a77f614ab5

Best regards,
--  
Kurt Kanzenbach <kurt@linutronix.de>

[PATCH net v3 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()

[Kurt Kanzenbach]

Attaching an XDP program while Tx traffic is running results in kernel
crashes in stmmac_xmit() -> dwmac4_set_addr().

Loading an XDP program tears down and reallocates all DMA resources via
stmmac_xdp_release() and stmmac_xdp_open(). stmmac_xdp_release() stops
the Tx queues before disabling NAPI:

 stmmac_xdp_release:
   netif_tx_disable
   stmmac_disable_all_queues
   ...
   free_dma_desc_resources

A Tx NAPI poll may still be in flight at that point. stmmac_tx_clean()
takes the Tx queue lock, reaps completed descriptors and wakes the queue
again when it observes it stopped with enough descriptors available.
Nothing stops the queue afterwards, so the Tx path resumes while
free_dma_desc_resources() releases the descriptor rings underneath it.

On non-coherent platforms dma_free_coherent() tears down the vmalloc
mapping of the descriptors, so the subsequent stmmac_xmit() faults on an
unmapped address instead of corrupting memory silently.

Disable NAPI first and stop the Tx queues afterwards, which is the order
already used by __stmmac_release().

The issue can be easily reproduced by:

 1. Run iperf
 2. Run application which opens an AF_XDP/ZC socket

Assisted-by: Claude:claude-opus-5
Fixes: 77711683a504 ("net: stmmac: ensure tx function is not running in stmmac_xdp_release()")
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # stm32mp215
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b837e8e27a35..2aa5a2695473 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7132,15 +7132,15 @@ void stmmac_xdp_release(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	u8 chan;
 
-	/* Ensure tx function is not running */
-	netif_tx_disable(dev);
-
 	/* Disable NAPI process */
 	stmmac_disable_all_queues(priv);
 
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
 
+	/* Ensure tx function is not running */
+	netif_tx_disable(dev);
+
 	/* Free the IRQ lines */
 	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
 

-- 
2.47.3

[PATCH net v3 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

[Kurt Kanzenbach]

When enabling or disabling XSK pools in parallel to Tx traffic, kernel
crashes occur. For VLAN tagged frames that happens in stmmac_xmit() ->
dwmac4_set_vlan_tag() and for normal frames in stmmac_xmit() ->
dwmac4_set_addr(). Both of these functions access the Tx DMA descriptors.

The XDP pool (en|dis)ablement frees and reallocates the Tx DMA resources:

 stmmac_disable_tx_queue:
   __free_dma_tx_desc_resources

 stmmac_enable_tx_queue:
   __alloc_dma_tx_desc_resources
   __init_dma_tx_desc_rings

NAPI is disabled during that allocation window, but the Tx queue is not
stopped. Therefore, add the stopping of the Tx queue during the enabling
and disabling of XSK pools.

The issue can be easily reproduced by:

 1. Run iperf
 2. Run application which opens an AF_XDP/ZC socket

Also add a dedicated queue check for the XDP Tx paths.

Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
Suggested-by: Nicolai Buchwitz <nb@tipi-net.de> # XDP Tx checks
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c  | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2aa5a2695473..0e8ab2bd8f3e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5216,6 +5216,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 	dma_addr_t dma_addr;
 	bool set_ic;
 
+	/* Ring may be torn down for an XSK pool switch */
+	if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
+		return STMMAC_XDP_CONSUMED;
+
 	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
 		return STMMAC_XDP_CONSUMED;
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
index d7e4db7224b0..a75ab93aef53 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
@@ -6,6 +6,15 @@
 #include "stmmac.h"
 #include "stmmac_xdp.h"
 
+static void stmmac_xdp_stop_tx_queue(struct stmmac_priv *priv, u16 queue)
+{
+	struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
+
+	__netif_tx_lock_bh(nq);
+	netif_tx_stop_queue(nq);
+	__netif_tx_unlock_bh(nq);
+}
+
 static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
 				  struct xsk_buff_pool *pool, u16 queue)
 {
@@ -36,6 +45,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
 	if (need_update) {
 		napi_disable(&ch->rx_napi);
 		napi_disable(&ch->tx_napi);
+		stmmac_xdp_stop_tx_queue(priv, queue);
 		stmmac_disable_rx_queue(priv, queue);
 		stmmac_disable_tx_queue(priv, queue);
 	}
@@ -46,6 +56,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
 		stmmac_enable_rx_queue(priv, queue);
 		stmmac_enable_tx_queue(priv, queue);
 		napi_enable(&ch->rxtx_napi);
+		netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
 
 		err = stmmac_xsk_wakeup(priv->dev, queue, XDP_WAKEUP_RX);
 		if (err)
@@ -73,6 +84,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
 
 	if (need_update) {
 		napi_disable(&ch->rxtx_napi);
+		stmmac_xdp_stop_tx_queue(priv, queue);
 		stmmac_disable_rx_queue(priv, queue);
 		stmmac_disable_tx_queue(priv, queue);
 		synchronize_rcu();
@@ -87,6 +99,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
 		stmmac_enable_tx_queue(priv, queue);
 		napi_enable(&ch->rx_napi);
 		napi_enable(&ch->tx_napi);
+		netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));
 	}
 
 	return 0;

-- 
2.47.3

Re: [PATCH net v3 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

[Nicolai Buchwitz]

Hi Kurt

On 13.10.2026 10:57, Kurt Kanzenbach wrote:
> When enabling or disabling XSK pools in parallel to Tx traffic, kernel
> crashes occur. For VLAN tagged frames that happens in stmmac_xmit() ->
> dwmac4_set_vlan_tag() and for normal frames in stmmac_xmit() ->
> dwmac4_set_addr(). Both of these functions access the Tx DMA 
> descriptors.
> 
> The XDP pool (en|dis)ablement frees and reallocates the Tx DMA 
> resources:
> 
>  stmmac_disable_tx_queue:
>    __free_dma_tx_desc_resources
> 
>  stmmac_enable_tx_queue:
>    __alloc_dma_tx_desc_resources
>    __init_dma_tx_desc_rings
> 
> NAPI is disabled during that allocation window, but the Tx queue is not
> stopped. Therefore, add the stopping of the Tx queue during the 
> enabling
> and disabling of XSK pools.
> 
> The issue can be easily reproduced by:
> 
>  1. Run iperf
>  2. Run application which opens an AF_XDP/ZC socket
> 
> Also add a dedicated queue check for the XDP Tx paths.
> 
> Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
> Suggested-by: Nicolai Buchwitz <nb@tipi-net.de> # XDP Tx checks
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>

> [...]

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # stm32mp215

Thanks,
Nicolai

Re: [PATCH net v3 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: hardware UAF due to unconditional flush on stopped queue
--

commit ef870c171b8f7f53a01cef2bdd086a9bc0f02d35
Author: Kurt Kanzenbach <kurt@linutronix.de>

net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

When enabling or disabling XSK pools in parallel to Tx traffic, kernel
crashes occur. This patch adds stopping of the Tx queue during the enabling
and disabling of XSK pools to prevent accessing freed Tx DMA descriptors.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2aa5a26954731..0e8ab2bd8f3e8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5216,6 +5216,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>  	dma_addr_t dma_addr;
>  	bool set_ic;
>  
> +	/* Ring may be torn down for an XSK pool switch */
> +	if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
> +		return STMMAC_XDP_CONSUMED;
> +

[Severity: High]
Could this unconditional flush cause a hardware use-after-free regression?

While this protects stmmac_xdp_xmit_xdpf() from accessing descriptors when the
queue is stopped, stmmac_xdp_xmit() calls it in a loop and then unconditionally
flushes if XDP_XMIT_FLUSH is set:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_xdp_xmit() {
    ...
	for (i = 0; i < num_frames; i++) {
		...
		res = stmmac_xdp_xmit_xdpf(priv, queue, frames[i], true);
		if (res == STMMAC_XDP_CONSUMED)
			break;
		nxmit++;
	}

	if (flags & XDP_XMIT_FLUSH) {
		stmmac_flush_tx_descriptors(priv, queue);
		stmmac_tx_timer_arm(priv, queue);
	}
    ...
}

If stmmac_xdp_xmit() is called with XDP_XMIT_FLUSH during an AF_XDP pool switch,
it will break out of the loop but still call stmmac_flush_tx_descriptors().

Since stmmac_flush_tx_descriptors() calls stmmac_set_queue_tx_tail_ptr(), could
this write a stale physical address to the hardware DMA tail pointer while
stmmac_enable_tx_queue() is allocating new descriptors, causing the DMA engine
to fetch from freed memory?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-stmmac_xsk_crashes-v3-0-50fca7790af2@linutronix.de?part=2

Re: [PATCH net v3 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 1 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch adds a new race in stmmac_xdp_release()
  (drivers/net/ethernet/stmicro/stmmac/stmmac_main.c).
- [Medium] The fix still leaves a waker that runs after
  netif_tx_disable().
- [Low] Concern: the same Tx-ring free during an active stmmac_xmit() is
  reachable through stmmac_xdp_enable_pool()/stmmac_xdp_disable_pool().

Pre-existing issues:
- [Low] Concern: stmmac_xdp_xmit() (ndo_xdp_xmit), driven by remote
  devmap flushes, ignores the queue-stopped state and only checks…

--

net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release()

This patch reorders stmmac_xdp_release() so that NAPI is disabled and
the Tx coalescing hrtimers are cancelled before netif_tx_disable() runs.
The goal is to stop an in-flight Tx NAPI poll from waking the Tx queue
via stmmac_tx_clean() while free_dma_desc_resources() releases the
descriptor rings.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b837e8e27a352..2aa5a26954731 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7132,15 +7132,15 @@ void stmmac_xdp_release(struct net_device *dev)
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	u8 chan;
>  
> -	/* Ensure tx function is not running */
> -	netif_tx_disable(dev);
> -
>  	/* Disable NAPI process */
>  	stmmac_disable_all_queues(priv);
>  
>  	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>  		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>  
> +	/* Ensure tx function is not running */
> +	netif_tx_disable(dev);

[Severity: High]

hrtimer_cancel() now runs before netif_tx_disable(). Can a concurrent
stmmac_xmit() or stmmac_tso_xmit() re-arm txtimer after it has been
cancelled?

Both xmit paths end in stmmac_tx_timer_arm(). The check and the start
there are not serialized against napi_disable() or hrtimer_cancel():

stmmac_tx_timer_arm() {
    ...
	if (unlikely(!napi_is_scheduled(napi))) {
		if (unlikely(!(hrtimer_active(&tx_q->txtimer))))
			hrtimer_start(&tx_q->txtimer,
				      STMMAC_COAL_TIMER(tx_coal_timer),
				      HRTIMER_MODE_REL);
    ...
}

One possible interleaving:

CPU1 (stmmac_xmit, holds txq lock)    CPU0 (stmmac_xdp_release)
napi_is_scheduled() == false
                                      stmmac_disable_all_queues()
                                      hrtimer_cancel() (timer inactive)
hrtimer_active() == false
hrtimer_start()
                                      netif_tx_disable()
                                        only waits for the txq lock

The timer then stays armed. The default is 5000us, and ethtool allows
up to 100000us.

stmmac_xdp_set_prog() then calls stmmac_xdp_open(), which sets up the
timer again without cancelling it first:

stmmac_xdp_open() {
    ...
		hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    ...
}

__hrtimer_setup() does memset(timer, 0, sizeof(struct hrtimer)). Could
that corrupt the per-CPU hrtimer base rbtree while the node is still
enqueued?

Before this patch, netif_tx_disable() ran first, so no regular xmit
could run during the cancel loop. The later patch in this series does
not touch stmmac_xdp_release(), so the window seems to remain at the
end of the series.

Would moving the hrtimer_cancel() loop after netif_tx_disable() close
this race and still keep NAPI disabled before Tx is stopped?

> +
>  	/* Free the IRQ lines */
>  	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);

[Severity: Medium]

Can the error interrupt path still wake the Tx queue after
netif_tx_disable()?

The IRQ handlers stay registered until stmmac_free_irq().
stmmac_xdp_set_prog() never sets STMMAC_DOWN, so the STMMAC_DOWN checks
in stmmac_interrupt() and stmmac_msi_intr_tx() let these interrupts
through.

On tx_hard_error, stmmac_dma_interrupt() and stmmac_msi_intr_tx() call
stmmac_tx_err(), which ends with:

stmmac_tx_err() {
    ...
	priv->xstats.tx_errors++;
	netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, chan));
}

free_irq() only waits for a running handler to finish. Nothing stops
the queue again before stmmac_stop_all_dma() and
free_dma_desc_resources().

Could stmmac_xmit() then run on rings that are being freed and hit the
same dwmac4_set_addr() fault?

The netif_tx_queue_stopped() check that the later patch in the series
adds to stmmac_xdp_xmit_xdpf() would not help here, because the queue
has already been woken.

The commit message says:

  Nothing stops the queue afterwards, so the Tx path resumes while
  free_dma_desc_resources() releases the descriptor rings underneath it.

The reordering only handles the NAPI waker, though. Would stopping Tx
after stmmac_free_irq(), or having stmmac_tx_err() skip the wake during
teardown, cover this case as well?

[Severity: Low]

This is a pre-existing issue and was not introduced by this patch. Can
stmmac_xdp_xmit() (ndo_xdp_xmit), driven by remote devmap flushes,
write into rings freed by free_dma_desc_resources() during an XDP
detach?

At this commit, stmmac_xdp_xmit() ignores the queue stopped state and
only checks STMMAC_DOWN, which the XDP release path never sets. So it
could write tx_q->xdpf[] and descriptors into the freed rings.

The later patch "net: stmmac: Stop Tx queue when (en|dis)abling XSK
pools" adds this check at the start of stmmac_xdp_xmit_xdpf():

	/* Ring may be torn down for an XSK pool switch */
	if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
		return STMMAC_XDP_CONSUMED;

stmmac_xdp_xmit() calls that function under __netif_tx_lock(nq), the
same per-queue lock that netif_tx_disable() takes. After
netif_tx_disable() has run, later ndo_xdp_xmit calls are rejected
before they touch descriptor memory.

That still leaves the case where the queue is woken again through
stmmac_tx_err(), described above.

[Severity: Low]

stmmac_xdp_enable_pool() and stmmac_xdp_disable_pool() in stmmac_xdp.c
seem to have the same problem. Both call stmmac_disable_tx_queue(),
which frees the Tx ring without stopping the netdev Tx queue:

stmmac_disable_tx_queue() {
    ...
	stmmac_stop_tx_dma(priv, queue);
	__free_dma_tx_desc_resources(priv, &priv->dma_conf, queue);
}

At this commit, can stmmac_xmit() fault on that freed descriptor
memory?

The next patch in the series, "net: stmmac: Stop Tx queue when
(en|dis)abling XSK pools", appears to fix this. In both functions it
calls stmmac_xdp_stop_tx_queue() before stmmac_disable_rx_queue() and
stmmac_disable_tx_queue(). It also wakes the queue again after
stmmac_enable_tx_queue().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013-stmmac_xsk_crashes-v3-0-50fca7790af2%40linutronix.de

Re: [PATCH net v3 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Both stmmac_xdp_enable_pool() (stmmac_xdp.c:57-59) and
  stmmac_xdp_disable_pool() (stmmac_xdp.c:99-102) now call…
- [Medium] The patch's only guard is the __QUEUE_STATE_DRV_XOFF bit, set
  by stmmac_xdp_stop_tx_queue().
- [Medium] The stopped-queue check in stmmac_xdp_xmit_xdpf() does not
  order the XDP sender against ring re-initialisation, for two reasons.
- [Low] In stmmac_xdp_xmit() (ndo_xdp_xmit, stmmac_main.c:7013-7026),
  when the queue is stopped for an XSK pool switch, the new check makes…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it sits on the
  error path the patch touches.

--

net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

This patch stops the Tx queue around the Tx ring teardown and
re-allocation in stmmac_xdp_enable_pool() and stmmac_xdp_disable_pool().
It wakes the queue again once the queue has been re-enabled. It also makes
stmmac_xdp_xmit_xdpf() refuse frames while the Tx queue is stopped.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2aa5a26954731..0e8ab2bd8f3e8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5216,6 +5216,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>  	dma_addr_t dma_addr;
>  	bool set_ic;
>  
> +	/* Ring may be torn down for an XSK pool switch */
> +	if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
> +		return STMMAC_XDP_CONSUMED;
> +

[Severity: Medium]

Does this check order the XDP sender against the ring re-initialisation?
The producer index is loaded in the declaration block, before the new test
runs:

    unsigned int entry = tx_q->cur_tx;

On the other side, stmmac_xdp_enable_pool() and stmmac_xdp_disable_pool()
call stmmac_enable_tx_queue() and then netif_tx_wake_queue() without
holding nq->_xmit_lock. The reset happens in
stmmac_enable_tx_queue()->stmmac_reset_tx_queue(), which sets cur_tx = 0
and dirty_tx = 0.

The XDP senders stmmac_xdp_xmit() and stmmac_xdp_xmit_back() hold
__netif_tx_lock(), but that lock does not exclude the waker. Could this
interleaving happen?

CPU A (XDP sender, tx lock held)   CPU B (pool switch)
entry = tx_q->cur_tx; /* 57 */
<delayed by hardirq or RT
 preemption>
                                   stmmac_enable_tx_queue()
                                     stmmac_reset_tx_queue()
                                       tx_q->cur_tx = 0;
                                   netif_tx_wake_queue()
netif_tx_queue_stopped() == false
stmmac_tx_avail() sees empty ring
writes descriptor 57 and xdpf[57]
tx_q->cur_tx = 58;

On weakly ordered CPUs, the plain cur_tx load can also be satisfied
before the relaxed test_bit().

Descriptor 0 has OWN = 0, so the DMA suspends there. stmmac_tx_clean()
also stops at entry 57. Would the queue then stall until the watchdog
resets the device?

Could the cur_tx load be moved after the stopped check? The wake could
also be done under __netif_tx_lock_bh(), the same way
stmmac_xdp_stop_tx_queue() does the stop.

[Severity: Low]

When this returns STMMAC_XDP_CONSUMED for the first frame,
stmmac_xdp_xmit() breaks out of its loop with nxmit == 0, but it still
runs the flush:

stmmac_xdp_xmit() {
    ...
	if (flags & XDP_XMIT_FLUSH) {
		stmmac_flush_tx_descriptors(priv, queue);
		stmmac_tx_timer_arm(priv, queue);
	}
    ...
}

stmmac_flush_tx_descriptors()->stmmac_set_queue_tx_tail_ptr() computes
tx_q->dma_tx_phy + cur_tx * desc_size and writes the result to the
channel's TX tail pointer register. stmmac_tx_timer_arm() reads
tx_q->xsk_pool.

At the same time, stmmac_disable_tx_queue() and stmmac_enable_tx_queue()
can be running on another CPU without the tx lock. They assign a new
dma_tx_phy, reset cur_tx, change xsk_pool and program the same registers.

dma_tx_phy and cur_tx live in the persistent tx_q, so no freed memory is
dereferenced. But can this tail write land after the enable path has
already programmed the new ring?

Should the flush only run when nxmit > 0, or when the queue is not
stopped?

>  	if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
>  		return STMMAC_XDP_CONSUMED;
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> index d7e4db7224b0c..a75ab93aef531 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> @@ -6,6 +6,15 @@
>  #include "stmmac.h"
>  #include "stmmac_xdp.h"
>  
> +static void stmmac_xdp_stop_tx_queue(struct stmmac_priv *priv, u16 queue)
> +{
> +	struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
> +
> +	__netif_tx_lock_bh(nq);
> +	netif_tx_stop_queue(nq);
> +	__netif_tx_unlock_bh(nq);
> +}

[Severity: Medium]

During the switch, the XOFF bit set here is the only thing keeping Tx
away from the ring. Can stmmac_tx_err() clear it in the middle of the
teardown?

stmmac_tx_err() runs in hardirq context, called from
stmmac_dma_interrupt() or stmmac_msi_intr_tx() when tx_hard_error is
reported. It ends with:

	stmmac_init_tx_chan(priv, priv->ioaddr, priv->plat->dma_cfg,
			    tx_q->dma_tx_phy, chan);
	stmmac_start_tx_dma(priv, chan);

	priv->xstats.tx_errors++;
	netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, chan));

It has no way of knowing that a pool switch is in progress. Before that,
it also calls dma_free_tx_skbufs() and stmmac_clear_tx_descriptors() on
the tx_q ring.

Nothing seems to quiesce that path during the switch:

- stmmac_disable_tx_queue() masks only the normal TX interrupt. There is
  no synchronize_irq() before __free_dma_tx_desc_resources().
- dwmac4_disable_dma_irq() clears only RIE/TIE, so abnormal interrupts
  stay enabled.
- stmmac_disable_rx_queue() stops RX DMA, which can raise RPS and
  therefore AIS.
- dwmac4_dma_interrupt() classifies TPS as tx_hard_error. TXSE is not in
  DMA_CHAN_INTR_DEFAULT_MASK, so a TPS latched by an earlier
  stmmac_stop_tx_dma() is never cleared.

Suppose tx_hard_error is reported for this channel inside the window.
Could stmmac_xmit() and stmmac_xdp_xmit_xdpf() then write into the freed
ring, with TX DMA restarted on freed descriptor memory?

Whether the hardware actually reports AIS together with a latched TPS in
that window depends on DWMAC behaviour.

[ ... ]

> @@ -46,6 +56,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
>  		stmmac_enable_rx_queue(priv, queue);
>  		stmmac_enable_tx_queue(priv, queue);
>  		napi_enable(&ch->rxtx_napi);
> +		netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));

[Severity: High]

Is it safe to wake the queue unconditionally here?
stmmac_enable_tx_queue() returns void. When
__alloc_dma_tx_desc_resources() fails, it only logs and returns:

stmmac_enable_tx_queue() {
	ret = __alloc_dma_tx_desc_resources(priv, &priv->dma_conf, queue);
	if (ret) {
		netdev_err(priv->dev, "Failed to alloc TX desc.\n");
		return;
	}
    ...
}

That skips stmmac_reset_tx_queue(), so cur_tx and dirty_tx keep their
old values and stmmac_tx_avail() still reports free space.

The ring was already torn down by
stmmac_disable_tx_queue()->__free_dma_tx_desc_resources(). That function
frees dma_tx/dma_etx/dma_entx, tx_skbuff_dma and tx_skbuff, but sets none
of these pointers to NULL. The resulting state depends on which allocation
failed:

- tx_skbuff_dma kzalloc fails: tx_skbuff_dma is NULL. tx_skbuff and
  dma_tx still point at freed memory.
- tx_skbuff kzalloc fails: tx_skbuff/xdpf is NULL. dma_tx still points
  at freed memory.
- dma_alloc_coherent() fails: it returns before dma_tx is reassigned, so
  dma_tx and dma_tx_phy still refer to the freed ring.

The wake clears __QUEUE_STATE_DRV_XOFF. After that, the new
netif_tx_queue_stopped() check in stmmac_xdp_xmit_xdpf() passes, and the
qdisc calls stmmac_xmit() again. Both paths write des0-des3 through the
stale tx_q->dma_tx, and they also write tx_skbuff[entry] and
tx_skbuff_dma[entry].

Isn't this the same crash the commit is fixing, still reachable when the
allocation fails? The netif_tx_wake_queue() added to
stmmac_xdp_disable_pool() has the same problem.

Could stmmac_enable_tx_queue() return an error, so that the queue can be
left stopped (or the device reset) when it fails?

>  
>  		err = stmmac_xsk_wakeup(priv->dev, queue, XDP_WAKEUP_RX);
>  		if (err)

[ ... ]

> @@ -87,6 +99,7 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
>  		stmmac_enable_tx_queue(priv, queue);
>  		napi_enable(&ch->rx_napi);
>  		napi_enable(&ch->tx_napi);
> +		netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, queue));

[Severity: High]

This is a pre-existing issue and was not introduced by this patch, but it
is on the same error path.

If __alloc_dma_tx_desc_resources() partially fails during the pool
switch, tx_q is left inconsistent. The function returns -ENOMEM without
undoing its earlier allocations, and stmmac_enable_tx_queue() does no
cleanup either.

The next teardown can come from
__stmmac_release() or stmmac_xdp_release() through
free_dma_desc_resources()->free_dma_tx_desc_resources()->
__free_dma_tx_desc_resources(). It can also come from the next pool
switch's stmmac_disable_tx_queue(). What it does depends on which
allocation failed:

- tx_skbuff_dma kzalloc failed:
  dma_free_tx_skbufs()->stmmac_free_tx_buffer() dereferences
  tx_q->tx_skbuff_dma[i], which is NULL.
- tx_skbuff kzalloc failed: stmmac_free_tx_buffer() dereferences
  tx_q->xdpf[i], which is NULL.
- dma_alloc_coherent() failed: __free_dma_tx_desc_resources() calls
  dma_free_coherent() again on the stale dma_tx/dma_etx/dma_entx address
  and dma_tx_phy.

Can this cause a NULL pointer oops, or free the coherent descriptor memory
twice?

>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013-stmmac_xsk_crashes-v3-0-50fca7790af2%40linutronix.de