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

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

[Myeonghun Pak]

mtk_hsdma_hw_init() enables runtime PM and gets a runtime PM reference
before enabling the HSDMA clock. It currently ignores failures from
pm_runtime_get_sync(); if runtime resume fails, the usage count remains
held. If clk_prepare_enable() then fails, runtime PM is left enabled with
the usage count held.

Use pm_runtime_resume_and_get() so resume failures do not leak the usage
count, and unwind runtime PM when clk_prepare_enable() fails.

The probe path also ignores the return value from mtk_hsdma_hw_init(), so a
failed hardware init can continue as a successful probe. Propagate
mtk_hsdma_hw_init() failures from probe, while keeping a separate unwind
label so mtk_hsdma_hw_deinit() is only called after hardware init succeeds.

Fixes: 548c4597e984 ("dmaengine: mediatek: Add MediaTek High-Speed DMA controller for MT7622 and MT7623 SoC")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

---
 drivers/dma/mediatek/mtk-hsdma.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
index a43412ff5e..987e5274fc 100644
--- a/drivers/dma/mediatek/mtk-hsdma.c
+++ b/drivers/dma/mediatek/mtk-hsdma.c
@@ -849,16 +849,25 @@ static int mtk_hsdma_hw_init(struct mtk_hsdma_device *hsdma)
 	int err;
 
 	pm_runtime_enable(hsdma2dev(hsdma));
-	pm_runtime_get_sync(hsdma2dev(hsdma));
+	err = pm_runtime_resume_and_get(hsdma2dev(hsdma));
+	if (err < 0)
+		goto err_disable_pm;
 
 	err = clk_prepare_enable(hsdma->clk);
 	if (err)
-		return err;
+		goto err_put_pm;
 
 	mtk_dma_write(hsdma, MTK_HSDMA_INT_ENABLE, 0);
 	mtk_dma_write(hsdma, MTK_HSDMA_GLO, MTK_HSDMA_GLO_DEFAULT);
 
 	return 0;
+
+err_put_pm:
+	pm_runtime_put_sync(hsdma2dev(hsdma));
+err_disable_pm:
+	pm_runtime_disable(hsdma2dev(hsdma));
+
+	return err;
 }
 
 static int mtk_hsdma_hw_deinit(struct mtk_hsdma_device *hsdma)
@@ -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;
 
 	err = devm_request_irq(&pdev->dev, hsdma->irq,
 			       mtk_hsdma_irq, 0,
@@ -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);
-- 
2.47.1

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Premature DMA controller registration exposes uninitialized hardware to consumers.
- [High] Hardware and clocks are disabled before the DMA engine is unregistered during teardown, causing unclocked accesses.
- [High] Unsafe `tasklet_kill()` ordering in `remove` allows tasklets to be rescheduled, leading to Use-After-Free.
- [High] Manual channel list deletion subverts the DMA subsystem's cleanup, leaking sysfs entries.
--

commit a02c13cb6b6d7dd4b66478026a93c3fbb55427ef
Author: Myeonghun Pak <mhun512@gmail.com>

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

mtk_hsdma_hw_init() enables runtime PM and gets a runtime PM reference
before enabling the HSDMA clock. It currently ignores failures from
pm_runtime_get_sync(); if runtime resume fails, the usage count remains
held. If clk_prepare_enable() then fails, runtime PM is left enabled with
the usage count held.

Use pm_runtime_resume_and_get() so resume failures do not leak the usage
count, and unwind runtime PM when clk_prepare_enable() fails.

> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5eddc..31f05d5d04151 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 I noticed that premature DMA
controller registration exposes uninitialized hardware to consumers.

In drivers/dma/mediatek/mtk-hsdma.c:mtk_hsdma_probe(),
dma_async_device_register() and of_dma_controller_register() are invoked
before mtk_hsdma_hw_init(). A consumer device probing concurrently can
request a channel via the device tree.

The consumer will then trigger mtk_hsdma_alloc_chan_resources(), which
accesses MTK_HSDMA_GLO to configure the hardware. Because the clock is not
yet enabled by mtk_hsdma_hw_init(), this unclocked access crashes the
system.

Could we initialize the hardware before registering the DMA controller to
avoid this race condition?

>  
>  	err = devm_request_irq(&pdev->dev, hsdma->irq,
>  			       mtk_hsdma_irq, 0,
> @@ -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 is a pre-existing issue, but hardware and clocks are disabled before
the DMA engine is unregistered during teardown, which can cause unclocked
accesses.

In the probe error path (err_deinit) and in mtk_hsdma_remove(),
mtk_hsdma_hw_deinit() is called before dma_async_device_unregister().
mtk_hsdma_hw_deinit() disables the device clocks.

Since the DMA controller is still registered and active, consumers might
still attempt to use the channels or release them. Releasing a channel
calls mtk_hsdma_free_chan_resources(), which accesses MTK_HSDMA_GLO. With
the clock disabled, this results in an external abort.

Should dma_async_device_unregister() be called before disabling the
hardware?

While reviewing this driver, I also noticed a couple of pre-existing issues
in drivers/dma/mediatek/mtk-hsdma.c:mtk_hsdma_remove():

mtk_hsdma_remove() {
    ...
    list_del(&vc->vc.chan.device_node);
    tasklet_kill(&vc->vc.task);
    ...
    mtk_dma_write(hsdma, MTK_HSDMA_INT_ENABLE, 0);
    synchronize_irq(hsdma->irq);
    ...
    dma_async_device_unregister(&hsdma->ddev);
}

[Severity: High]
This is a pre-existing issue, but the unsafe tasklet_kill() ordering in
mtk_hsdma_remove() allows tasklets to be rescheduled, leading to a potential
use-after-free.

tasklet_kill() is called to stop the tasklets for all virtual channels
before disabling the DMA interrupt (MTK_HSDMA_INT_ENABLE) and synchronizing
IRQs.

If an interrupt fires after the tasklet is killed, the IRQ handler
(mtk_hsdma_irq) will execute, calling vchan_cookie_complete(), which
explicitly schedules the tasklet via tasklet_schedule().

After mtk_hsdma_remove() returns, the device memory is freed by devres,
but the tasklet remains queued and will execute on the freed memory.

Can we disable and synchronize the interrupts before killing the tasklets?


[Severity: High]
This is another pre-existing issue in mtk_hsdma_remove(), but manual channel
list deletion seems to subvert the DMA subsystem's cleanup, leaking sysfs
entries.

The driver manually loops over the channels and calls
list_del(&vc->vc.chan.device_node).

When it subsequently calls dma_async_device_unregister(), the DMA core
subsystem loops over &device->channels to unregister each channel's sysfs
devices (via __dma_async_device_channel_unregister()).

Because the list was prematurely emptied by the driver, the core sees an
empty list, skips the cleanup, and leaks the sysfs structures.

Is it safe to remove the manual list_del() calls so the DMA core can
properly clean up the channels?

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

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

[Frank Li]

On Wed, Jun 24, 2026 at 05:16:38PM +0900, Myeonghun Pak wrote:
> mtk_hsdma_hw_init() enables runtime PM and gets a runtime PM reference
> before enabling the HSDMA clock. It currently ignores failures from
> pm_runtime_get_sync(); if runtime resume fails, the usage count remains
> held. If clk_prepare_enable() then fails, runtime PM is left enabled with
> the usage count held.
>
> Use pm_runtime_resume_and_get() so resume failures do not leak the usage
> count, and unwind runtime PM when clk_prepare_enable() fails.
>
> The probe path also ignores the return value from mtk_hsdma_hw_init(), so a
> failed hardware init can continue as a successful probe. Propagate
> mtk_hsdma_hw_init() failures from probe, while keeping a separate unwind
> label so mtk_hsdma_hw_deinit() is only called after hardware init succeeds.
>
> Fixes: 548c4597e984 ("dmaengine: mediatek: Add MediaTek High-Speed DMA controller for MT7622 and MT7623 SoC")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
>
> ---
>  drivers/dma/mediatek/mtk-hsdma.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5e..987e5274fc 100644
> --- a/drivers/dma/mediatek/mtk-hsdma.c
> +++ b/drivers/dma/mediatek/mtk-hsdma.c
> @@ -849,16 +849,25 @@ static int mtk_hsdma_hw_init(struct mtk_hsdma_device *hsdma)
>  	int err;
>
>  	pm_runtime_enable(hsdma2dev(hsdma));

use devm_pm_runtime_enable()

> -	pm_runtime_get_sync(hsdma2dev(hsdma));
> +	err = pm_runtime_resume_and_get(hsdma2dev(hsdma));

It enable runtime pm and resume_get here. and suspend at driver remove,
so whole life cycle, pm is enable, why need enable runtime pm management?

Frank

> +	if (err < 0)
> +		goto err_disable_pm;
>
>  	err = clk_prepare_enable(hsdma->clk);
>  	if (err)
> -		return err;
> +		goto err_put_pm;
>
>  	mtk_dma_write(hsdma, MTK_HSDMA_INT_ENABLE, 0);
>  	mtk_dma_write(hsdma, MTK_HSDMA_GLO, MTK_HSDMA_GLO_DEFAULT);
>
>  	return 0;
> +
> +err_put_pm:
> +	pm_runtime_put_sync(hsdma2dev(hsdma));
> +err_disable_pm:
> +	pm_runtime_disable(hsdma2dev(hsdma));
> +
> +	return err;
>  }
>
>  static int mtk_hsdma_hw_deinit(struct mtk_hsdma_device *hsdma)
> @@ -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;
>
>  	err = devm_request_irq(&pdev->dev, hsdma->irq,
>  			       mtk_hsdma_irq, 0,
> @@ -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);
> --
> 2.47.1

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

[Myeonghun Pak]

mtk_hsdma_hw_init() enables runtime PM and gets a runtime PM reference
before enabling the HSDMA clock. It currently ignores failures from
pm_runtime_get_sync(); if runtime resume fails, the usage count remains
held. If clk_prepare_enable() then fails, the usage count remains held.

Use devm_pm_runtime_enable() to manage runtime PM enablement, and use
pm_runtime_resume_and_get() so resume failures do not leak the usage count.
If clk_prepare_enable() fails after a successful runtime resume, drop the
runtime PM reference before returning.

The probe path also ignores the return value from mtk_hsdma_hw_init(), so a
failed hardware init can continue as a successful probe. Propagate
mtk_hsdma_hw_init() failures from probe, while keeping a separate unwind
label so mtk_hsdma_hw_deinit() is only called after hardware init succeeds.

Fixes: 548c4597e984 ("dmaengine: mediatek: Add MediaTek High-Speed DMA controller for MT7622 and MT7623 SoC")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

---
Changes in v2:
- Use devm_pm_runtime_enable() for runtime PM enablement.
- Drop manual pm_runtime_disable() calls from init/deinit paths.

 drivers/dma/mediatek/mtk-hsdma.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
index a43412ff5e..27cb370106 100644
--- a/drivers/dma/mediatek/mtk-hsdma.c
+++ b/drivers/dma/mediatek/mtk-hsdma.c
@@ -848,17 +848,27 @@ static int mtk_hsdma_hw_init(struct mtk_hsdma_device *hsdma)
 {
 	int err;
 
-	pm_runtime_enable(hsdma2dev(hsdma));
-	pm_runtime_get_sync(hsdma2dev(hsdma));
+	err = devm_pm_runtime_enable(hsdma2dev(hsdma));
+	if (err)
+		return err;
+
+	err = pm_runtime_resume_and_get(hsdma2dev(hsdma));
+	if (err < 0)
+		return err;
 
 	err = clk_prepare_enable(hsdma->clk);
 	if (err)
-		return err;
+		goto err_put_pm;
 
 	mtk_dma_write(hsdma, MTK_HSDMA_INT_ENABLE, 0);
 	mtk_dma_write(hsdma, MTK_HSDMA_GLO, MTK_HSDMA_GLO_DEFAULT);
 
 	return 0;
+
+err_put_pm:
+	pm_runtime_put_sync(hsdma2dev(hsdma));
+
+	return err;
 }
 
 static int mtk_hsdma_hw_deinit(struct mtk_hsdma_device *hsdma)
@@ -868,7 +878,6 @@ static int mtk_hsdma_hw_deinit(struct mtk_hsdma_device *hsdma)
 	clk_disable_unprepare(hsdma->clk);
 
 	pm_runtime_put_sync(hsdma2dev(hsdma));
-	pm_runtime_disable(hsdma2dev(hsdma));
 
 	return 0;
 }
@@ -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;
 
 	err = devm_request_irq(&pdev->dev, hsdma->irq,
 			       mtk_hsdma_irq, 0,
@@ -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);
-- 
2.47.1

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

[sashiko-bot]

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