Re: [PATCH 1/2] dmaengine: use dma_chan_name() helper to get per-channel device name

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

Pre-existing issues:
- [High] Probe error path prematurely frees DMA channel structures and disables clocks before devm-managed IRQs are freed, leading to a Use-After-Free and potential bus hang.
--

--- Patch [1]: [PATCH 1/2] dmaengine: use dma_chan_name() helper to get per-channel device name ---
commit 77ba32b52a0d71c14db9e5276f81ba0fa2896131
Author: Frank Li <[email protected]>

dmaengine: use dma_chan_name() helper to get per-channel device name

This patch uses the existing dma_chan_name() helper to obtain the
per-channel device name instead of open-coding it. This simplifies the
code and improves consistency.

[ ... ]

> diff --git a/drivers/dma/stm32/stm32-dma.c b/drivers/dma/stm32/stm32-dma.c
> index d3ad78562a140..4e3ecb6763036 100644
> --- a/drivers/dma/stm32/stm32-dma.c
> +++ b/drivers/dma/stm32/stm32-dma.c
> @@ -1668,7 +1668,7 @@ static int stm32_dma_probe(struct platform_device *pdev)
>  
>  		ret = devm_request_irq(&pdev->dev, chan->irq,
>  				       stm32_dma_chan_irq, 0,
> -				       dev_name(chan2dev(chan)), chan);
> +				       dma_chan_name(&chan->vchan.chan), chan);

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but doesn't the error path in stm32_dma_probe() in
drivers/dma/stm32/stm32-dma.c prematurely free DMA channel structures and
disable clocks before the devm-managed IRQs are freed?

In stm32_dma_probe(), IRQs are requested using devm_request_irq() and
passed dma_chan_name(&chan->vchan.chan) as the action name.

If a subsequent initialization step fails, the error path manually calls
dma_async_device_unregister(dd):

err_unregister:
	dma_async_device_unregister(dd);
clk_free:
	clk_disable_unprepare(dmadev->clk);

	return ret;

This frees the channel device structures, including the embedded name
string, and clk_disable_unprepare(dmadev->clk) stops the hardware clock.

Because the IRQs are devres-managed, they remain active until the probe
function returns and the devres framework unwinds. During this window, if
/proc/interrupts is read, the kernel accesses the freed action->name
string, potentially triggering a Use-After-Free.

Furthermore, if a spurious interrupt fires during this window, the handler
stm32_dma_chan_irq() will read hardware registers with the device clock
disabled, which could trigger a Data Abort kernel panic.

>  		if (ret) {
>  			dev_err(&pdev->dev,
>  				"request_irq failed with err %d channel %d\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1