Re: [PATCH] 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 3 potential issue(s) to consider:

New issues:
- [High] Removing `stmmac_disable_dma_irq()` during XDP open leaves DMA interrupts unmasked before NAPI is enabled, leading to lost wakeups and potential interrupt storms.

Pre-existing issues:
- [Critical] Concurrent `ndo_xdp_xmit` access during XDP reconfiguration causes UAF of DMA descriptors.
- [High] The return value of `stmmac_xdp_open()` is ignored in `stmmac_xdp_setup_prog()`, leading to a silent network outage if reopening the device fails.
--

commit 28a5a0b2fab7c42eb59936b760afc9e0ec09c6ef
Author: Daniel Thompson <daniel@riscstar.com>

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

This commit removes the implicit IRQ synchronization by omitting calls to
stmmac_free_irq() and stmmac_request_irq() during XDP program setup. It
instead explicitly disables DMA interrupts and synchronizes the IRQ handler
to prevent tearing down and restarting IRQ threads, which disrupts TSN
system tuning.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2a0d7eff88d3f..acd145f1f3069 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]
This is a pre-existing issue, but does freeing the DMA descriptor rings
shortly after this point (via free_dma_desc_resources) cause a
use-after-free if ndo_xdp_xmit runs concurrently?

While stmmac_xdp_release() calls netif_tx_disable(), this does not stop
stmmac_xdp_xmit() from executing. If an XDP redirect occurs exactly while
the program is being detached, could stmmac_xdp_xmit() proceed and access
the freed tx_q->dma_tx array? It appears stmmac_xdp_xmit() only checks
STMMAC_DOWN, which is not set during XDP reconfiguration.

[ ... ]

> @@ -7156,10 +7201,8 @@ int stmmac_xdp_open(struct net_device *dev)
>  	stmmac_reset_queues_param(priv);
>  
>  	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++) {
> +	for (chan = 0; chan < dma_csr_ch; chan++)
>  		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> -		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> -	}
>  
>  	/* Adjust Split header */
>  	sph_en = (priv->hw->rx_csum > 0) && priv->sph_active;

[Severity: High]
When stmmac_disable_dma_irq() is removed here, stmmac_init_chan() still
unmasks interrupts at the hardware level. Does this leave the DMA interrupts
unmasked before NAPI is enabled?

Since the OS IRQ is now permanently active (not freed during release), could
a packet arriving right after stmmac_start_all_dma() fire an interrupt
before stmmac_enable_all_queues() has run? It looks like this might cause
the interrupt service routine's call to napi_schedule_prep() to fail,
leading to a lost wakeup or an interrupt storm.

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed that in stmmac_xdp_setup_prog()
in drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c, the return value of
stmmac_xdp_open() is ignored:

drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_setup_prog() {
    ...
    if (if_running && need_update)
        stmmac_xdp_open(dev);

    if (prog)
        xdp_features_set_redirect_target(dev, false);

    return 0;
}

If stmmac_xdp_open() fails (for example, due to a memory allocation failure
for DMA rings), it returns a negative error code. Does ignoring this error
mask the initialization failure and leave the interface functionally down,
potentially leading to a silent network outage?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-tc956x-stmmac-no_irq_teardown-v1-1-df009d0272bf@riscstar.com?part=1