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

On Sun, Aug 16, 2026 at 06:05:27PM +0000, Garg, Shivank wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Sun, 2026-08-16 at 16:07 +0000, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] Clearing `chan->slave` before stopping the hardware introduces a NULL pointer dereference in `stm32_dma3_pm_suspend()`.
> > --
> >
> > --- Patch [2]: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel() ---
> > commit c0bca60dc6977cca7352bacd1095081d89c88211
> > Author: Shivank Garg <[email protected]>
> >
> > dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
> >
> > When dma_device_put() drops the last reference on chan->device->ref,
> > dma_device_release() may free the dma_device. This commit caches the
> > module owner and delays dma_chan_put() to prevent reading freed memory.
> >
> > Link: https://sashiko.dev/#/patchset/[email protected]
> >
> > > --- a/drivers/dma/dmaengine.c
> > > +++ b/drivers/dma/dmaengine.c
> > > @@ -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);
> > >             kfree(chan->name);
> > >             chan->name = NULL;
> > >             chan->slave = NULL;
> > >     }
> > >
> > >  #ifdef CONFIG_DEBUG_FS
> > >     kfree(chan->dbg_client_name);
> > >     chan->dbg_client_name = NULL;
> > >  #endif
> > > +   dma_chan_put(chan);
> >
> > [Severity: High]
> > Could moving dma_chan_put() here introduce a NULL pointer dereference
> > during system suspend?
> >
> > By moving dma_chan_put() after the block that clears chan->slave, the
> > hardware channel may still be active while chan->slave is NULL. If a
> > system suspend occurs concurrently, drivers like stm32-dma3 might check
> > the hardware state and attempt to print the slave device name:
> >
> > drivers/dma/stm32/stm32-dma3.c:stm32_dma3_pm_suspend() {
> >     ...
> >       ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id));
> >       if (ccr & CCR_EN) {
> >               dev_warn(dev, "Suspend is prevented: %s still in use by %s\n",
> >                        dma_chan_name(c), dev_name(c->slave));
> >     ...
> > }
> >
> > Since dma_chan_put() (which stops the hardware) hasn't been called yet,
> > CCR_EN would still be active, and dev_name(c->slave) would dereference
> > the NULL pointer we just set.
> >
> > Is there a way to stop the channel before clearing the slave pointer
> > without triggering the original use-after-free?
>
> This is a valid concern.
> So, either we can have a NULL check in stm32 code while printing device
> name, or drop dma_chan_put() change for now to avoid this trigger?

Drop dma_chan_put() firstly.

Frank

>
> Thanks,
> Shivank
>
>
>