Re: [PATCH v2 2/6] dmaengine: plx_dma: fix descriptor ring leak in alloc_chan_resources()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] dmaengine: plx_dma: missing Fixes trailer for
  alloc_chan_resources() leak fix
- [Low] dmaengine: plx_dma: duplicate cleanup in plx_dma_free_desc()
--

commit db658b0054bd97de26e837fb78fec5b170b3aad1
Author: Logan Gunthorpe <logang@deltatee.com>

> dmaengine: plx_dma: fix descriptor ring leak in alloc_chan_resources()
>
> If the device is removed between plx_dma_alloc_desc() and the pdev
> check, the error path frees hw_ring but leaks desc_ring and every
> descriptor in it.
>
> Add plx_dma_free_desc() and use it here and in
> plx_dma_free_chan_resources().
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/dmaengine/20260717223217.B6FBF1F000E9@smtp.kernel.org
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

[Severity: Low]
Should this include a Fixes tag for the commit that introduced the leak in
plx_dma_alloc_chan_resources():

  Fixes: c2dbcaa8c672 ("dmaengine: plx-dma: Implement hardware initialization and cleanup")

> diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
> index 0a7ad2473cb13..bfccbb0c70e84 100644
> --- a/drivers/dma/plx_dma.c
> +++ b/drivers/dma/plx_dma.c
> @@ -404,6 +404,16 @@ static int plx_dma_alloc_desc(struct plx_dma_dev *plxdev)
>  	return -ENOMEM;
>  }
>  
> +static void plx_dma_free_desc(struct plx_dma_dev *plxdev)
> +{
> +	int i;
> +
> +	for (i = 0; i < PLX_DMA_RING_COUNT; i++)
> +		kfree(plxdev->desc_ring[i]);
> +
> +	kfree(plxdev->desc_ring);
> +}

[Severity: Low]
This isn't a bug, but plx_dma_free_desc() duplicates the free_and_exit
cleanup at the end of plx_dma_alloc_desc() right above it:

drivers/dma/plx_dma.c:plx_dma_alloc_desc() {
    ...
free_and_exit:
	for (i = 0; i < PLX_DMA_RING_COUNT; i++)
		kfree(plxdev->desc_ring[i]);
	kfree(plxdev->desc_ring);
	return -ENOMEM;
}

Moving plx_dma_free_desc() above plx_dma_alloc_desc() would allow
free_and_exit to call plx_dma_free_desc(plxdev) as well.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110173927.624629-1-logang@deltatee.com?part=2