DMA Engine development
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: "Garg, Shivank" <shivankg@amd.com>
Cc: "sashiko-reviews@lists.linux.dev"
	<sashiko-reviews@lists.linux.dev>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"vkoul@kernel.org" <vkoul@kernel.org>,
	"Frank.Li@kernel.org" <Frank.Li@kernel.org>
Subject: Re: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
Date: Mon, 17 Aug 2026 09:26:24 -0500	[thread overview]
Message-ID: <aoMaEEOgQhZduJoe@SMW015318> (raw)
In-Reply-To: <ba6d6fc86076b4f2e5244cad3c5c70d782dff74a.camel@amd.com>

On Sun, Aug 16, 2026 at 06:05:27PM +0000, Garg, Shivank wrote:
> [You don't often get email from shivankg@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Sun, 2026-08-16 at 16:07 +0000, sashiko-bot@kernel.org 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 <shivankg@amd.com>
> >
> > 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/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com
> >
> > > --- 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
>
>
>

  reply	other threads:[~2026-08-17 14:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-16 15:49 [PATCH v3 0/3] dmaengine: fix kref underflow and UAF in dma_chan_put() Shivank Garg
2026-08-16 15:49 ` [PATCH v3 1/3] dmaengine: Fix device kref underflow " Shivank Garg
2026-08-16 16:02   ` sashiko-bot
2026-08-16 18:14     ` Garg, Shivank
2026-08-17 14:17   ` Frank Li
2026-08-16 15:49 ` [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel() Shivank Garg
2026-08-16 16:07   ` sashiko-bot
2026-08-16 18:05     ` Garg, Shivank
2026-08-17 14:26       ` Frank Li [this message]
2026-08-17 14:24   ` Frank Li
2026-08-16 15:49 ` [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device Shivank Garg
2026-08-16 16:10   ` sashiko-bot
2026-08-17 14:29   ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoMaEEOgQhZduJoe@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=shivankg@amd.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox