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

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?

Thanks,
Shivank