Re: [PATCHv4 04/15] dmaengine: fsldma: provide device_release callback

[Frank Li]

On Wed, Jun 10, 2026 at 08:52:34PM -0700, Rosen Penev wrote:
> The DMA core requires drivers to set dma_device.device_release so that
> the container structure is only freed after all references to it have
> been dropped (see the comment above dma_async_device_register()).

why not use dmaenginem_async_device_register()
>
> This driver violated that contract: fdev was devm_kzalloc()'d with no
> device_release callback.  If a client still held a channel reference
> when the driver was unbound, dma_device_release() would eventually
> run on freed memory, causing a use-after-free.

new some convert chan[] to flex array.

see
https://lore.kernel.org/dmaengine/20260609194217.76E8B1F00893@xxxxxxxxxxxxxxx/

Frank
>
> Fix by allocating fdev with kzalloc_obj(), adding
> fsldma_device_release() to free it, and setting device_release.
> fsldma_of_remove() now saves channel pointers and frees IRQs before
> calling dma_async_device_unregister(), since fdev may be freed by
> the release callback inside that call.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  drivers/dma/fsldma.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 1ba10d065278..43d817f6ded1 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1219,6 +1219,8 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  	kfree(chan);
>  }
>
> +static void fsldma_device_release(struct dma_device *dma_dev);
> +
>  static int fsldma_of_probe(struct platform_device *op)
>  {
>  	struct fsldma_device *fdev;
> @@ -1257,6 +1259,7 @@ static int fsldma_of_probe(struct platform_device *op)
>  	fdev->common.device_issue_pending = fsl_dma_memcpy_issue_pending;
>  	fdev->common.device_config = fsl_dma_device_config;
>  	fdev->common.device_terminate_all = fsl_dma_device_terminate_all;
> +	fdev->common.device_release = fsldma_device_release;
>  	fdev->common.dev = &op->dev;
>
>  	fdev->common.src_addr_widths = FSL_DMA_BUSWIDTHS;
> @@ -1316,19 +1319,33 @@ static int fsldma_of_probe(struct platform_device *op)
>  	return err;
>  }
>
> +static void fsldma_device_release(struct dma_device *dma_dev)
> +{
> +	struct fsldma_device *fdev = container_of(dma_dev, struct fsldma_device,
> +						  common);
> +	kfree(fdev);
> +}
> +
>  static void fsldma_of_remove(struct platform_device *op)
>  {
> -	struct fsldma_device *fdev;
> +	struct fsldma_device *fdev = platform_get_drvdata(op);
> +	struct fsldma_chan *chans[FSL_DMA_MAX_CHANS_PER_DEVICE];
>  	unsigned int i;
>
> -	fdev = platform_get_drvdata(op);
> -	dma_async_device_unregister(&fdev->common);
> +	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++)
> +		chans[i] = fdev->chan[i];
>
>  	fsldma_free_irqs(fdev);
>
> +	/*
> +	 * fdev may be freed by fsldma_device_release inside this call;
> +	 * use saved copies of the channel pointers afterwards.
> +	 */
> +	dma_async_device_unregister(&fdev->common);
> +
>  	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
> -		if (fdev->chan[i])
> -			fsl_dma_chan_remove(fdev->chan[i]);
> +		if (chans[i])
> +			fsl_dma_chan_remove(chans[i]);
>  	}
>  	irq_dispose_mapping(fdev->irq);
>
> --
> 2.54.0
>