DMA Engine development
 help / color / mirror / Atom feed
* [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs
@ 2026-08-19 22:35 Linus Walleij
  2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw)
  To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot

This series fixes four pre-existing DMA40 bugs found while reviewing the
Ux500 LCLA SRAM power-domain conversion.

The fixes cover failed hardware-start cleanup, probe error unwinding for
runtime PM and IRQ registration, and validation of logical channel indexes
derived from dev_type before writing into lookup_log_chans.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Linus Walleij (4):
      dmaengine: ste_dma40: Fix failed start cleanup
      dmaengine: ste_dma40: Fix probe runtime PM disable
      dmaengine: ste_dma40: Fix probe IRQ leak
      dmaengine: ste_dma40: Fix logical channel bounds check

 drivers/dma/ste_dma40.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)
---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260820-dma40-fixes-b99af66002bf

Best regards,
-- 
Linus Walleij <linusw@kernel.org>


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

* [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup
  2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
@ 2026-08-19 22:35 ` Linus Walleij
  2026-08-19 22:46   ` sashiko-bot
  2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw)
  To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot

If d40_start() fails after a queued descriptor has been moved to the
active list, d40_queue_start() currently returns NULL without unwinding
the descriptor state or clearing the channel busy flag.

Fix this pre-existing error path by removing and freeing the descriptor,
clearing the busy flag and returning the error pointer to distinguish the
failure from the no-work case.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/dma/ste_dma40.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..d5837aa0a789 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1531,8 +1531,12 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
 		/* Start dma job */
 		err = d40_start(d40c);
 
-		if (err)
-			return NULL;
+		if (err) {
+			d40_desc_remove(d40d);
+			d40_desc_free(d40c, d40d);
+			d40c->busy = false;
+			return ERR_PTR(err);
+		}
 	}
 
 	return d40d;

-- 
2.55.0


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

* [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable
  2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
  2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
@ 2026-08-19 22:35 ` Linus Walleij
  2026-08-19 22:51   ` sashiko-bot
  2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
  2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw)
  To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot

d40_probe() can jump to destroy_cache before runtime PM has been enabled
for the DMA controller device. Calling pm_runtime_disable() on that path
increments disable_depth, which can leave runtime PM disabled after a later
successful deferred probe.

Track whether this probe attempt enabled runtime PM before disabling it on
the error path.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/dma/ste_dma40.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index d5837aa0a789..0c8ff299c2c2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3502,6 +3502,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct resource res_lcpa;
 	int num_reserved_chans;
+	bool runtime_pm_enabled = false;
 	u32 val;
 	int ret;
 
@@ -3623,6 +3624,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	pm_runtime_mark_last_busy(base->dev);
 	pm_runtime_set_active(base->dev);
 	pm_runtime_enable(base->dev);
+	runtime_pm_enabled = true;
 
 	ret = d40_dmaengine_init(base, num_reserved_chans);
 	if (ret)
@@ -3658,7 +3660,8 @@ static int __init d40_probe(struct platform_device *pdev)
 		regulator_disable(base->lcpa_regulator);
 		regulator_put(base->lcpa_regulator);
 	}
-	pm_runtime_disable(base->dev);
+	if (runtime_pm_enabled)
+		pm_runtime_disable(base->dev);
 
  report_failure:
 	d40_err(dev, "probe failed\n");

-- 
2.55.0


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

* [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak
  2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
  2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
  2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
@ 2026-08-19 22:35 ` Linus Walleij
  2026-08-19 22:52   ` sashiko-bot
  2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:35 UTC (permalink / raw)
  To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot

d40_probe() registers the hardware interrupt before several later probe
steps that can fail. Those error paths jump to destroy_cache without
freeing the IRQ, leaving the handler registered after probe resources have
been released.

Track successful IRQ registration and free the IRQ on later probe failure.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/dma/ste_dma40.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0c8ff299c2c2..a723133d096e 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3503,6 +3503,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	struct resource res_lcpa;
 	int num_reserved_chans;
 	bool runtime_pm_enabled = false;
+	bool irq_requested = false;
 	u32 val;
 	int ret;
 
@@ -3595,6 +3596,7 @@ static int __init d40_probe(struct platform_device *pdev)
 		d40_err(dev, "No IRQ defined\n");
 		goto destroy_cache;
 	}
+	irq_requested = true;
 
 	if (base->plat_data->use_esram_lcla) {
 
@@ -3660,6 +3662,8 @@ static int __init d40_probe(struct platform_device *pdev)
 		regulator_disable(base->lcpa_regulator);
 		regulator_put(base->lcpa_regulator);
 	}
+	if (irq_requested)
+		free_irq(base->irq, base);
 	if (runtime_pm_enabled)
 		pm_runtime_disable(base->dev);
 

-- 
2.55.0


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

* [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check
  2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
                   ` (2 preceding siblings ...)
  2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
@ 2026-08-19 22:36 ` Linus Walleij
  2026-08-19 22:50   ` sashiko-bot
  3 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2026-08-19 22:36 UTC (permalink / raw)
  To: Vinod Koul, Frank Li; +Cc: dmaengine, Linus Walleij, sashiko-bot

d40_validate_conf() checks the raw dev_type against num_log_chans,
but d40_allocate_channel() derives the lookup_log_chans index as either
2 * dev_type or 2 * dev_type + 1.

Validate the dev_type against the derived logical channel index limit so
channel allocation cannot write past lookup_log_chans.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/dma/ste_dma40.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index a723133d096e..eda3f91741ed 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1727,15 +1727,26 @@ static int d40_validate_conf(struct d40_chan *d40c,
 {
 	int res = 0;
 	bool is_log = conf->mode == STEDMA40_MODE_LOGICAL;
+	bool invalid_dev_type = conf->dev_type < 0;
 
 	if (!conf->dir) {
 		chan_err(d40c, "Invalid direction.\n");
 		res = -EINVAL;
 	}
 
-	if ((is_log && conf->dev_type > d40c->base->num_log_chans)  ||
-	    (!is_log && conf->dev_type > d40c->base->num_phy_chans) ||
-	    (conf->dev_type < 0)) {
+	if (!invalid_dev_type && is_log) {
+		int max_dev_type;
+
+		if (conf->dir == DMA_DEV_TO_MEM)
+			max_dev_type = DIV_ROUND_UP(d40c->base->num_log_chans, 2);
+		else
+			max_dev_type = d40c->base->num_log_chans / 2;
+
+		invalid_dev_type = conf->dev_type >= max_dev_type;
+	}
+
+	if (invalid_dev_type ||
+	    (!is_log && conf->dev_type > d40c->base->num_phy_chans)) {
 		chan_err(d40c, "Invalid device type (%d)\n", conf->dev_type);
 		res = -EINVAL;
 	}

-- 
2.55.0


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

* Re: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup
  2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
@ 2026-08-19 22:46   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-19 22:46 UTC (permalink / raw)
  To: Linus Walleij; +Cc: vkoul, dmaengine, Frank.Li

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Returning ERR_PTR(err) from d40_queue_start() introduces a PM runtime reference leak and breaks caller logic in dma_tc_handle().
- [High] Freeing a submitted descriptor directly on error bypasses the DMA framework, causing the client to hang.
--

--- Patch [1]: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup ---
commit 0baedf8f0f8ca2b28e1d6d6a6bce21f2db82302b
Author: Linus Walleij <linusw@kernel.org>
Date:   Wed Aug 19 22:35:57 2026 +0000

dmaengine: ste_dma40: Fix failed start cleanup

If d40_start() fails after a queued descriptor has been moved to the active
list, d40_queue_start() currently returns NULL without unwinding the
descriptor state or clearing the channel busy flag. This commit attempts
to fix the error path by freeing the descriptor and returning an error pointer.

Link: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/

> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1531,8 +1531,12 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
>  		/* Start dma job */
>  		err = d40_start(d40c);
>  
> -		if (err)
> -			return NULL;
> +		if (err) {
> +			d40_desc_remove(d40d);
> +			d40_desc_free(d40c, d40d);

[Severity: High]
Does silently freeing the submitted descriptor here cause the DMA client to
hang?

The descriptor has already been submitted to the DMA framework, which means the
client holds a cookie and expects a completion callback. Bypassing the
framework and freeing the memory directly prevents the framework from completing
the cookie, meaning the client will wait indefinitely for a callback that will
never arrive.

> +			d40c->busy = false;
> +			return ERR_PTR(err);

[Severity: High]
Does returning ERR_PTR(err) here introduce a PM runtime reference leak?

Earlier in d40_queue_start(), a PM runtime reference is acquired:

	if (d40d != NULL) {
		if (!d40c->busy) {
			d40c->busy = true;
			pm_runtime_get_sync(d40c->base->dev);
		}

This error path returns ERR_PTR(err) and sets busy to false without dropping
that newly acquired reference.

Additionally, changing the return value breaks the caller logic in
dma_tc_handle(), which explicitly checks for a NULL return to drop the PM
reference:

drivers/dma/ste_dma40.c:dma_tc_handle() {
    ...
		if (d40_queue_start(d40c) == NULL) {
			d40c->busy = false;
			pm_runtime_put_autosuspend(d40c->base->dev);
		}
    ...
}

Because ERR_PTR(err) is not NULL, this condition evaluates to false, skipping
pm_runtime_put_autosuspend() and leaking the reference.

> +		}
>  	}
>  
>  	return d40d;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=1

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

* Re: [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check
  2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
@ 2026-08-19 22:50   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-19 22:50 UTC (permalink / raw)
  To: Linus Walleij; +Cc: vkoul, dmaengine, Frank.Li

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

Pre-existing issues:
- [High] Out-of-bounds access on the `phys` array and potentially arbitrary MMIO writes during channel allocation due to unconstrained `event_group`.
- [High] The out-of-bounds write fix for `lookup_log_chans` is bypassed for memcpy channels configured via the Device Tree.
- [High] Global buffer overflow when reading `memcpy-channels` from Device Tree.
--

--- Patch [4]: [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check ---
commit 53c98a760123837b647fa917a72020a724b5f32f
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix logical channel bounds check
    
d40_validate_conf() checks the raw dev_type against num_log_chans,
but d40_allocate_channel() derives the lookup_log_chans index as either
2 * dev_type or 2 * dev_type + 1.

Validate the dev_type against the derived logical channel index limit so
channel allocation cannot write past lookup_log_chans.
    
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index a723133d096ec..eda3f91741ed5 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1727,15 +1727,26 @@ static int d40_validate_conf(struct d40_chan *d40c,
>  {
>  	int res = 0;
>  	bool is_log = conf->mode == STEDMA40_MODE_LOGICAL;
> +	bool invalid_dev_type = conf->dev_type < 0;
>  
>  	if (!conf->dir) {
>  		chan_err(d40c, "Invalid direction.\n");
>  		res = -EINVAL;
>  	}
>  
> -	if ((is_log && conf->dev_type > d40c->base->num_log_chans)  ||
> -	    (!is_log && conf->dev_type > d40c->base->num_phy_chans) ||
> -	    (conf->dev_type < 0)) {
> +	if (!invalid_dev_type && is_log) {
> +		int max_dev_type;
> +
> +		if (conf->dir == DMA_DEV_TO_MEM)
> +			max_dev_type = DIV_ROUND_UP(d40c->base->num_log_chans, 2);
> +		else
> +			max_dev_type = d40c->base->num_log_chans / 2;
> +
> +		invalid_dev_type = conf->dev_type >= max_dev_type;
> +	}

[Severity: High]
This is a pre-existing issue, but does this bounds check fully protect the
phys array during channel allocation?

In d40_allocate_channel(), phy_num is calculated as j + event_group * 2,
where event_group is derived from dev_type / 16.

If num_log_chans is heavily scaled compared to num_phy_chans, could an
allowed logical dev_type produce an event_group that causes
event_group * 2 to greatly exceed the physical channel count?

drivers/dma/ste_dma40.c:d40_allocate_channel() {
    ...
            for (i = phy_num; i < phy_num + 2; i++) {
                if (d40_alloc_mask_set(&phys[i], ...
    ...
}

Could this cause out-of-bounds accesses on the phys array, as well as
arbitrary MMIO writes later in __d40_set_prio_rt() which uses the
unvalidated group multiplier?

drivers/dma/ste_dma40.c:__d40_set_prio_rt() {
    ...
        writel(bit, d40c->base->virtbase + rtreg + group * 4);
    ...
}

[Severity: High]
This is also a pre-existing issue, but is the validation in
d40_validate_conf() bypassed for memcpy channels configured via the
Device Tree?

In the fallback configuration path d40_config_memcpy(), the driver
assigns dev_type directly from the dma40_memcpy_channels array without
calling d40_validate_conf():

drivers/dma/ste_dma40.c:d40_config_memcpy() {
    ...
        d40c->dma_cfg.dev_type = dma40_memcpy_channels[d40c->chan.chan_id];
    ...
}

If the device tree supplies an arbitrarily large ID for a memcpy channel
via the memcpy-channels property, could this unvalidated dev_type cause
an out-of-bounds write into lookup_log_chans during allocation, subverting
this patch's fix?

drivers/dma/ste_dma40.c:d40_allocate_channel() {
    ...
        d40c->base->lookup_log_chans[d40c->log_num] = d40c;
    ...
}

[Severity: High]
Another pre-existing issue, but is there a risk of a global buffer overflow
when reading memcpy-channels from the Device Tree?

The dma40_memcpy_channels array is statically sized with exactly 6 elements:

drivers/dma/ste_dma40.c:
static int dma40_memcpy_channels[] = {
    DB8500_DMA_MEMCPY_EV_0,
    ...
    DB8500_DMA_MEMCPY_EV_5,
};

However, in d40_probe(), the DT parsing logic reads memcpy-channels up to
D40_MEMCPY_MAX_CHANS (which is 8) via of_property_read_u32_array():

drivers/dma/ste_dma40.c:d40_probe() {
    ...
        of_property_read_u32_array(np, "memcpy-channels",
                                   dma40_memcpy_channels,
                                   num_memcpy);
    ...
}

If a Device Tree blob provides 7 or 8 elements in the memcpy-channels
property, will the driver write past the end of the 6-element array during
initialization?

> +
> +	if (invalid_dev_type ||
> +	    (!is_log && conf->dev_type > d40c->base->num_phy_chans)) {
>  		chan_err(d40c, "Invalid device type (%d)\n", conf->dev_type);
>  		res = -EINVAL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=4

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

* Re: [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable
  2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
@ 2026-08-19 22:51   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-19 22:51 UTC (permalink / raw)
  To: Linus Walleij; +Cc: vkoul, Frank.Li, dmaengine

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

New issues:
- [Medium] The commit message claims to fix a bug regarding a 'later successful deferred probe', but deferred probe is structurally impossible for this driver.

Pre-existing issues:
- [High] Calling `free_pages()` on an `ioremap` mapped address in the probe error path.
- [High] IRQ handler accesses hardware registers without checking runtime PM state.
- [High] Passing a raw page count instead of an allocation order to `__get_free_pages()` and `free_pages()`.
--

--- Patch [2]: [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable ---
commit 724a6133520581f191bdd536bd3123c1d0c353fb
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix probe runtime PM disable

d40_probe() can jump to destroy_cache before runtime PM has been enabled
for the DMA controller device. Calling pm_runtime_disable() on that path
increments disable_depth, which can leave runtime PM disabled after a later
successful deferred probe.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <linusw@kernel.org>

> d40_probe() can jump to destroy_cache before runtime PM has been enabled
> for the DMA controller device. Calling pm_runtime_disable() on that path
> increments disable_depth, which can leave runtime PM disabled after a later
> successful deferred probe.

[Severity: Medium]
Is this description accurate regarding deferred probe?

The stedma40 driver is registered using platform_driver_probe() via
stedma40_init(), which sets drv->prevent_deferred_probe = true and prevents
any future binds.

A later successful deferred probe appears to be impossible here, even though
fixing the unbalanced disable_depth is still semantically correct.

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index d5837aa0a789a..0c8ff299c2c2d 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c

[ ... ]

> @@ -3658,7 +3660,8 @@ static int __init d40_probe(struct platform_device *pdev)
>  		regulator_disable(base->lcpa_regulator);
>  		regulator_put(base->lcpa_regulator);
>  	}

[Severity: High]
This is a pre-existing issue, but does the error path in destroy_cache
call free_pages() on an ioremap address?

If base->plat_data->use_esram_lcla is true, memory is mapped using
devm_ioremap() and assigned to base->lcla_pool.base. Just above this
regulator cleanup in destroy_cache, the code does:

    if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
        free_pages((unsigned long)base->lcla_pool.base,
                   base->lcla_pool.pages);

Calling virt_to_page() on an ioremap address yields a garbage pointer,
which would cause a crash when __free_pages() dereferences it.

> -	pm_runtime_disable(base->dev);
> +	if (runtime_pm_enabled)
> +		pm_runtime_disable(base->dev);
>  
>   report_failure:
>  	d40_err(dev, "probe failed\n");

[Severity: High]
This is a pre-existing issue, but does the interrupt handler access
hardware registers without checking the runtime PM state?

A spurious interrupt could fire while the device is in runtime suspend.
The IRQ handler d40_handle_interrupt() unconditionally accesses the
PrimeCell registers:

drivers/dma/ste_dma40.c:d40_handle_interrupt() {
    ...
    regs[i] = readl(base->virtbase + il[i].src);
    ...
}

Since dma40_runtime_suspend() turns off the GCC clock, reading the unclocked
registers without calling pm_runtime_get_if_active() could cause a bus
fault (external abort).

[Severity: High]
This is a pre-existing issue, but does d40_lcla_allocate() pass a raw
page count instead of an allocation order?

During allocation, base->lcla_pool.pages is calculated as the absolute
number of pages needed:

drivers/dma/ste_dma40.c:d40_lcla_allocate() {
    ...
    base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE;
    ...
    page_list[i] = __get_free_pages(GFP_KERNEL,
                                    base->lcla_pool.pages);
    ...
}

This raw count is then erroneously passed as the order parameter to
__get_free_pages(). For example, an order of 8 would allocate 256 pages
instead of the 8 pages actually needed, leading to wasted memory or
potential allocation failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=2

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

* Re: [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak
  2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
@ 2026-08-19 22:52   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-19 22:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: dmaengine, vkoul, Frank.Li

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

Pre-existing issues:
- [High] The probe error path incorrectly calls free_pages() on an ioremap pointer when ESRAM is used for LCLA.
- [High] The DMA engine devices are registered using devres (dmaenginem_async_device_register), but core memory resources and the interrupt handler are manually freed in the probe error path, risking Use-After-Free.
- [High] d40_lcla_allocate() passes a calculated number of pages instead of a logarithmic order to __get_free_pages(), leading to severe memory overallocation.
- [High] The DMA engine is registered and exposed to other subsystems before hardware initialization is complete.
- [High] Out-of-bounds write in the global dma40_memcpy_channels array via unverified device tree input.
--

--- Patch [3]: [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak ---
commit 5f3ddaa41f4f5eab5f519e10436cd8e82088831f
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix probe IRQ leak

d40_probe() registers the hardware interrupt before several later probe
steps that can fail. Those error paths jump to destroy_cache without
freeing the IRQ, leaving the handler registered after probe resources have
been released. This patch tracks successful IRQ registration and frees the
IRQ on later probe failure.

Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 0c8ff299c2c2d..a723133d096ec 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3503,6 +3503,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  	struct resource res_lcpa;
>  	int num_reserved_chans;
>  	bool runtime_pm_enabled = false;
> +	bool irq_requested = false;
>  	u32 val;
>  	int ret;
>  

[ ... ]

> @@ -3595,6 +3596,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  		d40_err(dev, "No IRQ defined\n");
>  		goto destroy_cache;
>  	}
> +	irq_requested = true;
>  
>  	if (base->plat_data->use_esram_lcla) {
>  

[ ... ]

> @@ -3660,6 +3662,8 @@ static int __init d40_probe(struct platform_device *pdev)
>  		regulator_disable(base->lcpa_regulator);
>  		regulator_put(base->lcpa_regulator);
>  	}
> +	if (irq_requested)
> +		free_irq(base->irq, base);

[Severity: High]
This isn't a bug introduced by this patch, but does the destroy_cache error
path call free_pages() on an ioremap pointer when ESRAM is used for LCLA?

In d40_probe(), if base->plat_data->use_esram_lcla is true,
base->lcla_pool.base is initialized with devm_ioremap():

drivers/dma/ste_dma40.c:d40_probe() {
    ...
    if (base->plat_data->use_esram_lcla) {
        ...
        base->lcla_pool.base = devm_ioremap(dev, res->start,
                                            resource_size(res));
    ...
}

However, in the destroy_cache error path, it seems to be freed using
free_pages():

drivers/dma/ste_dma40.c:d40_probe() {
    ...
 destroy_cache:
    ...
    if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
        free_pages((unsigned long)base->lcla_pool.base,
                   base->lcla_pool.pages);
    ...
}

Would passing this MMIO address to the page allocator cause a VM_BUG_ON or
memory corruption?

[Severity: High]
This isn't a bug introduced by this patch, but does the error path risk a
use-after-free by manually freeing core memory resources while the DMA engine
devices are still registered?

In d40_probe(), the DMA engine devices are registered using
d40_dmaengine_init():

drivers/dma/ste_dma40.c:d40_probe() {
    ...
    ret = d40_dmaengine_init(base, num_reserved_chans);
    if (ret)
        goto destroy_cache;
    ...
}

If a failure occurs after this point, the destroy_cache path manually frees
lcla_pool and the IRQ handler. Since d40_dmaengine_init() likely uses devres
to register the DMA devices, they remain globally visible until the probe
function returns and devres unwinds.

Could this allow a concurrent client to initiate a DMA transfer using freed
resources?

[Severity: High]
This is a pre-existing issue, but is the DMA engine registered and exposed to
other subsystems before hardware initialization is complete?

In d40_probe(), the DMA devices are registered via d40_dmaengine_init()
before d40_hw_init() is called:

drivers/dma/ste_dma40.c:d40_probe() {
    ...
    ret = d40_dmaengine_init(base, num_reserved_chans);
    if (ret)
        goto destroy_cache;
    ...
    d40_hw_init(base);
    ...
}

If a client requests a channel and starts a transfer before d40_hw_init()
finishes, could this lead to a bus lockup or corrupted transfers?

[Severity: High]
This is a pre-existing issue, but does d40_lcla_allocate() pass a page count
instead of a logarithmic order to __get_free_pages()?

In d40_lcla_allocate(), the required number of pages is calculated and stored
in base->lcla_pool.pages:

drivers/dma/ste_dma40.c:d40_lcla_allocate() {
    ...
    base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE;
    ...
    page_list[i] = __get_free_pages(GFP_KERNEL,
                                    base->lcla_pool.pages);
    ...
}

If SZ_1K * base->num_phy_chans / PAGE_SIZE evaluates to 8, wouldn't passing 8
as the order request 2^8 (256) pages instead of 8, leading to massive memory
overallocation?

[Severity: High]
This isn't a bug introduced by this patch, but is there an out-of-bounds
write when parsing memcpy-channels from the device tree?

The global array dma40_memcpy_channels is sized to exactly 6 elements:

drivers/dma/ste_dma40.c:
    static int dma40_memcpy_channels[] = {
        DB8500_DMA_MEMCPY_EV_0,
        ...
        DB8500_DMA_MEMCPY_EV_5,
    };

In d40_of_probe(), the length is checked against D40_MEMCPY_MAX_CHANS (which
is 8), and then of_property_read_u32_array() writes up to 8 elements:

drivers/dma/ste_dma40.c:d40_of_probe() {
    ...
    if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) {
        ...
    }
    ...
    of_property_read_u32_array(np, "memcpy-channels",
                               dma40_memcpy_channels,
                               num_memcpy);
    ...
}

Could this overwrite adjacent kernel data if the device tree specifies 7 or 8
elements?

>  	if (runtime_pm_enabled)
>  		pm_runtime_disable(base->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org?part=3

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

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

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19 22:35 [PATCH 0/4] dmaengine: ste_dma40: Fix probe and allocation bugs Linus Walleij
2026-08-19 22:35 ` [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
2026-08-19 22:46   ` sashiko-bot
2026-08-19 22:35 ` [PATCH 2/4] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
2026-08-19 22:51   ` sashiko-bot
2026-08-19 22:35 ` [PATCH 3/4] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
2026-08-19 22:52   ` sashiko-bot
2026-08-19 22:36 ` [PATCH 4/4] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
2026-08-19 22:50   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox