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