Re: [PATCH 05/26] dmaengine: mediatek-mtk-hsdma: 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 DMA controller is registered and exposed to clients before hardware initialization and IRQ setup, causing race conditions and Use-After-Free vulnerabilities on probe failure.
- [High] The return value of mtk_hsdma_hw_init() is ignored, masking clock initialization failures and leading to hardware lockups and leaked PM runtime references.
- [Medium] In mtk_hsdma_remove(), the DMA engine is unregistered after internal resources are disabled, creating a window where clients can access unclocked hardware.
--

--- Patch [5]: [PATCH 05/26] dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe() ---
commit 572ded362561924159b30085ea17ce96e1c47b86
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe()

This commit removes redundant error messages after devm_request_irq() in
the MediaTek HSDMA driver. The core subsystem now handles logging detailed
error messages automatically on failure.

> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5eddc..6ee8911c12e0f 100644
> --- a/drivers/dma/mediatek/mtk-hsdma.c
> +++ b/drivers/dma/mediatek/mtk-hsdma.c
> @@ -988,11 +988,8 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>  	err = devm_request_irq(&pdev->dev, hsdma->irq,
>  			       mtk_hsdma_irq, 0,
>  			       dev_name(&pdev->dev), hsdma);
> -	if (err) {
> -		dev_err(&pdev->dev,
> -			"request_irq failed with err %d\n", err);
> +	if (err)
>  		goto err_free;

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
mtk_hsdma_hw_init() called just before devm_request_irq()?

If clk_prepare_enable() fails inside mtk_hsdma_hw_init(), it returns an
error but mtk_hsdma_probe() proceeds anyway. If devm_request_irq() then
fails here, the error path calls mtk_hsdma_hw_deinit(), which writes to a
DMA register with the clock disabled. Might this crash the system with a
bus fault?

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave
dangling pointers if it fails?

The DMA controller is registered globally via dma_async_device_register()
earlier in mtk_hsdma_probe(), making channels visible to clients. If a
client acquires a channel and then devm_request_irq() fails here, the probe
aborts and devres frees the mtk_hsdma_device structure.

Since dma_async_device_unregister() doesn't forcefully revoke channels held
by clients, could the client be left with a dangling pointer to freed
memory?

Looking at mtk_hsdma_remove() in the same file:

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

        dma_async_device_unregister(&hsdma->ddev);
        ...
}

[Severity: Medium]
This is a pre-existing issue, but could there be a race condition during
device removal?

The hardware is disabled via mtk_hsdma_hw_deinit() before the DMA
controller is unregistered. If a client attempts to use a DMA channel
while the driver is being unbound, could it issue a request after clocks
are disabled but before the device is unregistered?

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