All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure
@ 2026-08-08 10:56 Guangshuo Li
  2026-08-08 11:11 ` sashiko-bot
  2026-08-09 22:15 ` Linus Walleij
  0 siblings, 2 replies; 3+ messages in thread
From: Guangshuo Li @ 2026-08-08 10:56 UTC (permalink / raw)
  To: Linus Walleij, Vinod Koul, Frank Li, Srinidh Kasagar,
	Andrew Morton, Dan Williams, linux-arm-kernel, dmaengine,
	linux-kernel
  Cc: Guangshuo Li, stable

d40_probe() calls pm_runtime_use_autosuspend(), but its error path does
not call the matching pm_runtime_dont_use_autosuspend() before
disabling runtime PM.

If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during cleanup, this reference is not dropped and usage_count remains
unbalanced.

Add the missing pm_runtime_dont_use_autosuspend() call to the probe
failure path before disabling runtime PM.

This issue was found by manual code inspection.

Fixes: 8d318a50b3d7 ("DMAENGINE: Support for ST-Ericssons DMA40 block v3")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/dma/ste_dma40.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..e2ad50e0fec4 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3654,6 +3654,7 @@ static int __init d40_probe(struct platform_device *pdev)
 		regulator_disable(base->lcpa_regulator);
 		regulator_put(base->lcpa_regulator);
 	}
+	pm_runtime_dont_use_autosuspend(base->dev);
 	pm_runtime_disable(base->dev);
 
  report_failure:
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure
  2026-08-08 10:56 [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure Guangshuo Li
@ 2026-08-08 11:11 ` sashiko-bot
  2026-08-09 22:15 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-08 11:11 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: vkoul, Frank.Li, dmaengine

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

Pre-existing issues:
- [High] Unbalanced PM runtime state due to unconditional disable on probe failure.
- [High] IRQ resource leak and subsequent Use-After-Free on probe failure.
- [High] Incorrect order parameter passed to `__get_free_pages()` causes exponentially oversized allocations.
- [High] Calling `free_pages()` on a `devm_ioremap()` pointer causes a kernel panic.
- [High] Missing tasklet cleanup on probe failure leads to Use-After-Free.
- [High] Teardown asymmetry leaves active DMA engines exposed to users after backing resources are freed.
--

--- Patch [1]: [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure ---
commit 314875d0410790018090d6a5f3fa67bbac67585d
Author: Guangshuo Li <lgs201920130244@gmail.com>

dmaengine: ste_dma40: fix autosuspend cleanup on probe failure

d40_probe() calls pm_runtime_use_autosuspend(), but its error path does
not call the matching pm_runtime_dont_use_autosuspend() before
disabling runtime PM.

If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during cleanup, this reference is not dropped and usage_count remains
unbalanced.

Add the missing pm_runtime_dont_use_autosuspend() call to the probe
failure path before disabling runtime PM.

Fixes: 8d318a50b3d7 ("DMAENGINE: Support for ST-Ericssons DMA40 block v3")
Cc: stable@vger.kernel.org

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 0d9ffa3e26639..e2ad50e0fec48 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3654,6 +3654,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  		regulator_disable(base->lcpa_regulator);
>  		regulator_put(base->lcpa_regulator);
>  	}
> +	pm_runtime_dont_use_autosuspend(base->dev);
>  	pm_runtime_disable(base->dev);
>  

[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying the
PM cleanup here, does this unconditional call to pm_runtime_disable() cause
an unbalanced runtime state if probe fails early?

If d40_probe() jumps to destroy_cache before pm_runtime_enable() is called
(e.g., during memory allocation or IRQ request), this will increment the
device's disable_depth from 1 to 2. If the device is probed again later,
the next pm_runtime_enable() will only decrement it to 1, leaving the
device permanently disabled.

[Severity: High]
This is a pre-existing issue, but looking at the surrounding error cleanup
path, is there an IRQ resource leak here?

The driver requests an unmanaged interrupt earlier:

    request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);

If a subsequent initialization step fails and we jump to destroy_cache,
free_irq() is omitted. Could this leave a dangling pointer registered as
the IRQ handler data, potentially causing a use-after-free if the hardware
fires an interrupt after devres frees the base structure?

[Severity: High]
This is a pre-existing issue, but does the cleanup in destroy_cache risk
calling free_pages() on an I/O mapped pointer?

When base->plat_data->use_esram_lcla is true, the base pointer is assigned
an I/O mapped address:

    base->lcla_pool.base = devm_ioremap(dev, res->start, resource_size(res));

If probe fails, destroy_cache evaluates if base_unaligned is NULL and
base->lcla_pool.base is true, leading to:

    free_pages((unsigned long)base->lcla_pool.base, base->lcla_pool.pages);

Passing an I/O memory address to free_pages() seems invalid.

[Severity: High]
This is a pre-existing issue, but is the pages parameter passed to
free_pages() and __get_free_pages() calculated correctly?

In d40_lcla_allocate(), the value is calculated as the raw number of pages:

    base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE;

This raw count is then passed as the order parameter to __get_free_pages():

    __get_free_pages(GFP_KERNEL, base->lcla_pool.pages);

Would passing the raw count instead of an order (e.g., 8 instead of 3 for
32 channels) cause exponentially oversized allocations?

[Severity: High]
This is a pre-existing issue, but does the error path omit tasklet cleanup?

During channel initialization, tasklets are set up:

    tasklet_setup(&d40c->tasklet, dma_tasklet);

If probe fails afterwards, tasklet_kill() is not called in the cleanup
path. If an interrupt fires and schedules a tasklet before probe fails,
could the tasklet still execute after devres frees the channel structures?

[Severity: High]
This is a pre-existing issue, but is there a teardown asymmetry with the
DMA engine registration?

The DMA engine is exposed via a managed API:

    dmaenginem_async_device_register(&base->dma_slave);

If probe fails later (e.g., in of_dma_controller_register), the code
jumps to destroy_cache and manually destroys the lcla_pool, unmaps DMA
addresses, and disables regulators. However, the successfully registered
DMA engines remain active until devres unregisters them later.

Could concurrent clients acquire a channel and initiate transfers during
this window, accessing freed memory or operating with disabled regulators?

>   report_failure:

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure
  2026-08-08 10:56 [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure Guangshuo Li
  2026-08-08 11:11 ` sashiko-bot
@ 2026-08-09 22:15 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2026-08-09 22:15 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Vinod Koul, Frank Li, Srinidh Kasagar, Andrew Morton,
	Dan Williams, linux-arm-kernel, dmaengine, linux-kernel, stable

On Sat, Aug 8, 2026 at 12:57 PM Guangshuo Li <lgs201920130244@gmail.com> wrote:

> d40_probe() calls pm_runtime_use_autosuspend(), but its error path does
> not call the matching pm_runtime_dont_use_autosuspend() before
> disabling runtime PM.
>
> If the autosuspend delay is set to a negative value while autosuspend
> is enabled, the runtime PM core increments usage_count to prevent
> runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
> during cleanup, this reference is not dropped and usage_count remains
> unbalanced.
>
> Add the missing pm_runtime_dont_use_autosuspend() call to the probe
> failure path before disabling runtime PM.
>
> This issue was found by manual code inspection.
>
> Fixes: 8d318a50b3d7 ("DMAENGINE: Support for ST-Ericssons DMA40 block v3")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-09 22:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 10:56 [PATCH] dmaengine: ste_dma40: fix autosuspend cleanup on probe failure Guangshuo Li
2026-08-08 11:11 ` sashiko-bot
2026-08-09 22:15 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.