Re: [PATCH v2 4/9] dmaengine: qcom: gpi: Use bus width capability helpers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing unregistration of the DMA device on probe error and driver unbind leads to Use-After-Free.
--

--- Patch [4]: [PATCH v2 4/9] dmaengine: qcom: gpi: Use bus width capability helpers ---
commit 7a53f944daa4cc2b2af28a305b5fb4e022bb0d4d
Author: Nuno Sá <nuno.sa@analog.com>

dmaengine: qcom: gpi: Use bus width capability helpers

Advertise the single supported source and destination bus width through
the new dma_set_src_bus_width() and dma_set_dst_bus_width() helpers
instead of assigning the legacy u32 fields directly.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -2265,8 +2265,15 @@ static int gpi_probe(struct platform_device *pdev)
>  	/* configure dmaengine apis */
>  	gpi_dev->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
>  	gpi_dev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> -	gpi_dev->dma_device.src_addr_widths = DMA_SLAVE_BUSWIDTH_8_BYTES;
> -	gpi_dev->dma_device.dst_addr_widths = DMA_SLAVE_BUSWIDTH_8_BYTES;
> +	ret = dma_set_src_bus_width(&gpi_dev->dma_device,
> +				    DMA_SLAVE_BUSWIDTH_8_BYTES);
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_set_dst_bus_width(&gpi_dev->dma_device,
> +				    DMA_SLAVE_BUSWIDTH_8_BYTES);
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but since we are modifying gpi_probe(), is it
possible that the DMA device is left registered if probe fails later?

Looking at the end of gpi_probe():

	ret = dma_async_device_register(&gpi_dev->dma_device);
	if (ret) {
		dev_err(gpi_dev->dev, "async_device_register failed ret:%d", ret);
		return ret;
	}

	ret = of_dma_controller_register(gpi_dev->dev->of_node,
					 gpi_of_dma_xlate, gpi_dev);
	if (ret) {
		dev_err(gpi_dev->dev, "of_dma_controller_reg failed ret:%d", ret);
		return ret;
	}

If of_dma_controller_register() fails, the function returns without calling
dma_async_device_unregister(). Since gpi_dev is allocated with devm_kzalloc,
it will be freed automatically by devres, leaving dangling pointers in the core
DMA subsystem's device list.

Additionally, the gpi_driver platform driver definition lacks a .remove
callback entirely:

static struct platform_driver gpi_driver = {
	.probe = gpi_probe,
	.driver = {
		.name = KBUILD_MODNAME,
		.of_match_table = gpi_of_match,
	},
};

Does this mean a manual driver unbind via sysfs or device removal will also
cause a use-after-free for the same reason?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-dmaengine-support-wider-dma-masks-v2-0-1f7b798d035f@analog.com?part=4