Re: [PATCH net 1/3] net: stmmac: handle partially allocated DMA queues

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi,

On 10/14/26 08:26, Karl Mehltretter wrote:
> DMA allocation failure unwinds every configured queue, including the
> failed queue and queues not yet allocated. The cleanup helpers then
> dereference buffer arrays that may not exist.
> 
> Only traverse allocated buffer arrays. Clear freed pointers because the
> queue structures persist and XDP reconfiguration can reuse them.
> 
> Found by inspection. On a BeagleV Ahead (TH1520), fault injection at
> the RX buffer metadata and TX DMA metadata allocations reached these
> invalid cleanup paths before a test guard stopped them. With this
> change, cleanup completed without the guard.
> 
> Fixes: 54139cf3bb33 ("net: stmmac: adding multiple buffers for rx")
> Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

Please look at https://lore.kernel.org/netdev/20261012142458.2845385-1-dinghui1111@163.com/ .

LLMs keep finding the same bugs again and again, make sure you ask
it if there isn't already an in-flight patch for it before sending it :)

Maxime


> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 22 ++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b837e8e27..495ff7aef 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2144,10 +2144,12 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
>  	void *addr;
>  
>  	/* Release the DMA RX socket buffers */
> -	if (rx_q->xsk_pool)
> -		dma_free_rx_xskbufs(priv, dma_conf, queue);
> -	else
> -		dma_free_rx_skbufs(priv, dma_conf, queue);
> +	if (rx_q->buf_pool) {
> +		if (rx_q->xsk_pool)
> +			dma_free_rx_xskbufs(priv, dma_conf, queue);
> +		else
> +			dma_free_rx_skbufs(priv, dma_conf, queue);
> +	}
>  
>  	rx_q->buf_alloc_num = 0;
>  	rx_q->xsk_pool = NULL;
> @@ -2161,13 +2163,17 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
>  	size = stmmac_get_rx_desc_size(priv) * dma_conf->dma_rx_size;
>  
>  	dma_free_coherent(priv->device, size, addr, rx_q->dma_rx_phy);
> +	rx_q->dma_erx = NULL;
> +	rx_q->dma_rx = NULL;
>  
>  	if (xdp_rxq_info_is_reg(&rx_q->xdp_rxq))
>  		xdp_rxq_info_unreg(&rx_q->xdp_rxq);
>  
>  	kfree(rx_q->buf_pool);
> +	rx_q->buf_pool = NULL;
>  	if (rx_q->page_pool)
>  		page_pool_destroy(rx_q->page_pool);
> +	rx_q->page_pool = NULL;
>  }
>  
>  static void free_dma_rx_desc_resources(struct stmmac_priv *priv,
> @@ -2196,7 +2202,8 @@ static void __free_dma_tx_desc_resources(struct stmmac_priv *priv,
>  	void *addr;
>  
>  	/* Release the DMA TX socket buffers */
> -	dma_free_tx_skbufs(priv, dma_conf, queue);
> +	if (tx_q->tx_skbuff_dma && tx_q->tx_skbuff)
> +		dma_free_tx_skbufs(priv, dma_conf, queue);
>  
>  	if (priv->extend_desc) {
>  		addr = tx_q->dma_etx;
> @@ -2209,9 +2216,14 @@ static void __free_dma_tx_desc_resources(struct stmmac_priv *priv,
>  	size = stmmac_get_tx_desc_size(priv, tx_q) * dma_conf->dma_tx_size;
>  
>  	dma_free_coherent(priv->device, size, addr, tx_q->dma_tx_phy);
> +	tx_q->dma_etx = NULL;
> +	tx_q->dma_entx = NULL;
> +	tx_q->dma_tx = NULL;
>  
>  	kfree(tx_q->tx_skbuff_dma);
> +	tx_q->tx_skbuff_dma = NULL;
>  	kfree(tx_q->tx_skbuff);
> +	tx_q->tx_skbuff = NULL;
>  }
>  
>  static void free_dma_tx_desc_resources(struct stmmac_priv *priv,