Re: [PATCH net] net: macb: fix dma_alloc_coherent() leak on macb_alloc() error paths

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Théo

On 18.9.2026 21:53, Théo Lebrun wrote:
> Fix 3 leaks in macb_alloc() error paths:
> - Tx buffer allocated but crossing a 4G boundary: Tx leaked.
> - Rx buffer allocation fails: Tx leaked.
> - Rx buffer allocated but crossing a 4G boundary: Tx & Rx leaked.
> 
> This is because our error handling calls macb_free(bp) which in turn
> frees the buffers stored in bp->queues[0], but nothing has been stored
> in there. Fix by storing allocated buffers into bp->queues[0] ASAP.
> 
> Fixes: 78d901897b3c ("net: macb: single dma_alloc_coherent() for DMA 
> descriptors")
> Cc: stable@vger.kernel.org
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index b8234ac4b602..8e5c034dc3a4 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2749,14 +2749,24 @@ static int macb_alloc(struct macb *bp)
> 
>  	size = bp->num_queues * macb_tx_ring_size_per_queue(bp);
>  	tx = dma_alloc_coherent(dev, size, &tx_dma, GFP_KERNEL);
> -	if (!tx || upper_32_bits(tx_dma) != upper_32_bits(tx_dma + size - 1))
> +	if (!tx)
> +		goto out_err;
> +	/* Record the buffer so that the error path frees it. */
> +	bp->queues[0].tx_ring = tx;
> +	bp->queues[0].tx_ring_dma = tx_dma;
> +	if (upper_32_bits(tx_dma) != upper_32_bits(tx_dma + size - 1))
>  		goto out_err;
>  	netdev_dbg(bp->netdev, "Allocated %zu bytes for %u TX rings at %08lx 
> (mapped %p)\n",
>  		   size, bp->num_queues, (unsigned long)tx_dma, tx);
> 
>  	size = bp->num_queues * macb_rx_ring_size_per_queue(bp);
>  	rx = dma_alloc_coherent(dev, size, &rx_dma, GFP_KERNEL);
> -	if (!rx || upper_32_bits(rx_dma) != upper_32_bits(rx_dma + size - 1))
> +	if (!rx)
> +		goto out_err;
> +	/* Record the buffer so that the error path frees it. */
> +	bp->queues[0].rx_ring = rx;
> +	bp->queues[0].rx_ring_dma = rx_dma;
> +	if (upper_32_bits(rx_dma) != upper_32_bits(rx_dma + size - 1))
>  		goto out_err;
>  	netdev_dbg(bp->netdev, "Allocated %zu bytes for %u RX rings at %08lx 
> (mapped %p)\n",
>  		   size, bp->num_queues, (unsigned long)rx_dma, rx);
> 
> ---
> base-commit: 994db8ab9d90c64dd641b7ead6efe2eaea7a50dc
> change-id: 20260918-macb-alloc-leak-ef38989605c7
> 
> Best regards,
> --
> Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai