[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 3/4] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free in dma_release_channel() due to accessing 'chan' after dropping its reference via dma_chan_put()
--

--- Patch [3]: [PATCH v4 3/4] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel() ---
commit 4ca601ffe76f0ae6fcc1118c90ce61d882e870d0
Author: Shivank Garg <shivankg@amd.com>

dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

This commit addresses a use-after-free in dma_chan_put() where the channel's
device owner is accessed after the device reference is dropped. It caches the
module owner before calling dma_device_put() to prevent KASAN warnings.

Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com

> dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

[Severity: High]
This is a pre-existing issue, but the commit title mentions fixing a
use-after-free in dma_release_channel(), yet the patch doesn't modify that
function.

Looking at dma_release_channel() in drivers/dma/dmaengine.c, there appears to
still be a use-after-free if dma_chan_put() drops the last reference and frees
the channel:

void dma_release_channel(struct dma_chan *chan)
{
	...
	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);
		...
	}
	...
}

Does this code still cause a use-after-free if dma_chan_put() triggers the
release of the dma_device and its embedded channel structures?

If the underlying DMA device has been unregistered, dropping the channel's
client count could drop the final dma_device kref and free the channel. The
subsequent accesses to chan->slave, chan->dev, and chan->name would then
dereference freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-dmaengine-kref-fix-v4-0-c6ef991462a0@amd.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double list_del_rcu() on device->global_node causing list corruption or crash when a provider module is removed.
--

--- Patch [4]: [PATCH v4 4/4] dmaengine: wait for RCU readers before releasing dma_device ---
commit 82fe03be2ee5cd7002af0d140a5056712e128cb6
Author: Shivank Garg <shivankg@amd.com>

dmaengine: wait for RCU readers before releasing dma_device

This commit adds a synchronize_rcu() call to dma_device_release() before
the device is freed. This ensures concurrent RCU readers traversing the
dma_device_list finish their traversal, preventing use-after-free bugs.

Link: https://sashiko.dev/#/patchset/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index d075051dd187c..604c9af199369 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);

[Severity: High]
This is a pre-existing issue, but does this unconditional removal run the
risk of a double list_del_rcu() causing list corruption?

If a DMA provider module is being unloaded concurrently, try_module_get()
can fail in dma_chan_get(). The error paths in find_candidate() and
dmaengine_get() proactively unlink the device:

find_candidate() {
    ...
    err = dma_chan_get(chan);
    if (err) {
        if (err == -ENODEV) {
            ...
            list_del_rcu(&device->global_node);
        ...
}

However, they do not appear to drop the kref. Later, the module unload
process calls dma_async_device_unregister(), which puts the final kref
and triggers dma_device_release().

Since dma_device_release() unconditionally calls list_del_rcu() again on
the same node, couldn't this crash the kernel by dereferencing LIST_POISON2
during the second removal?

>  	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);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-dmaengine-kref-fix-v4-0-c6ef991462a0@amd.com?part=4

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Dereferencing `chan->device->owner` in `dma_chan_put()` after `dma_device_put(chan->device)` may access freed device memory.
- [High] Accessing `chan` members in `dma_release_channel()` after dropping the final reference in `dma_chan_put()`.
- [High] Traversing the RCU list in `dma_issue_pending_all` races with device memory freeing due to a missing RCU grace period.
- [High] New channels can be requested and allocated from a DMA device that has already been unregistered and unbound.
--

--- Patch [2]: [PATCH v4 2/4] dmaengine: Fix device kref underflow in dma_chan_put() ---
commit 7dc19aec12ae9d7422f3459f7a5276b21afb0ad9
Author: Shivank Garg <shivankg@amd.com>

dmaengine: Fix device kref underflow in dma_chan_put()

This commit addresses a device reference counting underflow by ensuring
dma_device_put() is only called when chan->client_count reaches zero. It
matches the slow-path reference taken in dma_chan_get().

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 77638dc16e71d..3ae53e11e54a6 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));

[Severity: High]
This is a pre-existing issue, but does this code risk a use-after-free of the
device memory?

When dma_device_put() drops the final reference, the driver's release callback
can free the device memory. Evaluating dma_chan_to_owner(chan) immediately
afterward will dereference chan->device->owner from the potentially freed
device.

[Severity: High]
This isn't a bug introduced by this patch, but can we access a freed channel
structure in dma_release_channel()?

dma_release_channel() calls dma_chan_put(chan). If this drops the last device
reference, the DMA device and its embedded dma_chan structures can be freed.

Immediately following this, dma_release_channel() accesses the channel to
perform cleanup tasks:

drivers/dma/dmaengine.c:dma_release_channel() {
	...
	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);
	...
}

Does this sequence cause memory corruption if the channel is freed?

[Severity: High]
This is a pre-existing issue, but is there a race condition between
traversing the RCU list and freeing device memory?

dma_issue_pending_all() iterates over dma_device_list under an RCU read lock:

drivers/dma/dmaengine.c:dma_issue_pending_all() {
	rcu_read_lock();
	list_for_each_entry_rcu(device, &dma_device_list, global_node) {
	...
}

Concurrently, dma_device_release() removes a device from the list and
immediately calls its release callback without waiting for an RCU grace
period:

drivers/dma/dmaengine.c:dma_device_release() {
	...
	list_del_rcu(&device->global_node);
	dma_channel_rebalance();

	if (device->device_release)
		device->device_release(device);
}

If the driver synchronously frees the memory, could the concurrent traversal
in dma_issue_pending_all() dereference a freed pointer?

[Severity: High]
This isn't a bug introduced by this patch, but can new channels be requested
from a DMA device that has already been unregistered?

When a driver is unbound, dma_async_device_unregister() sets DMA_PRIVATE
to hide the device and drops its kref:

drivers/dma/dmaengine.c:dma_async_device_unregister() {
	...
	dma_cap_set(DMA_PRIVATE, device->cap_mask);
	...
}

However, if the device has other active channels, it remains in the list.
When __dma_request_channel() searches for a candidate, it calls
private_candidate():

drivers/dma/dmaengine.c:private_candidate() {
	...
	if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask))
		list_for_each_entry(chan, &dev->channels, device_node) {
	...
}

Because DMA_PRIVATE is set, this multi-channel exclusivity check is bypassed.
Could this allow an idle channel to be allocated and its hardware accessed
even after the driver has completed its cleanup and powered off the hardware?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-dmaengine-kref-fix-v4-0-c6ef991462a0@amd.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Double list_del_rcu() on device->global_node causing kernel panic.
- [High] Use-after-free in dma_chan_put() when dereferencing chan->device->owner.
- [High] Unbalanced dma_device_put() in dma_chan_put() causes kref underflow.
--

--- Patch [1]: [PATCH v4 1/4] dmaengine: add dma_device_get() helper ---
commit bb05480fd3a83327685100c3fb961c2f1ceadfda
Author: Shivank Garg <shivankg@amd.com>

dmaengine: add dma_device_get() helper

This commit adds a dma_device_get() helper function to pair with the existing
dma_device_put() for symmetry. It wraps the open-coded kref_get_unless_zero()
and adds a lockdep assertion to ensure dma_list_mutex is held.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 6ffd8bd82154a..77638dc16e71d 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);
>  }

[Severity: Critical]
This is a pre-existing issue, but can this lead to a double free causing a
kernel panic?

If a module is concurrently unloaded while a client queries for DMA channels,
try_module_get() can fail. When it fails, callers like dmaengine_get() remove
the device from the list:

drivers/dma/dmaengine.c:dmaengine_get() {
    ...
    if (err == -ENODEV) {
        /* module removed before we could use it */
        list_del_rcu(&device->global_node);
        break;
    }
    ...
}

The concurrent module unregistration will subsequently call
dma_async_device_unregister(), which drops the device reference and eventually
invokes dma_device_release(). 

However, dma_device_release() unconditionally calls
list_del_rcu(&device->global_node) again on the already unlinked node. Does
this need a check to prevent dereferencing the poisoned list pointers?

>  
> +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)) {

[Severity: High]
This isn't a bug introduced by this patch, but does this create a kref
underflow?

In dma_chan_get(), if a channel already has clients, it skips acquiring the
device reference but still increments the client count:

drivers/dma/dmaengine.c:dma_chan_get() {
    ...
    /* The channel is already in use, update client count */
    if (chan->client_count) {
        __module_get(owner);
        chan->client_count++;
        return 0;
    }
    ...
}

However, dma_chan_put() appears to call dma_device_put() unconditionally for
every decrement of client_count. Will a shared channel drop the device
reference multiple times while only acquiring it once?


[Severity: High]
This is also a pre-existing issue, but can this sequence result in a
use-after-free?

In dma_chan_put(), the device reference is dropped:

drivers/dma/dmaengine.c:dma_chan_put() {
    ...
    dma_device_put(chan->device);
    module_put(dma_chan_to_owner(chan));
    ...
}

If this is the final reference, the device struct could be freed by
dma_device_put(). The very next line expands dma_chan_to_owner(chan) into
chan->device->owner, dereferencing the potentially freed chan and chan->device
pointers.

Should the owner pointer be cached locally before dropping the device
reference?

>  		ret = -ENODEV;
>  		goto module_put_out;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-dmaengine-kref-fix-v4-0-c6ef991462a0@amd.com?part=1