Re: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

[Frank Li <Frank.li@oss.nxp.com>]

On Sun, Aug 16, 2026 at 03:49:26PM +0000, Shivank Garg wrote:
> [You don't often get email from shivankg@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> When dma_device_put() drops the last reference on chan->device->ref,
> dma_device_release() runs and may free the dma_device along with its
> channels.
>
> Two paths still read that memory after the put:
>  - dma_chan_put() reads chan->device->owner via dma_chan_to_owner()
>    for the trailing module_put().
>  - dma_release_channel() calls dma_chan_put() before reading chan->slave,
>    chan->name, chan->dev and chan->dbg_client_name.
>
> KASAN catches the first one:
>
>         slab-use-after-free in dma_chan_put+0x3e6/0x4c0
>         Read of size 8 by task insmod/6319
>         Freed by task 6319:
>           kfree+0x225/0x470
>           dma_chan_put+0x395/0x4c0
>           dmaengine_put+0xf8/0x160
>
> Cache the module owner in dma_chan_put() before the put so the trailing
> module_put() does not need chan->device. In dma_release_channel(), move
> dma_chan_put() to the end, after every chan/device read.
>
> Fixes: 8ad342a86359 ("dmaengine: Add reference counting to dma_device struct")
> Suggested-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---


Reviewed-by: Frank Li <Frank.Li@nxp.com>


>  drivers/dma/dmaengine.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 516d6d933208..bf491eb10596 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -495,10 +495,13 @@ static int dma_chan_get(struct dma_chan *chan)
>   */
>  static void dma_chan_put(struct dma_chan *chan)
>  {
> +       struct module *owner;
> +
>         /* This channel is not in use, bail out */
>         if (!chan->client_count)
>                 return;
>
> +       owner = dma_chan_to_owner(chan);
>         chan->client_count--;
>
>         /* This channel is not in use anymore, free it */
> @@ -518,7 +521,7 @@ static void dma_chan_put(struct dma_chan *chan)
>         /* This channel is not in use anymore, drop the device ref */
>         if (!chan->client_count)
>                 dma_device_put(chan->device);
> -       module_put(dma_chan_to_owner(chan));
> +       module_put(owner);
>  }
>
>  enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
> @@ -916,8 +919,6 @@ void dma_release_channel(struct dma_chan *chan)
>         if (--chan->device->privatecnt == 0)
>                 dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
>
> -       dma_chan_put(chan);
> -
>         if (chan->slave) {
>                 sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);
>                 sysfs_remove_link(&chan->slave->kobj, chan->name);
> @@ -930,6 +931,7 @@ void dma_release_channel(struct dma_chan *chan)
>         kfree(chan->dbg_client_name);
>         chan->dbg_client_name = NULL;
>  #endif
> +       dma_chan_put(chan);
>         mutex_unlock(&dma_list_mutex);
>  }
>  EXPORT_SYMBOL_GPL(dma_release_channel);
>
> --
> 2.43.0
>