Re: [PATCH v3 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 dma_async_device_unregister() on probe error path causes Use-After-Free.
--

commit 95d22ef1cb4b51af1c95483d343ee6c541c2475d
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
> index a5055a6273af6..3a35bf4c33978 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c

[ ... ]

> @@ -2265,8 +2266,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_bus_width_set(gpi_dev->dma_device.src_bus_widths,
> +				DMA_SLAVE_BUSWIDTH_8_BYTES);
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_bus_width_set(gpi_dev->dma_device.dst_bus_widths,
> +				DMA_SLAVE_BUSWIDTH_8_BYTES);
> +	if (ret)
> +		return ret;
>  	gpi_dev->dma_device.device_alloc_chan_resources = gpi_alloc_chan_resources;
>  	gpi_dev->dma_device.device_free_chan_resources = gpi_free_chan_resources;
>  	gpi_dev->dma_device.device_tx_status = dma_cookie_status;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free on the error path further down in this function.

If of_dma_controller_register() fails, the function currently returns
directly without calling dma_async_device_unregister():

gpi_probe() {
	...
	ret = dma_async_device_register(&gpi_dev->dma_device);
	...
	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;
	}
	...
}

Since the primary gpi_dev structure is allocated with devm_kzalloc(), it
will automatically be freed by devres if the probe fails here.

Does this leave a dangling pointer in the global DMA engine lists,
potentially causing a crash later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-dmaengine-support-wider-dma-masks-v3-0-507d97496f2d@analog.com?part=4