[PATCH v4 0/4] dmaengine: fix kref underflow and UAF in dma_chan_put()

[PATCH v4 0/4] dmaengine: fix kref underflow and UAF in dma_chan_put()

[Shivank Garg]

Fix bugs related to dma_chan_put(), found while testing with SDXI[1].

[1]: https://lore.kernel.org/dmaengine/20260605-sdxi-base-v3-0-4d38ca2bdffe@amd.com

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
Changes in v4:
- Add dma_device_get() helper (Frank)
- Drop dma_chan_put() move change (Frank)
- Link to v3: https://lore.kernel.org/r/20260816-dmaengine-kref-fix-v3-0-7e76187145df@amd.com

Changes in v3:
- Add patch 3: add synchronize_rcu() to wait for RCU readers to prevent
  use-after-free. (Sashiko)
- Link to v2: https://lore.kernel.org/r/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com

Changes in v2:
- Add patch 2 fixing the dma_chan_put()/dma_release_channel() use-after-free (sashiko)
- Link to v1: https://lore.kernel.org/r/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com

---
Shivank Garg (4):
      dmaengine: add dma_device_get() helper
      dmaengine: Fix device kref underflow in dma_chan_put()
      dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
      dmaengine: wait for RCU readers before releasing dma_device

 drivers/dma/dmaengine.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)
---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260518-dmaengine-kref-fix-7b21acb09455

Best regards,
-- 
Shivank Garg <shivankg@amd.com>

[PATCH v4 1/4] dmaengine: add dma_device_get() helper

[Shivank Garg]

Add dma_device_get() helper to match dma_device_put() to make code
symmetric. It wraps open-coded kref_get_unless_zero() and asserts that
dma_list_mutex is held, matching its put counterpart.

No functional change intended.

Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 drivers/dma/dmaengine.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 6ffd8bd82154..77638dc16e71 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -433,6 +433,12 @@ static void dma_device_release(struct kref *ref)
 		device->device_release(device);
 }
 
+static bool dma_device_get(struct dma_device *device)
+{
+	lockdep_assert_held(&dma_list_mutex);
+	return kref_get_unless_zero(&device->ref);
+}
+
 static void dma_device_put(struct dma_device *device)
 {
 	lockdep_assert_held(&dma_list_mutex);
@@ -460,8 +466,7 @@ static int dma_chan_get(struct dma_chan *chan)
 	if (!try_module_get(owner))
 		return -ENODEV;
 
-	ret = kref_get_unless_zero(&chan->device->ref);
-	if (!ret) {
+	if (!dma_device_get(chan->device)) {
 		ret = -ENODEV;
 		goto module_put_out;
 	}

-- 
2.43.0

[PATCH v4 2/4] dmaengine: Fix device kref underflow in dma_chan_put()

[Shivank Garg]

dma_chan_get() takes chan->device->ref only on the slow path:

	/* no kref on fast path */
	if (chan->client_count) {
		__module_get(owner);
		chan->client_count++;
		return 0;
	}
	if (!try_module_get(owner))
		return -ENODEV;
	if (!dma_device_get(chan->device)) { // calls kref_get_unless_zero()

dma_chan_put() drops the ref unconditionally, so every fast-path
get/put pair drops one extra device reference.

The bug fires when two conditions hold together: a non-private
provider has a persistent client holding chan->client_count > 0
and another client cycles dmaengine_get()/dmaengine_put().
When the kref hits zero, the subsequent dma_find_channel() returns
NULL even though the provider module is still loaded.

Fix this by dropping device->ref only on the last put, matching the
single slow-path get.

Fixes: 8ad342a86359 ("dmaengine: Add reference counting to dma_device struct")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 drivers/dma/dmaengine.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 77638dc16e71..3ae53e11e54a 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -520,7 +520,9 @@ static void dma_chan_put(struct dma_chan *chan)
 		chan->route_data = NULL;
 	}
 
-	dma_device_put(chan->device);
+	/* 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));
 }
 

-- 
2.43.0

[PATCH v4 3/4] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

[Shivank Garg]

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.

dma_chan_put() then still reads chan->device->owner via
dma_chan_to_owner() for the trailing module_put(). KASAN catches it:

	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.

Fixes: 8ad342a86359 ("dmaengine: Add reference counting to dma_device struct")
Suggested-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 drivers/dma/dmaengine.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 3ae53e11e54a..d075051dd187 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -500,10 +500,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 */
@@ -523,7 +526,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)

-- 
2.43.0

[PATCH v4 4/4] dmaengine: wait for RCU readers before releasing dma_device

[Shivank Garg]

dma_issue_pending_all() walks the dma_device_list with
list_for_each_entry_rcu() under rcu_read_lock(). dma_device_release()
unlinks the device with list_del_rcu() and then calls
device->device_release() (which in many drivers, such as plx_dma.c,
directly calls kfree()).

Because there is no grace period between unlinking the device and
freeing it, concurrent RCU readers in dma_issue_pending_all() can
access the device after it has been freed.

The lockless walk originally relied on clients holding a dmaengine
reference to pin the provider module, and therefore the device, for as
long as they might traverse the list. Commit 8ad342a86359 ("dmaengine:
Add reference counting to dma_device struct") decoupled the dma_device
lifetime from the module reference, so the device can now be released
while a reader is still walking the list.

Add synchronize_rcu() before the device is freed, so RCU readers are
guaranteed to have finished. Keep it unconditional: providers that do
not implement device_release() free the device themselves once
dma_async_device_unregister() returns, so they need the same grace
period.

Fixes: 2ba05622b8b1 ("dmaengine: provide a common 'issue_pending_all' implementation")
Suggested-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 drivers/dma/dmaengine.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index d075051dd187..604c9af19936 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -429,6 +429,12 @@ static void dma_device_release(struct kref *ref)
 	list_del_rcu(&device->global_node);
 	dma_channel_rebalance();
 
+	/*
+	 * Wait for RCU readers (e.g. dma_issue_pending_all()) that may still
+	 * be traversing dma_device_list before the device is freed.
+	 */
+	synchronize_rcu();
+
 	if (device->device_release)
 		device->device_release(device);
 }

-- 
2.43.0

Re: [PATCH v4 1/4] dmaengine: add dma_device_get() helper

[Logan Gunthorpe]

On 2026-08-17 21:43, Shivank Garg wrote:
> Add dma_device_get() helper to match dma_device_put() to make code
> symmetric. It wraps open-coded kref_get_unless_zero() and asserts that
> dma_list_mutex is held, matching its put counterpart.
> 
> No functional change intended.
> 
> Suggested-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>

Makes sense to me:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 2/4] dmaengine: Fix device kref underflow in dma_chan_put()

[Logan Gunthorpe]

On 2026-08-17 21:43, Shivank Garg wrote:
> dma_chan_get() takes chan->device->ref only on the slow path:
> 
> 	/* no kref on fast path */
> 	if (chan->client_count) {
> 		__module_get(owner);
> 		chan->client_count++;
> 		return 0;
> 	}
> 	if (!try_module_get(owner))
> 		return -ENODEV;
> 	if (!dma_device_get(chan->device)) { // calls kref_get_unless_zero()
> 
> dma_chan_put() drops the ref unconditionally, so every fast-path
> get/put pair drops one extra device reference.
> 
> The bug fires when two conditions hold together: a non-private
> provider has a persistent client holding chan->client_count > 0
> and another client cycles dmaengine_get()/dmaengine_put().
> When the kref hits zero, the subsequent dma_find_channel() returns
> NULL even though the provider module is still loaded.
> 
> Fix this by dropping device->ref only on the last put, matching the
> single slow-path get.
> 
> Fixes: 8ad342a86359 ("dmaengine: Add reference counting to dma_device struct")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>

Looks good to me, thanks.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

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

[Logan Gunthorpe]

On 2026-08-17 21:43, Shivank Garg wrote:
> 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.
> 
> dma_chan_put() then still reads chan->device->owner via
> dma_chan_to_owner() for the trailing module_put(). KASAN catches it:
> 
> 	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.
> 
> Fixes: 8ad342a86359 ("dmaengine: Add reference counting to dma_device struct")
> Suggested-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>

Nice catch, looks right to me and the patch is really easy to
understand. Thanks!

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Re: [PATCH v4 1/4] dmaengine: add dma_device_get() helper

[Frank Li]

On Tue, Aug 18, 2026 at 03:43:45AM +0000, Shivank Garg wrote:
> Add dma_device_get() helper to match dma_device_put() to make code
> symmetric. It wraps open-coded kref_get_unless_zero() and asserts that
> dma_list_mutex is held, matching its put counterpart.
>
> No functional change intended.
>
> Suggested-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  drivers/dma/dmaengine.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 6ffd8bd82154..77638dc16e71 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -433,6 +433,12 @@ static void dma_device_release(struct kref *ref)
>  		device->device_release(device);
>  }
>
> +static bool dma_device_get(struct dma_device *device)

Please the same return type of kref_get_unless_zero(),

it should be int.

Frank

> +{
> +	lockdep_assert_held(&dma_list_mutex);
> +	return kref_get_unless_zero(&device->ref);
> +}
> +
>  static void dma_device_put(struct dma_device *device)
>  {
>  	lockdep_assert_held(&dma_list_mutex);
> @@ -460,8 +466,7 @@ static int dma_chan_get(struct dma_chan *chan)
>  	if (!try_module_get(owner))
>  		return -ENODEV;
>
> -	ret = kref_get_unless_zero(&chan->device->ref);
> -	if (!ret) {
> +	if (!dma_device_get(chan->device)) {
>  		ret = -ENODEV;
>  		goto module_put_out;
>  	}
>
> --
> 2.43.0
>

Re: [PATCH v4 4/4] dmaengine: wait for RCU readers before releasing dma_device

[Logan Gunthorpe]

On 2026-08-17 21:43, Shivank Garg wrote:
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index d075051dd187..604c9af19936 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -429,6 +429,12 @@ static void dma_device_release(struct kref *ref)
>  	list_del_rcu(&device->global_node);
>  	dma_channel_rebalance();
>  
> +	/*
> +	 * Wait for RCU readers (e.g. dma_issue_pending_all()) that may still
> +	 * be traversing dma_device_list before the device is freed.
> +	 */
> +	synchronize_rcu();
> +
>  	if (device->device_release)
>  		device->device_release(device);
>  }
> 

A couple minor points on this:

1. I think the comment, as is, isn't useful. It's pretty clear the
synchronize_rcu() call is paired with the list_del_rcu() call above it.

2. The new synchronize_rcu() call will delay for a grace period with the
dma_list_mutex lock held. Is that okay? It might be, but if so, it could
use a note in the commit message.

Besides that, these are minor points and the patch does fix a real issue so:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks,

Logan

Re: [PATCH v4 1/4] dmaengine: add dma_device_get() helper

[Garg, Shivank]

On Tue, 2026-08-18 at 11:12 -0500, Frank Li wrote:
> On Tue, Aug 18, 2026 at 03:43:45AM +0000, Shivank Garg wrote:
> > Add dma_device_get() helper to match dma_device_put() to make code
> > symmetric. It wraps open-coded kref_get_unless_zero() and asserts that
> > dma_list_mutex is held, matching its put counterpart.
> > 
> > No functional change intended.
> > 
> > Suggested-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Shivank Garg <shivankg@amd.com>
> > ---
> >  drivers/dma/dmaengine.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index 6ffd8bd82154..77638dc16e71 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -433,6 +433,12 @@ static void dma_device_release(struct kref *ref)
> >  		device->device_release(device);
> >  }
> > 
> > +static bool dma_device_get(struct dma_device *device)
> 
> Please the same return type of kref_get_unless_zero(),
> 
> it should be int.
> 

Thanks, refcount_inc_not_zero() (and callees) returns bool to
kref_get_unless_zero(). I used bool because of boolean result value,
but I have no strong opinion here. I'll change dma_device_get() to
return int to match kref_get_unless_zero().


I think I should also add __must_check attribute, since kref_get_unless_zero()
has it.

Thanks,
Shivank

Re: [PATCH v4 4/4] dmaengine: wait for RCU readers before releasing dma_device

[Garg, Shivank]

On Tue, 2026-08-18 at 10:21 -0600, Logan Gunthorpe wrote:
> 
> On 2026-08-17 21:43, Shivank Garg wrote:
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index d075051dd187..604c9af19936 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -429,6 +429,12 @@ static void dma_device_release(struct kref *ref)
> >  	list_del_rcu(&device->global_node);
> >  	dma_channel_rebalance();
> >  
> > +	/*
> > +	 * Wait for RCU readers (e.g. dma_issue_pending_all()) that may still
> > +	 * be traversing dma_device_list before the device is freed.
> > +	 */
> > +	synchronize_rcu();
> > +
> >  	if (device->device_release)
> >  		device->device_release(device);
> >  }
> > 
> 
> A couple minor points on this:
> 
> 1. I think the comment, as is, isn't useful. It's pretty clear the
> synchronize_rcu() call is paired with the list_del_rcu() call above it.
> 

Sure, I'll drop it.

> 2. The new synchronize_rcu() call will delay for a grace period with the
> dma_list_mutex lock held. Is that okay? It might be, but if so, it could
> use a note in the commit message.
> 

yeah, I believe it is okay as the added latency is confined to only
teardown path. I'll add note.

> Besides that, these are minor points and the patch does fix a real issue so:
> 
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> 
> 

Thanks for the review

Best regards,
Shivank

Re: [PATCH v4 1/4] dmaengine: add dma_device_get() helper

[Frank Li]

On Tue, Aug 18, 2026 at 04:27:51PM +0000, Garg, Shivank wrote:
> On Tue, 2026-08-18 at 11:12 -0500, Frank Li wrote:
> > On Tue, Aug 18, 2026 at 03:43:45AM +0000, Shivank Garg wrote:
> > > Add dma_device_get() helper to match dma_device_put() to make code
> > > symmetric. It wraps open-coded kref_get_unless_zero() and asserts that
> > > dma_list_mutex is held, matching its put counterpart.
> > >
> > > No functional change intended.
> > >
> > > Suggested-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Shivank Garg <shivankg@amd.com>
> > > ---
> > >  drivers/dma/dmaengine.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > > index 6ffd8bd82154..77638dc16e71 100644
> > > --- a/drivers/dma/dmaengine.c
> > > +++ b/drivers/dma/dmaengine.c
> > > @@ -433,6 +433,12 @@ static void dma_device_release(struct kref *ref)
> > >  		device->device_release(device);
> > >  }
> > >
> > > +static bool dma_device_get(struct dma_device *device)
> >
> > Please the same return type of kref_get_unless_zero(),
> >
> > it should be int.
> >
>
> Thanks, refcount_inc_not_zero() (and callees) returns bool to
> kref_get_unless_zero(). I used bool because of boolean result value,
> but I have no strong opinion here. I'll change dma_device_get() to
> return int to match kref_get_unless_zero().
>
>
> I think I should also add __must_check attribute, since kref_get_unless_zero()
> has it.

Yes,

Frank

>
> Thanks,
> Shivank