* [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2
@ 2026-10-22 7:01 Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach
2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach
0 siblings, 2 replies; 5+ messages in thread
From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Eric Dumazet
Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim,
Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski,
Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf,
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 v4:
- Rebase to net-next (Jakub)
- Add tags (Nicolai)
- Link to v3: https://patch.msgid.link/20261013-stmmac_xsk_crashes-v3-0-50fca7790af2@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: f71ff7d406523748bfb8236830c7eaa6a308aaa1
change-id: 20260914-stmmac_xsk_crashes-10a77f614ab5
Best regards,
--
Kurt Kanzenbach <kurt@linutronix.de>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() 2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach @ 2026-10-22 7:01 ` Kurt Kanzenbach 2026-10-26 7:24 ` netdev-bot+sashiko 2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach 1 sibling, 1 reply; 5+ messages in thread From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw) To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim, Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski, Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf, 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: LLM 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 fb53b1f17714..27c4a249e560 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -7222,15 +7222,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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() 2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach @ 2026-10-26 7:24 ` netdev-bot+sashiko 0 siblings, 0 replies; 5+ messages in thread From: netdev-bot+sashiko @ 2026-10-26 7:24 UTC (permalink / raw) To: kurt Cc: maxime.chevallier, andrew+netdev, davem, kuba, pabeni, edumazet, mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk, john.fastabend, sdf, yoong.siang.song, faizal.abdul.rahim, boon.leong.ong, bigeasy, maciej.fijalkowski, nb, netdev, linux-stm32, linux-arm-kernel, bpf Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider. Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0 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] stmmac_xdp_release() now runs napi_disable (via stmmac_disable_all_queues()), then hrtimer_cancel() on every… - [High] The commit says the Tx queue must not be woken again after netif_tx_disable() in stmmac_xdp_release(), and it fixes this only for… Pre-existing issues: - [High] This problem wasn't introduced by this patch, but stmmac_xdp_set_prog() discards the return value of stmmac_xdp_open()… -- 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 timers are cancelled before netif_tx_disable(). The goal is to stop stmmac_tx_clean() from waking the Tx queue after it was stopped, which let stmmac_xmit() run on descriptor rings freed by free_dma_desc_resources(). > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index fb53b1f17714..27c4a249e560 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -7222,15 +7222,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] With this order, ndo_start_xmit is still live while the hrtimer_cancel() loop runs. Can stmmac_xmit() or stmmac_tso_xmit() re-arm txtimer after it has been cancelled? Both of them end with stmmac_tx_timer_arm(), which does: 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); ... } Nothing makes this check-then-start sequence atomic with respect to napi_disable() and hrtimer_cancel(). One possible interleaving: CPU1 (stmmac_xmit, holding txq lock) napi_is_scheduled(napi) -> false CPU0 (stmmac_xdp_release) stmmac_disable_all_queues() hrtimer_cancel(&tx_q->txtimer) timer not armed, returns at once hrtimer_start(&tx_q->txtimer, ...) netif_tx_disable(dev) only waits for CPU1 to drop txq lock Nothing later in stmmac_xdp_release() cancels the timer. It stays queued for tx_coal_timer, which is 5ms by default and up to 100ms via ethtool -C. A hardirq on CPU1 widens the window. On PREEMPT_RT it can be arbitrarily long. stmmac_xdp_set_prog() then calls stmmac_xdp_open() right away, and that function does: stmmac_xdp_open() { ... hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); ... } Would this re-initialize an hrtimer that is still linked in the per-CPU timerqueue and corrupt the hrtimer base? Before this patch, netif_tx_disable() ran first and serialized against in-flight xmit callers. Only NAPI could arm the timer, and NAPI was disabled before the cancel. Would the order napi disable, then netif_tx_disable(), then the hrtimer_cancel() loop avoid this? > /* Free the IRQ lines */ > stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0); > [Severity: High] The commit message says the problem is the Tx queue being woken again after netif_tx_disable(), and this patch covers the stmmac_tx_clean() waker. Is there still a waker in IRQ context? netif_tx_disable() still runs before stmmac_free_irq() and stmmac_stop_all_dma(). At that point the DMA interrupt handlers are still installed and Tx DMA is still running. stmmac_xdp_release() never sets STMMAC_DOWN, so the STMMAC_DOWN check in stmmac_interrupt() and stmmac_msi_intr_tx() does not stop the handlers. On a Tx fatal bus error (tx_hard_error), stmmac_dma_interrupt() or stmmac_msi_intr_tx() calls stmmac_tx_err(), which ends with an unconditional wake: stmmac_tx_err() { ... priv->xstats.tx_errors++; netif_tx_wake_queue(netdev_get_tx_queue(priv->dev, chan)); } The qdisc is still active in the XDP reconfigure path. In ndo_stop, dev_deactivate_many() has already run, but not here. So the wake can start stmmac_xmit() again: stmmac_xdp_release() netif_tx_disable(dev) stmmac_dma_interrupt() stmmac_tx_err() netif_tx_wake_queue() stmmac_free_irq() stmmac_stop_all_dma() free_dma_desc_resources() stmmac_xmit() writes into tx_q->dma_tx / dma_etx free_irq() only waits for handlers that are already running. It does not undo the wake, and nothing stops the queue again after it. Could stmmac_xmit() then write descriptors into rings that free_dma_desc_resources() has already freed? Moving netif_tx_disable() after stmmac_free_irq() might close this path. Another option is a teardown flag that stmmac_tx_err() and the ISRs check. The last patch of the series ("net: stmmac: Stop Tx queue when (en|dis)abling XSK pools") does not seem to cover this case. [Severity: High] This is a pre-existing issue and was not introduced by this patch. It is on the same release/reopen path, though. stmmac_xdp_set_prog() ignores the return value of stmmac_xdp_open(): drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() { ... if (if_running && need_update) stmmac_xdp_open(dev); ... } stmmac_xdp_open() can fail in alloc_dma_desc_resources(), init_dma_desc_rings() or stmmac_request_irq(). On failure it frees the descriptor resources and returns before stmmac_enable_all_queues(), netif_carrier_on() and netif_tx_start_all_queues(). Userspace is still told the XDP update succeeded. What happens on the next ifdown in that state? The call chain stmmac_release()->__stmmac_release()->stmmac_disable_all_queues() calls napi_disable() on NAPI instances that stmmac_xdp_release() has already disabled. In napi_disable_locked(): val = READ_ONCE(n->state); do { while (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) { usleep_range(20, 200); val = READ_ONCE(n->state); } Both bits are already set, so wouldn't this loop forever while holding rtnl_lock and the netdev instance lock? Another XDP program change would take the same path through stmmac_xdp_release(). Should stmmac_xdp_set_prog() propagate the error, and should the device be left in a state that a later close can handle? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-stmmac_xsk_crashes-v4-0-15c0c750bb96%40linutronix.de ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools 2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach 2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach @ 2026-10-22 7:01 ` Kurt Kanzenbach 2026-10-26 7:24 ` netdev-bot+sashiko 1 sibling, 1 reply; 5+ messages in thread From: Kurt Kanzenbach @ 2026-10-22 7:01 UTC (permalink / raw) To: Maxime Chevallier, Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet Cc: Maxime Coquelin, Alexandre Torgue, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Song Yoong Siang, Mohd Faizal Abdul Rahim, Ong Boon Leong, Sebastian Andrzej Siewior, Maciej Fijalkowski, Nicolai Buchwitz, netdev, linux-stm32, linux-arm-kernel, bpf, 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 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 | 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 27c4a249e560..2083520179a6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5270,6 +5270,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue, if (unlikely(xdp_frame_has_frags(xdpf))) num_frames += sinfo->nr_frags; + /* 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) < max(num_frames, txq_thr)) 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 9af9e0cb366c..14445683a5c3 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools 2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach @ 2026-10-26 7:24 ` netdev-bot+sashiko 0 siblings, 0 replies; 5+ messages in thread From: netdev-bot+sashiko @ 2026-10-26 7:24 UTC (permalink / raw) To: kurt Cc: maxime.chevallier, andrew+netdev, davem, kuba, pabeni, edumazet, mcoquelin.stm32, alexandre.torgue, ast, daniel, hawk, john.fastabend, sdf, yoong.siang.song, faizal.abdul.rahim, boon.leong.ong, bigeasy, maciej.fijalkowski, nb, netdev, linux-stm32, linux-arm-kernel, bpf Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider. Critical: 0 · High: 1 (1) · Medium: 2 · Low: 0 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] stmmac_xdp_enable_pool() (stmmac_xdp.c:59) and stmmac_xdp_disable_pool() (stmmac_xdp.c:102) now call… - [Medium] The patch only works if __QUEUE_STATE_DRV_XOFF stays set from stmmac_xdp_stop_tx_queue() until the ring is rebuilt. - [Medium] In stmmac_xdp_xmit_xdpf(), first_entry and entry are read from tx_q->cur_tx when the locals are declared (stmmac_main.c:5263-5264). Pre-existing issues: - [High] This problem wasn't introduced by this patch, but if __alloc_dma_tx_desc_resources() fails during an XSK pool switch, tx_q… -- net: stmmac: Stop Tx queue when (en|dis)abling XSK pools This patch stops the netdev Tx queue while an XSK pool is enabled or disabled on a running interface. It does this because stmmac_disable_tx_queue() and stmmac_enable_tx_queue() free and reallocate the Tx DMA ring. The queue is woken once the ring is rebuilt, and stmmac_xdp_xmit_xdpf() now drops XDP frames while the queue is stopped. > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 27c4a249e560..2083520179a6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -5270,6 +5270,10 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue, > if (unlikely(xdp_frame_has_frags(xdpf))) > num_frames += sinfo->nr_frags; > > + /* 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] Is this check early enough? first_entry and entry have already been read from tx_q->cur_tx at the top of stmmac_xdp_xmit_xdpf(): unsigned int first_entry = tx_q->cur_tx; unsigned int entry = tx_q->cur_tx; The pool switch path does not hold the Tx lock while it rebuilds the ring or wakes the queue. stmmac_enable_tx_queue()->stmmac_reset_tx_queue() sets cur_tx and dirty_tx to 0. After that, netif_tx_wake_queue() clears XOFF without taking the lock. Take an XDP sender (stmmac_xdp_xmit_back() or stmmac_xdp_xmit()) that: 1. takes __netif_tx_lock() while the queue is stopped 2. loads the old cur_tx == N 3. is delayed past the whole rebuild and wake, for example by being preempted on PREEMPT_RT 4. then finds the queue running and passes stmmac_tx_avail() Could that sender fill and OWN-mark descriptors at the stale index N on the new ring and then set cur_tx = N + 1? DMA restarts at descriptor 0, which nobody owns. That looks like it would stall Tx. stmmac_tx_clean() would then walk entries 0..N-1 as if they had completed. > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c > index 9af9e0cb366c..14445683a5c3 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] Can stmmac_tx_err() undo this stop while the pool switch is still running? stmmac_tx_err() runs in hardirq context from stmmac_dma_interrupt() and stmmac_msi_intr_tx() when the channel status is tx_hard_error. It takes no lock and always finishes by waking the queue: stmmac_tx_err() { ... stmmac_stop_tx_dma(priv, chan); dma_free_tx_skbufs(priv, &priv->dma_conf, chan); stmmac_clear_tx_descriptors(priv, &priv->dma_conf, chan); stmmac_reset_tx_queue(priv, chan); 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)); } The switch does not seem to keep it out. dwmac4_disable_dma_irq() clears only RIE and TIE, so AIE, RPS, RBU and FBE stay enabled. There is also no synchronize_irq(). Stopping RX DMA during the switch raises RPS, and with it AIS. dwmac4_dma_interrupt() decodes AIS together with TPS or FBE as tx_hard_error. TPS is never acknowledged, because only the enabled bits get written back: writel(intr_status & intr_en, ioaddr + DMA_CHAN_STATUS(dwmac4_addrs, chan)); That means TPS can stay latched from an earlier Tx DMA stop. Suppose tx_hard_error is decoded inside the stop-to-wake window. Wouldn't stmmac_tx_err() then work on the freed or half-rebuilt ring and restart DMA on freed coherent memory? It would also reopen the queue for stmmac_xmit() and the XDP Tx paths. The source alone does not show whether the hardware reports exactly tx_hard_error in that window. For example, a latched RI bit would change the status value. [ ... ] > @@ -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] What happens here if stmmac_enable_tx_queue() failed? It returns void. When an allocation fails, it only logs a message 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; } ... } The state left behind depends on which allocation in __alloc_dma_tx_desc_resources() failed: - tx_q->tx_skbuff_dma or tx_q->tx_skbuff is NULL, or - dma_tx/dma_etx/dma_entx still point at the coherent ring that __free_dma_tx_desc_resources() just freed. That function never clears those pointers. stmmac_reset_tx_queue() is skipped as well, so cur_tx and dirty_tx keep their old values. Once this unconditional wake runs, stmmac_xmit() can execute: WARN_ON(tx_q->tx_skbuff[first_entry]); It would then write tx_skbuff/tx_skbuff_dma entries, and descriptors via stmmac_get_tx_desc(), into NULL or freed memory. The new netif_tx_queue_stopped() check in stmmac_xdp_xmit_xdpf() is open again too. stmmac_xdp_disable_pool() gets the same unconditional wake. The commit message says the crash comes from Tx touching the ring while it is freed and reallocated. On this error path, that still seems reachable. Could stmmac_enable_tx_queue() return the error, so the queue stays stopped (or the switch is unwound) instead of being woken? [ ... ] > @@ -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; [Severity: High] This is a pre-existing issue and was not introduced by this patch. After stmmac_enable_tx_queue() fails, tx_q looks set up for a double free on the next teardown. __alloc_dma_tx_desc_resources() does no unwind. It returns -ENOMEM at the first step that fails: tx_q->tx_skbuff_dma = kzalloc_objs(*tx_q->tx_skbuff_dma, dma_conf->dma_tx_size); if (!tx_q->tx_skbuff_dma) return -ENOMEM; tx_q->tx_skbuff = kzalloc_objs(struct sk_buff *, dma_conf->dma_tx_size); if (!tx_q->tx_skbuff) return -ENOMEM; ... addr = dma_alloc_coherent(priv->device, size, &tx_q->dma_tx_phy, GFP_KERNEL); if (!addr) return -ENOMEM; __free_dma_tx_desc_resources() never clears dma_tx/dma_etx/dma_entx, tx_skbuff or tx_skbuff_dma. The next teardown therefore calls it again on the stale pointers. That teardown can be stmmac_disable_tx_queue() during another pool switch, or free_dma_tx_desc_resources() from stmmac_release(). Wouldn't that call dma_free_coherent() a second time on the ring that was already freed? If the tx_skbuff_dma allocation was the one that failed, two more things happen: - dma_free_tx_skbufs() dereferences the NULL tx_skbuff_dma in stmmac_free_tx_buffer() - kfree(tx_q->tx_skbuff) frees the old array a second time -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-stmmac_xsk_crashes-v4-0-15c0c750bb96%40linutronix.de ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-26 7:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-22 7:01 [PATCH net-next v4 0/2] net: stmmac: Fix XSK crashes on stm32mp2 Kurt Kanzenbach 2026-10-22 7:01 ` [PATCH net-next v4 1/2] net: stmmac: Disable NAPI before stopping Tx queues in stmmac_xdp_release() Kurt Kanzenbach 2026-10-26 7:24 ` netdev-bot+sashiko 2026-10-22 7:01 ` [PATCH net-next v4 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools Kurt Kanzenbach 2026-10-26 7:24 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox