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

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 <[email protected]>

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/[email protected]

> 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/[email protected]?part=4