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

On Mon, 2026-08-17 at 09:26 -0500, Frank Li wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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
> 
> 

Sure

Thanks,
Shivank