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

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

[Shivank Garg]

Fix bugs related to dma_chan_put().

Signed-off-by: Shivank Garg <shivankg@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 (2):
      dmaengine: Fix device kref underflow in dma_chan_put()
      dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

 drivers/dma/dmaengine.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
---
base-commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7
change-id: 20260518-dmaengine-kref-fix-7b21acb09455

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

[PATCH v2 1/2] 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;
	ret = kref_get_unless_zero(&chan->device->ref);

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")
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 405bd2fbb4a3b94fd0bf44526f656f6a19feaad0..605bfa477a004cc0b03957ffb85a52308f903441 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -515,7 +515,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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] RCU Use-After-Free during device release.
- [High] Double `list_del_rcu()` leading to list corruption and kernel crash.
--

commit c36a9f0553b166fe38c0d70bbc4a093494dada44
Author: Shivank Garg <shivankg@amd.com>

dmaengine: Fix device kref underflow in dma_chan_put()

This commit fixes a kref underflow in dma_chan_put() by ensuring that the
device reference is dropped only on the last put, matching the slow-path
get. It prevents situations where a fast-path get/put pair drops an extra
reference, leading to a zero kref and a subsequent dma_find_channel()
returning NULL.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 405bd2fbb4a3b..605bfa477a004 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -515,7 +515,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);

[Severity: Critical]
This is a pre-existing issue, but does device release here allow an RCU
use-after-free regression?

In dma_issue_pending_all(), the dma_device_list is iterated using
list_for_each_entry_rcu() under rcu_read_lock().

However, when a device is released, dma_device_release() removes it from
the list with list_del_rcu() and immediately calls device->device_release()
(which in many drivers, such as plx_dma.c, directly calls kfree()).

Because there is no synchronize_rcu() or equivalent mechanism to wait
for the RCU grace period before freeing the memory, concurrent RCU
readers in dma_issue_pending_all() can access freed memory, leading to
a use-after-free.

[Severity: High]
This is a pre-existing issue, but can the module unloading sequence
lead to a double list_del_rcu() regression and list corruption?

In dmaengine_get(), if dma_chan_get(chan) returns -ENODEV (which occurs
when the driver module is unloading and try_module_get() fails), it
explicitly calls list_del_rcu(&device->global_node).

Subsequently, the module's exit routine will call
dma_async_device_unregister(), which eventually calls dma_device_put().

If the reference count drops to zero here, dma_device_release() runs and
unconditionally calls list_del_rcu(&device->global_node) again.

Calling list_del_rcu() on an already unlinked list node manipulates
LIST_POISON pointers, predictably leading to a kernel crash.

>  	module_put(dma_chan_to_owner(chan));
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com?part=1

[PATCH v2 2/2] 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.

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() first, then reads
   chan->device->privatecnt, chan->slave, chan->name 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 <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 drivers/dma/dmaengine.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 605bfa477a004cc0b03957ffb85a52308f903441..9c4e206f246864ee185e8d9df96a89014d9e6edf 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)
@@ -907,7 +910,6 @@ void dma_release_channel(struct dma_chan *chan)
 	mutex_lock(&dma_list_mutex);
 	WARN_ONCE(chan->client_count != 1,
 		  "chan reference count %d != 1\n", chan->client_count);
-	dma_chan_put(chan);
 	/* drop PRIVATE cap enabled by __dma_request_channel() */
 	if (--chan->device->privatecnt == 0)
 		dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
@@ -924,6 +926,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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Use-After-Free in dmaengine_put() during channel iteration.
- [Critical] Use-After-Free in dma_release_channel() after forceful DMA provider unbind.
--

commit 5c65161b4e60f0575ac687342210c12a79a7958e
Author: Shivank Garg <shivankg@amd.com>

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

This commit resolves use-after-free bugs in the DMA engine channel
management by caching the module owner prior to releasing channel
references and reordering the cleanup in dma_release_channel().

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

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 605bfa477a004..9c4e206f24686 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
[ ... ]
> @@ -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);
>  }

[Severity: Critical]
This is a pre-existing issue, but can this lead to a use-after-free
regression during channel iteration?

If list_for_each_entry() iterates over a device's channels in
dmaengine_put():

dmaengine_put() {
    list_for_each_entry(chan, &device->channels, device_node)
        dma_chan_put(chan);
}

When dma_chan_put() drops the final client_count reference, it triggers
dma_device_put(), which can free the device and its embedded channels via
dma_device_release().

When execution returns to the loop macro, evaluating chan = list_next_entry()
would dereference the freed chan->device_node.next pointer. Even
list_for_each_entry_safe() appears insufficient here since the list head is
also inside the freed device.

[ ... ]
> @@ -924,6 +926,7 @@ void dma_release_channel(struct dma_chan *chan)
>  	kfree(chan->dbg_client_name);
>  	chan->dbg_client_name = NULL;
>  #endif
> +	dma_chan_put(chan);

[Severity: Critical]
This is a pre-existing issue, but can this code trigger a use-after-free
regression if a DMA provider is forcefully unbound?

If a provider is unbound via sysfs while clients hold channel references,
dma_async_device_unregister() iterates over channels and unregisters them:

__dma_async_device_channel_unregister() {
    ...
    device_unregister(&chan->dev->device);
}

This drops the reference to chan->dev and frees it without setting the
pointer to NULL.

Later, when the client driver closes the channel via dma_release_channel(),
it calls sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME) before
reaching the dma_chan_put() call.

Will this dereference the dangling chan->dev pointer?

>  	mutex_unlock(&dma_list_mutex);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com?part=2