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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: Use-After-Free in stmmac_tx_err() due to active ISR after DMA descriptors freed
--

commit 4c2320440039ed3235615c60f92cbcfdc0584d5a
Author: Daniel Thompson <daniel@riscstar.com>

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

This commit changes the XDP setup process to avoid completely freeing and
re-requesting IRQs, which disrupts TSN thread tuning. Instead of calling
stmmac_free_irq(), it masks the DMA interrupts and synchronizes them during
XDP release.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 7b80fe66cf199..f1359637007e2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[ ... ]
> @@ -7141,8 +7186,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: High]
Could this change introduce a use-after-free when setting up an XDP program?

Since the ISR stmmac_interrupt() remains registered after replacing
stmmac_free_irq(), the interrupt handler can still run if a shared IRQ or
unmasked MAC interrupt fires.

When stmmac_stop_all_dma() stops the TX DMA, it sets the TPS (Tx Process
Stopped) bit in the hardware DMA status register.

Immediately after this in stmmac_xdp_release(), free_dma_desc_resources()
is called, which frees the descriptor memory without assigning NULL.

If an interrupt fires at this point, stmmac_interrupt() reads the raw DMA
status and evaluates TPS regardless of whether DMA interrupts are masked:

drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c:dwmac4_dma_interrupt() {
    ...
    if (unlikely(intr_status & DMA_CHAN_STATUS_TPS)) {
        x->tx_process_stopped_irq++;
        ret = tx_hard_error;
    }
    ...
}

This unconditionally returns tx_hard_error, which then triggers
stmmac_tx_err() to execute.

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_tx_err() {
    ...
    dma_free_tx_skbufs(priv, &priv->dma_conf, chan);
    stmmac_clear_tx_descriptors(priv, &priv->dma_conf, chan);
    ...
}

Will stmmac_tx_err() then try to clear and re-free the already freed
descriptors, resulting in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-tc956x-stmmac-no_irq_teardown-v3-0-8189870be545@riscstar.com?part=2