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

[sashiko-bot@kernel.org]

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