Re: [PATCH net v4 13/15] net: stmmac: retain DMA memory until hardware shutdown completes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] net: stmmac: GMAC4/XGMAC DMA memory corruption on resume due to missed descriptor reinit
- [Critical] net: stmmac: hardware UAF/memory corruption on failed DMA shutdown for AF_XDP
--

commit d5468125f4e40d54601ca95b471fb3b78f38dc5d
Author: James Hilliard <james.hilliard1@gmail.com>

net: stmmac: retain DMA memory until hardware shutdown completes

This commit updates the stmmac driver to wait for the hardware DMA to
actually stop before releasing the descriptor rings and buffers. If the
hardware fails to idle, it falls back to a global reset or explicitly
retains the memory to prevent use-after-free by the hardware. It also
updates AF_XDP pools to handle the delayed release properly.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[ ... ]
> @@ -2481,14 +2554,36 @@ static int alloc_dma_desc_resources(struct stmmac_priv *priv,
> +static void stmmac_detach_xsk_buffers(struct stmmac_priv *priv,
> +				      struct stmmac_dma_conf *dma_conf)
> +{
> +	u32 queue;
> +
> +	/* Socket teardown cannot retain the pool itself. Drop software-only
> +	 * references, but keep each xsk_dma reference: hardware can still reach
> +	 * the mapped UMEM pages even after the pool and its heads are freed.
> +	 */
> +	for (queue = 0; queue < MTL_MAX_RX_QUEUES; queue++) {
> +		struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue];
> +
> +		if (!rx_q->xsk_pool)
> +			continue;
> +		dma_free_rx_xskbufs(priv, dma_conf, queue);

[Severity: Critical]
Does this code cause a hardware use-after-free when the DMA engine fails
to stop?

This function is explicitly called by the new quarantine logic below when
dma_owned is true, meaning the hardware is still actively processing.
However, dma_free_rx_xskbufs() unconditionally executes xsk_buff_free()
to return the UMEM buffers to the AF_XDP pool.

While the pages are pinned via xsk_pool_dma_get(), returning them to the
pool allows userspace applications to immediately reallocate and reuse
these memory chunks. Since the hardware DMA is still active and programmed
with these physical addresses, the MAC can asynchronously overwrite the
in-use userspace memory with incoming network packets.

[ ... ]
> @@ -3350,6 +3476,19 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>  		netdev_err(priv->dev, "Failed to reset the dma\n");
>  		return ret;
>  	}
> +	stmmac_dma_reset_complete(priv);
> +	priv->dma_reset_needed = false;
> +
> +	if (reinit) {
> +		stmmac_reinit_dma_desc(priv);
> +	} else if (priv->datapath == STMMAC_DATAPATH_SUSPENDED) {
> +		/* Suspend only requested a stop. Do not modify its descriptors
> +		 * or release pending TX buffers until this reset has completed.
> +		 */
> +		stmmac_reset_queues_param(priv);
> +		stmmac_free_tx_skbufs(priv);
> +		stmmac_clear_descriptors(priv, priv->dma_conf);
> +	}

[Severity: Critical]
Can this fallback path corrupt memory during device resume?

When reinit is false, this code falls back to stmmac_clear_descriptors().
stmmac_clear_descriptors() simply ORs the OWN bit into des3. If there
were pending RX packets when the device suspended, the MAC may have already
written status bits into the descriptors, such as bit 25 RS1V in GMAC4.

By preserving these status bits and setting the OWN bit, the descriptor
is returned to the MAC in Read Format. The MAC then misinterprets these
leftover status bits as physical address bits. For example, bit 25 becomes
BUFFER2_VALID_ADDR, and the packet length becomes the upper 32 bits of
the secondary address. The MAC will then blindly DMA into this random
64-bit physical address.

[ ... ]
> @@ -8721,12 +8969,7 @@ int stmmac_resume(struct device *dev)
>  
>  	mutex_lock(&priv->lock);
>  
> -	stmmac_reset_queues_param(priv);
> -
> -	stmmac_free_tx_skbufs(priv);
> -	stmmac_clear_descriptors(priv, priv->dma_conf);
> -
> -	ret = stmmac_hw_setup(ndev);
> +	ret = stmmac_hw_setup(ndev, false, false);

[Severity: Critical]
Should the reinit parameter be passed as true here instead of false?

Passing false skips the newly introduced stmmac_reinit_dma_desc() rebuild
logic in stmmac_init_dma_engine(), directly triggering the fallback path
described above and causing memory corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-submit-stmmac-reset-fixes-v1-v4-0-ec1c0250b3c9@gmail.com?part=13