Re: [PATCH v2 RESEND] dmaengine: mediatek: hsdma: fix runtime PM leak on init failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Premature DMA device registration exposes uninitialized hardware, leading to a kernel panic.
- [High] Devm-managed IRQ causes a race condition during remove, leading to bus faults.
- [High] Missing tasklet cleanup on probe failure leads to potential use-after-free.
--

--- Patch [1]: [PATCH v2 RESEND] dmaengine: mediatek: hsdma: fix runtime PM leak on init failure ---
commit 51dcb76c3dfae4c384b90e41e90bc15393906d0e
Author: Myeonghun Pak <mhun512@gmail.com>

dmaengine: mediatek: hsdma: fix runtime PM leak on init failure

This commit uses devm_pm_runtime_enable and pm_runtime_resume_and_get to fix
runtime PM usage count leaks on init failures. It also propagates hardware
init errors out of the probe function.

> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5eddc..27cb370106c77 100644
> --- a/drivers/dma/mediatek/mtk-hsdma.c
> +++ b/drivers/dma/mediatek/mtk-hsdma.c
[ ... ]
> @@ -983,7 +992,9 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>  		goto err_unregister;
>  	}
>  
> -	mtk_hsdma_hw_init(hsdma);
> +	err = mtk_hsdma_hw_init(hsdma);
> +	if (err)
> +		goto err_free;

[Severity: High]
This isn't a bug introduced by this patch, but does the registration order
expose uninitialized hardware to clients?

In mtk_hsdma_probe(), dma_async_device_register() and 
of_dma_controller_register() are called before mtk_hsdma_hw_init().
This exposes the DMA channels to client drivers before the device clocks
are enabled. If a client accesses the controller before it is initialized,
could this trigger a bus fault and a kernel panic?

Similarly, in mtk_hsdma_remove(), mtk_hsdma_hw_deinit() powers down the
hardware before dma_async_device_unregister() is called, allowing active
clients to access powered-off hardware.

>  
>  	err = devm_request_irq(&pdev->dev, hsdma->irq,
>  			       mtk_hsdma_irq, 0,

[Severity: High]
This is a pre-existing issue, but does using devm_request_irq() here create
a race condition during remove?

The IRQ is managed by devres, meaning it remains active throughout the entire
mtk_hsdma_remove() function until devres cleanup runs.

However, mtk_hsdma_remove() disables the hardware via mtk_hsdma_hw_deinit():

drivers/dma/mediatek/mtk-hsdma.c:mtk_hsdma_remove() {
    ...
    mtk_hsdma_hw_deinit(hsdma);
    ...
}

If a spurious interrupt fires after the hardware has been disabled,
mtk_hsdma_irq() will invoke mtk_dma_clr() and access unclocked registers.
Could this trigger an external abort and a kernel panic?

> @@ -991,7 +1002,7 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>  	if (err) {
>  		dev_err(&pdev->dev,
>  			"request_irq failed with err %d\n", err);
> -		goto err_free;
> +		goto err_deinit;
>  	}
>  
>  	platform_set_drvdata(pdev, hsdma);
> @@ -1000,8 +1011,9 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -err_free:
> +err_deinit:
>  	mtk_hsdma_hw_deinit(hsdma);
> +err_free:
>  	of_dma_controller_free(pdev->dev.of_node);
>  err_unregister:
>  	dma_async_device_unregister(dd);

[Severity: High]
This isn't a bug introduced by this patch, but does the error unwinding path
miss tasklet cleanup?

Earlier in mtk_hsdma_probe(), vchan_init() initializes a tasklet for each
virtual channel. Because the DMA controller is registered before 
mtk_hsdma_hw_init(), a client can request a channel and start a transfer,
scheduling the tasklet. 

If probe subsequently fails (e.g. devm_request_irq() fails) and takes this
error path, it unregisters the device but fails to call tasklet_kill().
Devres will then free the hsdma and vc memory while the tasklets may still
be active or pending. Could this lead to a use-after-free when the tasklet
executes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804153924.49627-1-mhun512@gmail.com?part=1