Re: [PATCH 20/26] dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `.remove` callback causes resource leaks and use-after-free on driver unbind.
- [High] Devres leak of DMA pools on channel allocation error path.
- [High] Disabling the clock before freeing the IRQ on the probe error path can cause a synchronous external abort.
--

--- Patch [20]: [PATCH 20/26] dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe() ---
commit 6440956ea9b4ed6aa697c4eb90a3a35f158a8e26
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe()

This patch removes redundant dev_err() calls after devm_request_irq() 
since the core now logs detailed error messages automatically.

> diff --git a/drivers/dma/stm32/stm32-mdma.c b/drivers/dma/stm32/stm32-mdma.c
> --- a/drivers/dma/stm32/stm32-mdma.c
> +++ b/drivers/dma/stm32/stm32-mdma.c
> @@ -1713,10 +1713,8 @@ static int stm32_mdma_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, dmadev->irq, stm32_mdma_irq_handler,
>  			       0, dev_name(&pdev->dev), dmadev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to request IRQ\n");
> +	if (ret)
>  		goto err_clk;

[Severity: High]
This is a pre-existing issue, but does disabling the clock on the error path
before devres frees the IRQ risk a synchronous external abort?

If a spurious interrupt arrives after the goto err_clk path calls
clk_disable_unprepare() but before devres cleans up the IRQ handler,
stm32_mdma_irq_handler() will execute.

Could attempting to read STM32_MDMA_GISR0 while the APB clock is disabled
cause a bus fault and crash the kernel?

>  
>  	ret = dmaenginem_async_device_register(dd);
>  	if (ret)

I also noticed a couple of other pre-existing issues in this file:

drivers/dma/stm32/stm32-mdma.c:stm32_mdma_driver

static struct platform_driver stm32_mdma_driver = {
	.probe = stm32_mdma_probe,
	.driver = {
		.name = "stm32-mdma",
...

[Severity: High]
This is a pre-existing issue, but does this platform driver lack a .remove
callback?

Since suppress_bind_attrs is not set, a privileged user can unbind the
driver via sysfs. When devres frees the device memory for dmadev,
of_dma_controller_free() is never called, leaving a dangling pointer
in the global of_dma_list.

Will subsequent DMA requests by other devices dereference this freed
pointer? Also, could scheduled vchan tasklets cause a use-after-free if
they execute during unbind?


drivers/dma/stm32/stm32-mdma.c:stm32_mdma_alloc_chan_resources()

	...
	ret = pm_runtime_resume_and_get(dmadev->ddev.dev);
	if (ret < 0)
		return ret;

	ret = stm32_mdma_disable_chan(chan);
	if (ret < 0)
		goto err_pm;
	...
err_pm:
	pm_runtime_put(dmadev->ddev.dev);
	return ret;

[Severity: High]
This is a pre-existing issue, but does this error path leak the descriptor
pool?

Earlier in stm32_mdma_alloc_chan_resources(), dmam_pool_create() allocates
a new pool. If a subsequent step like pm_runtime_resume_and_get() or
stm32_mdma_disable_chan() fails, the function returns without destroying
the pool.

Since the DMA engine core does not call the free callback if allocation
fails, could repeated channel requests by userspace continuously accumulate
leaked DMA pools and lead to an out of memory condition?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=20