Re: [PATCH net-next v2] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Removing `stmmac_free_irq()` leaves interrupt handlers active during XDP reconfiguration. Stopping the DMA engine triggers an unmasked abnormal interrupt that races with descriptor teardown, causing the error handler `stmmac_tx_err()` to access freed DMA memory.
--

--- Patch [1]: [PATCH net-next v2] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog ---
commit 5d6c0328f64a95acf920d677c1576229f816f722
Author: Daniel Thompson <[email protected]>

net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

This patch modifies the XDP setup path in the stmmac driver to avoid freeing
and re-requesting IRQs. Instead of fully tearing down the interrupts, it
silences the DMA interrupts and synchronizes them before stopping the DMA
channels.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2a0d7eff88d3f..3cf436014686a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -7108,8 +7152,9 @@ void stmmac_xdp_release(struct net_device *dev)
>  	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>  		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>  
> -	/* Free the IRQ lines */
> -	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
> +	/* Silence DMA interrupts */
> +	stmmac_disable_all_dma_irq(priv);
> +	stmmac_synchronize_irq(dev);
>  
>  	/* Stop TX/RX DMA channels */
>  	stmmac_stop_all_dma(priv);

[Severity: Critical]
Could this lead to a race condition and use-after-free in the interrupt
handler?

By replacing stmmac_free_irq() with stmmac_disable_all_dma_irq(), the OS-level
interrupt handlers remain active. Since stmmac_disable_all_dma_irq() only
disables normal RX/TX interrupts, abnormal interrupts remain unmasked.

When stmmac_stop_all_dma() is called immediately after, it halts the DMA engine.
This sets the Transmit Process Stopped (TPS) status flag, which triggers an
unmasked abnormal hardware interrupt.

Because the handler is still registered, the interrupt fires and calls
stmmac_tx_err(), which accesses the DMA queues. Concurrently, the thread
executing stmmac_xdp_release() proceeds to call free_dma_desc_resources():

Thread 1 (stmmac_xdp_release):
    stmmac_stop_all_dma(priv);
    // TPS interrupt raised here
    free_dma_desc_resources(priv, ...);
    // Frees TX resources and socket buffers

Thread 2 (stmmac_interrupt):
    // Interrupt fires due to TPS error
    stmmac_tx_err(priv, chan);
    // Accesses and attempts to clean up the same freed DMA resources

Is there a way to safely mask abnormal interrupts as well before stopping the
DMA channels to avoid this race?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-tc956x-stmmac-no_irq_teardown-v2-1-a6c31932ba84@riscstar.com?part=1