Re: [PATCH v3 4/4] dmaengine: add union chan_dev for dma_chan::dev for clarity

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing put_device() on device_register() failure leads to resource leak and potential Use-After-Free.
- [High] Modifying device bus after registration circumvents the device core lifecycle and causes mismatched bus notifications.
- [High] Missing synchronize_rcu() in DMA device unregistration path leads to Use-After-Free in dma_issue_pending_all().
--

commit 99f7cae9d62138cb68f0b59f43bd77ab9e9bb860
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: add union chan_dev for dma_chan::dev for clarity

The current dma_chan structure contains both "device" and "dev", which
increases reader confusion. Add union dma_chan::chan_dev to make its
purpose explicit and clearly identify it as the per-channel device.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index a7ac2729e4d9d..bb30a9aa54d3f 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c

[ ... ]

> @@ -1122,7 +1122,7 @@ static int __dma_async_device_channel_register(struct dma_device *device,
>   err_out_ida:
>  	ida_free(&device->chan_ida, chan->chan_id);
>   err_free_dev:
> -	kfree(chan->dev);
> +	kfree(chan->chan_dev);

[Severity: High]
This is a pre-existing issue, but is it correct to use kfree() directly on the
device structure here instead of put_device() after device_register() fails?

In __dma_async_device_channel_register(), if device_register() fails (e.g.
due to memory pressure or name collision), the error path jumps to
err_free_dev. Using kfree() directly circumvents the device core reference
counting.

Does this leak the device structure's associated resources, such as its
allocated name? Also, if the device's kobject was already linked into sysfs
ksets, can this lead to a Use-After-Free?

[ ... ]

> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
> index b3327aa35de40..e76c58f213d84 100644
> --- a/drivers/dma/tegra186-gpc-dma.c
> +++ b/drivers/dma/tegra186-gpc-dma.c
> @@ -1554,7 +1554,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
>  				return dev_err_probe(chdev, -EINVAL,
>  					   "Failed to get stream ID for channel %d\n", tdc->id);
>  
> -			chan->dev->chan_dma_dev = true;
> +			chan->chan_dev->chan_dma_dev = true;
>  		}

[Severity: High]
This is a pre-existing issue, but does modifying the device bus after
registration circumvent the device core lifecycle and cause mismatched bus
notifications?

During device probe in tegra_dma_probe(), the channels are registered via
dmaenginem_async_device_register(), and then chdev->bus = pdev->dev.bus
is set manually in this use_iommu_map block:

drivers/dma/tegra186-gpc-dma.c:tegra_dma_probe() {
    ...
		if (use_iommu_map) {
			chdev->bus = pdev->dev.bus;
    ...
}

Does bypassing bus_add_device() during initialization but later triggering
bus_remove_device() on teardown lead to missing add notifications? During
driver unbind or probe rollback, device_del() sees the non-NULL bus and blindly
triggers bus_remove_device(). Can this crash or corrupt listeners such as the
IOMMU subsystem that expect properly ordered device lifecycle notifications?

While reviewing this patch, I also noticed a pre-existing issue in
drivers/dma/dmaengine.c that isn't directly touched by this commit:

[Severity: High]
This is a pre-existing issue, but can the missing synchronize_rcu() in the
DMA device unregistration path lead to a Use-After-Free in
dma_issue_pending_all()?

When a DMA controller is unbound, dma_async_device_unregister() puts the
device reference. Then dma_device_release() unlinks the device using
list_del_rcu(&device->global_node) but fails to wait for an RCU grace period
before allowing devres to automatically free the backing memory:

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

Concurrently, the RCU reader iterates over the list in dma_issue_pending_all()
and accesses the freed device memory:

drivers/dma/dmaengine.c:dma_issue_pending_all() {
    ...
	list_for_each_entry_rcu(device, &dma_device_list, global_node) {
		if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
    ...
}

Could concurrent RCU readers dereferencing the freed device pointer here
experience memory corruption or a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-dma-chan_dev-rename-v3-0-685601b0b788@nxp.com?part=4