Re: [PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver requests a shared interrupt before device clocks are enabled and channel registers are initialized, risking a kernel panic if a shared interrupt fires early.
- [High] Unbalanced PM runtime usage counter and hardware clock leaks on `dw_probe` error paths.
- [High] `dw_remove` leaks clock reference counts if the device was runtime active, leaving clocks permanently enabled.
- [High] Ignored return values from `clk_prepare_enable()` in `dw_remove()` can lead to fatal bus faults.
- [High] The driver shuts down hardware and kills tasklets in `dw_remove()` before unregistering the DMA engine, exposing it to fatal race conditions.
- [High] Bypassing dmaengine channel unregistration in `dw_remove()` causes resource leaks.
--

--- Patch [1]: [PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path ---
commit c0653e2ca540837590c802ab80fc07b1689246e9
Author: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>

dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

This commit addresses a clock imbalance in the resume error path of the
dw-axi-dmac driver. It ensures the configuration clock is properly
disabled if the core clock fails to enable.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bcefaff03b5c6..9db8993e0bc6a 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1335,8 +1335,10 @@ static int axi_dma_resume(struct axi_dma_chip *chip)
>  		return ret;
>  
>  	ret = clk_prepare_enable(chip->core_clk);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		clk_disable_unprepare(chip->cfgr_clk);
>  		return ret;
> +	}
>  
>  	axi_dma_enable(chip);
>  	axi_dma_irq_enable(chip);

[Severity: High]
This is a pre-existing issue, but does this sequence in dw_probe() risk a
NULL pointer dereference if a shared interrupt fires early?

dw_probe() {
    ...
    ret = axi_req_irqs(pdev, chip);
    ...
}

Interrupts are requested here before chan_regs initialization and clock
enablement. If an interrupt fires early, dw_axi_dma_interrupt() calls
axi_dma_irq_disable() which accesses unclocked registers and subsequently
dereferences chan->chan_regs.

[Severity: High]
This is a pre-existing issue, but does dw_probe() leave the PM runtime
usage count unbalanced on error paths?

dw_probe() {
    ...
    goto err_pm_disable;
    ...
err_pm_disable:
    ...
}

If axi_dma_resume() fails, the execution jumps to err_pm_disable but fails
to call pm_runtime_put_noidle(). If dmaenginem_async_device_register()
fails, it jumps to err_pm_disable calling pm_runtime_disable() before the
scheduled pm_runtime_put() executes, leaving hardware clocks permanently
enabled.

[Severity: High]
This is a pre-existing issue, but does dw_remove() leak clock reference
counts if the device was runtime active?

dw_remove() {
    ...
    clk_prepare_enable(chip->cfgr_clk);
    clk_prepare_enable(chip->core_clk);
    ...
    axi_dma_suspend(chip);
    ...
}

If the device is active, PM runtime holds a clock reference. Calling
clk_prepare_enable() unconditionally increments it. axi_dma_suspend()
decrements it, but pm_runtime_disable() prevents PM runtime from dropping
the remaining reference, permanently leaving the clocks enabled.

[Severity: High]
This is a pre-existing issue, but can ignoring these return values lead to
fatal bus faults?

dw_remove() {
    ...
    clk_prepare_enable(chip->cfgr_clk);
    clk_prepare_enable(chip->core_clk);
    ...
}

If clock enablement fails here, the driver proceeds to access hardware
registers via axi_dma_irq_disable() and axi_chan_disable(), which will
trigger a fatal bus error.

[Severity: High]
This is a pre-existing issue, but does this tear down the hardware while
the DMA engine is still fully registered and accessible to clients?

dw_remove() {
    ...
    list_del(&chan->vc.chan.device_node);
    tasklet_kill(&chan->vc.task);
    ...
}

Since dw_probe() uses dmaenginem_async_device_register(), the actual DMA
engine unregistration is deferred until after dw_remove() returns. The
hardware is aggressively shut down here while clients can still submit
transactions, potentially causing use-after-free or bus faults.

[Severity: High]
This is a pre-existing issue, but does bypassing dmaengine channel
unregistration cause resource leaks?

dw_remove() {
    ...
    list_del(&chan->vc.chan.device_node);
    ...
}

Manually emptying the channel list breaks the subsequent deferred devres
unregistration loop. When dma_async_device_unregister() finally runs, the
list is empty, preventing __dma_async_device_channel_unregister() from
being called and leaking sysfs device nodes and IDAs for every channel.

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