Re: [PATCH net] net: ethernet: i825xx: Fix dma_alloc_coherent() size

[Jacob Keller]

On 10/6/2026 7:32 AM, Thomas Fourier wrote:
> In sni_82596_probe(), the lp->dma buffer is allocated with
> dma_alloc_coherent() and with size sizeof(struct i596_dma), and possibly
> freed in the error path with the same size. However, in
> sni_82596_driver_remove(), the same buffer is freed but with size
> sizeof(struct i596_private). This error may leave the freed buffers
> mapped, leaking a resource and allowing the device to access freed
> memory.
> 
> Change the length in sni_82596_driver_remove() to
> sizeof(struct i596_dma).
> 
> This patch was compile tested only, and found by hand.
> 
> Fixes: f2ec8030085a ("Ethernet driver for EISA only SNI RM200/RM400 machines")

Hmm. At first this didn't seem like the right fixes tag. The offending
code was changed multiple times before being caught.

> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Thomas Fourier <fourier.thomas@xxxxxxxxx>
> ---
>  drivers/net/ethernet/i825xx/sni_82596.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
> index baa598988f47..73e1e153cb78 100644
> --- a/drivers/net/ethernet/i825xx/sni_82596.c
> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
> @@ -159,7 +159,7 @@ static void sni_82596_driver_remove(struct platform_device *pdev)
>  	struct i596_private *lp = netdev_priv(dev);
>  
>  	unregister_netdev(dev);
> -	dma_free_coherent(&pdev->dev, sizeof(struct i596_private), lp->dma,
> +	dma_free_coherent(&pdev->dev, sizeof(struct i596_dma), lp->dma,
>  			  lp->dma_addr);

This dma_free_coherent call was added by commit 48d15814dd0f ("lib82596:
move DMA allocation into the callers of i82596_probe").

But I guess previous to this it was using dma_free_attrs inside of the
probe function and that also appears to have also mistakenly used a
different size.

Digging deeper, this was changed to dma_free_attrs as part of commit
7f683b920479 ("i825xx: switch to switch to dma_alloc_attrs"), previously
using DMA_FREE. But even prior to this it still had the incorrect size.

Strictly, a backport to that old version would have merge conflicts due
to the changes, but it is accurate that the bug exists all the way back
to 2.6.23... Hopefully no one is going to bother trying though and every
currently supported stable release has the current code and should apply
clean.

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>

>  	iounmap(lp->ca);
>  	iounmap(lp->mpu_port);