Re: [PATCH 19/26] dmaengine: stm32-stm32-dma3: 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] The OF DMA controller is registered using the non-managed `of_dma_controller_register()`, but it is never freed in the driver's `remove` function, leading to a resource leak and a Use-After-Free vulnerability.
- [High] The DMA controller exposes its channels to clients before completing initialization, creating a fatal race condition if subsequent channel setups fail.
- [High] Disabling the clock manually before devres cleans up devm-managed IRQs leaves a window where active IRQs can access unclocked hardware.
--

--- Patch [19]: [PATCH 19/26] dmaengine: stm32-stm32-dma3: Remove redundant dev_err()/dev_err_probe() ---
commit 71644e471343978f46dd63e4905f5c779768528f
Author: Pan Chuang <panchuang@vivo.com>

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

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> --- a/drivers/dma/stm32/stm32-dma3.c
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -1893,11 +1893,8 @@ static int stm32_dma3_probe(struct platform_device *pdev)
>  
>  		ret = devm_request_irq(&pdev->dev, chan->irq, stm32_dma3_chan_irq, 0,
>  				       dev_name(chan2dev(chan)), chan);
> -		if (ret) {
> -			dev_err_probe(&pdev->dev, ret, "Failed to request channel %s IRQ\n",
> -				      dev_name(chan2dev(chan)));
> +		if (ret)
>  			goto err_clk_disable;

[Severity: High]
This is a pre-existing issue, but by jumping to err_clk_disable here, could
we trigger a crash if an interrupt fires?

If stm32_dma3_probe() fails on a later iteration of this loop, we jump to
the manual clock disable cleanup:

err_clk_disable:
        clk_disable_unprepare(ddata->clk);

Since we used devm_request_irq() for previous channels, those IRQs remain
active until devres runs. If an interrupt fires during this window, the
handler would access unclocked registers.

[Severity: High]
This is also a pre-existing issue, but does failing here leave dangling
channel registrations?

Earlier in stm32_dma3_probe(), we called dmaenginem_async_device_register()
and dma_async_device_channel_register() before this channel initialization
loop completes. 

If a failure occurs in this loop and we jump to the error path, any client
that has already requested the successfully registered channels could be
exposed to an asynchronous teardown, potentially accessing unclocked hardware
or freed memory.

> -		}
>  	}
>  
>  	ret = of_dma_controller_register(np, stm32_dma3_of_xlate, ddata);

[Severity: High]
This is a pre-existing issue, but does this leak the DMA controller?

The of_dma_controller_register() function is used here, but looking at
stm32_dma3_remove(), there is no corresponding call to
of_dma_controller_free().

stm32_dma3_remove() simply disables runtime PM:

static void stm32_dma3_remove(struct platform_device *pdev)
{
        pm_runtime_disable(&pdev->dev);
}

If the driver is unbound, the devm-managed ddata will be freed, but the
controller remains registered. A subsequent DMA request via devicetree
would then invoke stm32_dma3_of_xlate() with a dangling ddata pointer,
causing a use-after-free.

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