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

[email protected]
Newsgroups org.kernel.vger.dmaengine
Message-ID <[email protected]>
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á <[email protected]>

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.