Re: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
Frank Li <[email protected]>
| Newsgroups | org.kernel.vger.dmaengine,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <aoMZrvEp4aNgaE46@SMW015318> |
On Sun, Aug 16, 2026 at 03:49:26PM +0000, Shivank Garg wrote: > [You don't often get email from [email protected]. 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 <[email protected]> > Link: https://sashiko.dev/#/patchset/[email protected] > Signed-off-by: Shivank Garg <[email protected]> > --- Reviewed-by: Frank Li <[email protected]> > 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 >