* [PATCH v3 0/3] dmaengine: fix kref underflow and UAF in dma_chan_put()
@ 2026-08-16 15:49 Shivank Garg
2026-08-16 15:49 ` [PATCH v3 1/3] dmaengine: Fix device kref underflow " Shivank Garg
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Shivank Garg @ 2026-08-16 15:49 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Logan Gunthorpe, Andrew Morton
Cc: stable, dmaengine, linux-kernel, Shivank Garg, Sashiko
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 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 (3):
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 | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260518-dmaengine-kref-fix-7b21acb09455
Best regards,
--
Shivank Garg <shivankg@amd.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] dmaengine: Fix device kref underflow in dma_chan_put()
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 ` Shivank Garg
2026-08-16 16:02 ` sashiko-bot
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 15:49 ` [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device Shivank Garg
2 siblings, 1 reply; 9+ messages in thread
From: Shivank Garg @ 2026-08-16 15:49 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Logan Gunthorpe, Andrew Morton
Cc: stable, dmaengine, linux-kernel, 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 6ffd8bd82154..516d6d933208 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
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 15:49 ` Shivank Garg
2026-08-16 16:07 ` sashiko-bot
2026-08-16 15:49 ` [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device Shivank Garg
2 siblings, 1 reply; 9+ messages in thread
From: Shivank Garg @ 2026-08-16 15:49 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Logan Gunthorpe, Andrew Morton
Cc: stable, dmaengine, linux-kernel, Shivank Garg, Sashiko
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 <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 | 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device
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 15:49 ` [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel() Shivank Garg
@ 2026-08-16 15:49 ` Shivank Garg
2026-08-16 16:10 ` sashiko-bot
2 siblings, 1 reply; 9+ messages in thread
From: Shivank Garg @ 2026-08-16 15:49 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Logan Gunthorpe, Andrew Morton
Cc: stable, dmaengine, linux-kernel, Shivank Garg, Sashiko
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
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 bf491eb10596..29b128aa0bea 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dmaengine: Fix device kref underflow in dma_chan_put()
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
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-08-16 16:02 UTC (permalink / raw)
To: Shivank Garg; +Cc: dmaengine, Frank.Li, vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] dma_chan_put() accesses chan->device->owner after potentially freeing chan and chan->device via dma_device_put().
- [High] dma_release_channel() accesses `chan` fields after calling `dma_chan_put()`, which may have freed the channel.
- [High] RCU read-side critical section in `dma_issue_pending_all()` can access a synchronously freed `struct dma_device`.
--
--- Patch [1]: [PATCH v3 1/3] dmaengine: Fix device kref underflow in dma_chan_put() ---
commit 0f010c1f7452238748341b884bda3d3bba2c74ac
Author: Shivank Garg <shivankg@amd.com>
dmaengine: Fix device kref underflow in dma_chan_put()
This commit fixes a device reference underflow bug by ensuring the device
reference is only dropped on the last put of a channel, matching the behavior
of the get path.
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 6ffd8bd82154a..516d6d9332083 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));
[Severity: High]
This is a pre-existing issue, but does accessing chan after dma_device_put()
risk a use-after-free?
If dma_device_put() drops the last reference to the DMA device, it can
trigger synchronous release of the device memory, which might also free the
embedded dma_chan.
Could dma_chan_to_owner(chan) then dereference freed memory by evaluating
chan->device->owner?
> }
[Severity: High]
This isn't a bug introduced by this patch, but does dma_release_channel()
also access freed memory under the same conditions?
drivers/dma/dmaengine.c:dma_release_channel() {
...
dma_chan_put(chan);
if (chan->slave) {
sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);
...
}
If dma_chan_put() frees the channel when the last device reference is dropped,
is it safe to access chan->slave and chan->dev afterwards to tear down
the sysfs links?
[Severity: High]
This is also a pre-existing issue, but does the RCU read side in
dma_issue_pending_all() race with synchronous device release?
dma_issue_pending_all() relies on an RCU read lock to traverse the device list:
drivers/dma/dmaengine.c:dma_issue_pending_all() {
...
rcu_read_lock();
list_for_each_entry_rcu(device, &dma_device_list, global_node) {
if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
...
}
However, dma_device_release() unlinks the device and synchronously frees
it without observing 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);
...
}
Could the RCU reader dereference a struct dma_device that was just
freed by dma_device_release()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260816-dmaengine-kref-fix-v3-0-7e76187145df@amd.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
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
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-08-16 16:07 UTC (permalink / raw)
To: Shivank Garg; +Cc: vkoul, Frank.Li, dmaengine
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?
> mutex_unlock(&dma_list_mutex);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260816-dmaengine-kref-fix-v3-0-7e76187145df@amd.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device
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
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-16 16:10 UTC (permalink / raw)
To: Shivank Garg; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Double list_del_rcu() causes list corruption when DMA provider module unloads concurrently with a channel request.
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition between `dma_issue_pending_all()` and `dma_chan_put()` leads to memory corruption by operating on freed DMA channel resources.
--
--- Patch [3]: [PATCH v3 3/3] dmaengine: wait for RCU readers before releasing dma_device ---
commit 6738eea6e01509c5c24f2cdb086fe3fd0ffab13a
Author: Shivank Garg <shivankg@amd.com>
dmaengine: wait for RCU readers before releasing dma_device
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(). This patch adds synchronize_rcu() before
freeing the device so RCU readers are guaranteed to have finished.
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 bf491eb10596d..29b128aa0beaa 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 isn't a bug introduced by this patch, but can this lead to a double
list_del_rcu() execution and list corruption if the module is unloaded
concurrently with a channel request?
If a thread calls dmaengine_get() or find_candidate(), it iterates
dma_device_list and calls dma_chan_get(). If another thread concurrently
unloads the module:
1. dma_chan_get() calls try_module_get(), which fails and returns -ENODEV
2. dmaengine_get() evaluates -ENODEV and eagerly deletes the device via
list_del_rcu(&device->global_node)
3. Later, the unloading thread unregisters the device, which invokes
dma_device_release()
4. dma_device_release() unconditionally calls
list_del_rcu(&device->global_node) again on the already removed node
Calling list_del_rcu() twice on the same node poisons the prev pointer,
which can cause a panic in __list_del_entry().
> 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();
[Severity: High]
This is a pre-existing issue, but does dma_issue_pending_all() still have
a TOCTOU race condition with dma_chan_put() that could lead to memory
corruption?
While this synchronize_rcu() protects the dma_device itself during driver
unbind, it doesn't seem to protect against channel release.
If dma_issue_pending_all() evaluates chan->client_count locklessly
under rcu_read_lock():
Thread A:
dma_issue_pending_all()
if (chan->client_count)
// preempted here
Thread B:
dma_release_channel()
dma_chan_put()
// drops client_count to 0
chan->device->device_free_chan_resources(chan);
Thread A resumes:
device->device_issue_pending(chan);
The driver writes to the hardware, forcing it to fetch from the newly
freed DMA ring. Can the hardware be safely started here if the resources
were just freed?
> +
> if (device->device_release)
> device->device_release(device);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260816-dmaengine-kref-fix-v3-0-7e76187145df@amd.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/3] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()
2026-08-16 16:07 ` sashiko-bot
@ 2026-08-16 18:05 ` Garg, Shivank
0 siblings, 0 replies; 9+ messages in thread
From: Garg, Shivank @ 2026-08-16 18:05 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, Frank.Li@kernel.org
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?
Thanks,
Shivank
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dmaengine: Fix device kref underflow in dma_chan_put()
2026-08-16 16:02 ` sashiko-bot
@ 2026-08-16 18:14 ` Garg, Shivank
0 siblings, 0 replies; 9+ messages in thread
From: Garg, Shivank @ 2026-08-16 18:14 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, Frank.Li@kernel.org
On Sun, 2026-08-16 at 16:02 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] dma_chan_put() accesses chan->device->owner after potentially freeing chan and chan->device via dma_device_put().
> - [High] dma_release_channel() accesses `chan` fields after calling `dma_chan_put()`, which may have freed the channel.
> - [High] RCU read-side critical section in `dma_issue_pending_all()` can access a synchronously freed `struct dma_device`.
> --
>
> --- Patch [1]: [PATCH v3 1/3] dmaengine: Fix device kref underflow in dma_chan_put() ---
> commit 0f010c1f7452238748341b884bda3d3bba2c74ac
> Author: Shivank Garg <shivankg@amd.com>
>
> dmaengine: Fix device kref underflow in dma_chan_put()
>
> This commit fixes a device reference underflow bug by ensuring the device
> reference is only dropped on the last put of a channel, matching the behavior
> of the get path.
>
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index 6ffd8bd82154a..516d6d9332083 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));
>
> [Severity: High]
> This is a pre-existing issue, but does accessing chan after dma_device_put()
> risk a use-after-free?
>
> If dma_device_put() drops the last reference to the DMA device, it can
> trigger synchronous release of the device memory, which might also free the
> embedded dma_chan.
>
> Could dma_chan_to_owner(chan) then dereference freed memory by evaluating
> chan->device->owner?
>
I'm already fixing this in patch 2 by caching chan->device->owner.
> > }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does dma_release_channel()
> also access freed memory under the same conditions?
>
> drivers/dma/dmaengine.c:dma_release_channel() {
> ...
> dma_chan_put(chan);
>
> if (chan->slave) {
> sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);
> ...
> }
>
> If dma_chan_put() frees the channel when the last device reference is dropped,
> is it safe to access chan->slave and chan->dev afterwards to tear down
> the sysfs links?
I addressed this in patch 2, but Sashiko pointed the fix would cause NULL
dereference in stm32_dma3_pm_suspend().
>
> [Severity: High]
> This is also a pre-existing issue, but does the RCU read side in
> dma_issue_pending_all() race with synchronous device release?
>
> dma_issue_pending_all() relies on an RCU read lock to traverse the device list:
>
> drivers/dma/dmaengine.c:dma_issue_pending_all() {
> ...
> rcu_read_lock();
> list_for_each_entry_rcu(device, &dma_device_list, global_node) {
> if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
> ...
> }
>
> However, dma_device_release() unlinks the device and synchronously frees
> it without observing 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);
> ...
> }
>
> Could the RCU reader dereference a struct dma_device that was just
> freed by dma_device_release()?
>
Fixed in Patch 3 by adding synchronize_rcu().
Thanks,
Shivank
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-16 18:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-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-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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).