[PATCH v2 00/13] dmaengine: ste_dma40: Fix probe and allocation bugs

[PATCH v2 00/13] dmaengine: ste_dma40: Fix probe and allocation bugs

[Linus Walleij]

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

The fixes cover failed hardware-start cleanup, IRQ handling while runtime
suspended, hardware initialization before DMAengine registration,
DMAengine registration unwinding, LCLA allocation and probe error
unwinding, runtime PM and IRQ registration, device tree memcpy channel
parsing and validation, validation of logical channel and event-group
indexes derived from dev_type before writing into lookup_log_chans or
touching phy_res, and duplicate DMAengine debugfs names when several
DMAengine devices share the same parent device.

The method taken is: whenever Sashiko complains: fix the bug it complains
about if possible.

This has been boot tested on the Samsung Skomer device.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Changes in v2:
- In patch 1, complete the failed-start descriptor through the normal
  tasklet path and drop the runtime PM reference instead of freeing the
  submitted descriptor directly.
- Add an IRQ fix to avoid register access when DMA40 is runtime suspended.
- Add a probe ordering fix so DMA40 hardware is initialized before DMAengine
  devices are registered.
- Add a probe unwind fix so DMAengine registrations are released before
  freeing IRQ and LCLA resources.
- Add an LCLA allocation fix so __get_free_pages() and free_pages() use an
  allocation order instead of a raw page count.
- Add a probe unwind fix so ESRAM LCLA mappings are not released with
  free_pages().
- Add a device tree parsing fix so memcpy-channels cannot overflow the
  memcpy channel array.
- Add a dev_type bounds fix so derived event groups cannot overflow phy_res
  or the priority/realtime register window.
- Add validation for fallback memcpy configurations so memcpy-channels
  entries cannot bypass the dev_type bounds checks.
- Add a DMAengine debugfs naming fix so drivers registering several
  DMAengine devices for one parent device do not trigger duplicate-name
  warnings.
- Link to v1: https://lore.kernel.org/r/20260820-dma40-fixes-v1-0-5e14815ad689@kernel.org

---
Linus Walleij (13):
      dmaengine: ste_dma40: Fix failed start cleanup
      dmaengine: ste_dma40: Check runtime PM in IRQ
      dmaengine: ste_dma40: Init hardware before registration
      dmaengine: ste_dma40: Fix DMA registration unwind
      dmaengine: ste_dma40: Fix LCLA allocation order
      dmaengine: ste_dma40: Fix probe LCLA free
      dmaengine: ste_dma40: Fix probe runtime PM disable
      dmaengine: ste_dma40: Fix probe IRQ leak
      dmaengine: ste_dma40: Fix memcpy channel parsing
      dmaengine: ste_dma40: Fix logical channel bounds check
      dmaengine: ste_dma40: Fix event group bounds
      dmaengine: ste_dma40: Validate memcpy configuration
      dmaengine: Use unique debugfs names

 drivers/dma/dmaengine.c |  21 +++++++-
 drivers/dma/ste_dma40.c | 126 ++++++++++++++++++++++++++++++++++++------------
 2 files changed, 113 insertions(+), 34 deletions(-)
---
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
change-id: 20260820-dma40-fixes-b99af66002bf

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

[PATCH v2 01/13] dmaengine: ste_dma40: Fix failed start cleanup

[Linus Walleij]

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

Fix this pre-existing error path by completing the descriptor through the
normal tasklet path, clearing the busy flag, balancing the runtime PM
reference and returning an error pointer to distinguish the failure from
the no-work case. Do not free the descriptor directly, since it has
already been submitted and has a DMA cookie.

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 | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..facc1bcf42a0 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1531,8 +1531,15 @@ 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_done(d40c, d40d);
+			d40c->pending_tx++;
+			d40c->busy = false;
+			pm_runtime_put_autosuspend(d40c->base->dev);
+			tasklet_schedule(&d40c->tasklet);
+			return ERR_PTR(err);
+		}
 	}
 
 	return d40d;

-- 
2.55.0

Re: [PATCH v2 01/13] dmaengine: ste_dma40: Fix failed start cleanup

[sashiko-bot]

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

New issues:
- [Medium] Out-of-order descriptor completion corrupts dmaengine cookie tracking.
- [Medium] Failed cyclic descriptors are moved to the `done` list, causing a permanent tasklet soft-lock.

Pre-existing issues:
- [High] The return value of pm_runtime_get_sync() is ignored, risking hardware access on a powered-off device.
--

--- Patch [1]: [PATCH v2 01/13] dmaengine: ste_dma40: Fix failed start cleanup ---
commit b33a3592aafc5e2714045326450fe12077ee3161
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix failed start cleanup

This commit addresses a bug where d40_queue_start() returned NULL on start
failure without unwinding the transfer state or clearing the busy flag. It
introduces an error path to complete the descriptor through the tasklet and
balance the runtime PM reference.

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
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1531,8 +1531,15 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
>  	if (d40d != NULL) {
>  		if (!d40c->busy) {
>  			d40c->busy = true;
>  			pm_runtime_get_sync(d40c->base->dev);

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
pm_runtime_get_sync() here?

If the power domain fails to resume, the driver might proceed with hardware
register accesses like d40_desc_load() and d40_start() while the device is
powered off, which could trigger a synchronous external abort.

[ ... ]
>  
>  		/* Start dma job */
>  		err = d40_start(d40c);
>  
> -		if (err)
> -			return NULL;
> +		if (err) {
> +			d40_desc_remove(d40d);
> +			d40_desc_done(d40c, d40d);

[Severity: Medium]
Does calling d40_desc_done() here break dmaengine cookie tracking by
completing descriptors out-of-order?

If dma_tc_handle() calls d40_queue_start() and this new descriptor fails to
start, it is placed into the done list. Then dma_tc_handle() proceeds to
place the successfully completed old descriptor into the done list. Since it
is a FIFO queue, the newly failed descriptor gets processed before the old
successful one, which could cause the channel's completed cookie to go
backwards.

[Severity: Medium]
Also, if the descriptor is cyclic, could moving it to the done list upon
failure cause a permanent tasklet soft-lock?

The dma_tasklet() deliberately skips removing cyclic descriptors from the
done list:

dma_tasklet() {
        ...
        if (!d40d->cyclic) {
                if (async_tx_test_ack(&d40d->txd)) {
                        d40_desc_remove(d40d);
        ...
}

If a failed cyclic descriptor is forced into the done list, it looks like it
would never be dequeued, causing the tasklet to infinitely loop and starve
other DMA jobs.

> +			d40c->pending_tx++;
> +			d40c->busy = false;
> +			pm_runtime_put_autosuspend(d40c->base->dev);
> +			tasklet_schedule(&d40c->tasklet);
> +			return ERR_PTR(err);
> +		}
>  	}
>  
>  	return d40d;

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

[PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ

[Linus Walleij]

d40_handle_interrupt() reads DMA40 interrupt registers unconditionally. A
spurious interrupt can arrive while the device is runtime suspended, after
dma40_runtime_suspend() has disabled the GCC clock.

Avoid touching the registers unless the device is runtime active by taking
a conditional runtime PM reference. Return IRQ_NONE when the device is
suspended, and drop the reference after handling an active interrupt.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index facc1bcf42a0..dd449d4285ef 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1674,6 +1674,11 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
 	u32 *regs = base->regs_interrupt;
 	struct d40_interrupt_lookup *il = base->gen_dmac.il;
 	u32 il_size = base->gen_dmac.il_size;
+	int ret;
+
+	ret = pm_runtime_get_if_active(base->dev);
+	if (ret <= 0)
+		return IRQ_NONE;
 
 	spin_lock(&base->interrupt_lock);
 
@@ -1722,6 +1727,8 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
 
 	spin_unlock(&base->interrupt_lock);
 
+	pm_runtime_put_autosuspend(base->dev);
+
 	return IRQ_HANDLED;
 }
 

-- 
2.55.0

Re: [PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ

[sashiko-bot]

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

New issues:
- [High] Checking `ret <= 0` from `pm_runtime_get_if_active` unconditionally returns IRQ_NONE when Runtime PM is disabled, causing a deterministic interrupt storm.

Pre-existing issues:
- [High] The interrupt handler unconditionally returns IRQ_HANDLED even if no interrupt status bits were found, defeating spurious interrupt detection and causing CPU lockups.
--

--- Patch [2]: [PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ ---
commit a32adbd5256a0eee9c50c9b0d29d46183f86ef06
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Check runtime PM in IRQ
    
d40_handle_interrupt() reads DMA40 interrupt registers unconditionally. A
spurious interrupt can arrive while the device is runtime suspended, after
dma40_runtime_suspend() has disabled the GCC clock.

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

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index facc1bcf42a03..dd449d4285efb 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1674,6 +1674,11 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
>  	u32 *regs = base->regs_interrupt;
>  	struct d40_interrupt_lookup *il = base->gen_dmac.il;
>  	u32 il_size = base->gen_dmac.il_size;
> +	int ret;
> +
> +	ret = pm_runtime_get_if_active(base->dev);
> +	if (ret <= 0)
> +		return IRQ_NONE;

[Severity: High]
If the kernel is compiled with CONFIG_PM=n, pm_runtime_get_if_active() returns
-EINVAL. Doesn't checking ret <= 0 mean that valid interrupts will be
rejected with IRQ_NONE when Runtime PM is disabled?

This would leave the hardware interrupt line asserted and cause a
deterministic interrupt storm until the kernel's spurious interrupt detector
disables the IRQ line.

[ ... ]

> @@ -1722,6 +1727,8 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
>  
>  	spin_unlock(&base->interrupt_lock);
>  
> +	pm_runtime_put_autosuspend(base->dev);
> +
>  	return IRQ_HANDLED;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but the interrupt handler
unconditionally returns IRQ_HANDLED even if no interrupt status bits were
found earlier in the function:

	/* No more set bits found? */
	if (chan == BITS_PER_LONG * il_size)
		break;

If a hardware glitch asserts the DMA interrupt line without setting any bits,
the loop breaks without clearing any interrupts. By unconditionally returning
IRQ_HANDLED here at the end of the function, does this prevent the kernel's
spurious interrupt detector from counting the unhandled interrupt and
gracefully disabling the faulty IRQ line?

This could cause the CPU to enter an infinite loop of servicing the
unacknowledged hardware interrupt.

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

[PATCH v2 03/13] dmaengine: ste_dma40: Init hardware before registration

[Linus Walleij]

d40_probe() registers DMAengine devices before calling d40_hw_init(). Once
the DMAengine devices are registered, channels can be requested and a
client can issue transfers before the DMA40 hardware has been initialized.

Initialize the hardware and set the DMA segment limit before registering
the DMAengine devices, so no globally visible channel can start from the
uninitialized hardware state.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/dmaengine/20260819225215.5EA2F1F000E9@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, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index dd449d4285ef..145f7105218c 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3634,14 +3634,14 @@ static int __init d40_probe(struct platform_device *pdev)
 	pm_runtime_set_active(base->dev);
 	pm_runtime_enable(base->dev);
 
-	ret = d40_dmaengine_init(base, num_reserved_chans);
-	if (ret)
-		goto destroy_cache;
-
 	dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
 
 	d40_hw_init(base);
 
+	ret = d40_dmaengine_init(base, num_reserved_chans);
+	if (ret)
+		goto destroy_cache;
+
 	ret = of_dma_controller_register(np, d40_xlate, NULL);
 	if (ret) {
 		dev_err(dev,

-- 
2.55.0

[PATCH v2 04/13] dmaengine: ste_dma40: Fix DMA registration unwind

[Linus Walleij]

d40_dmaengine_init() registers DMAengine devices using devres-managed
unregister actions. If probe fails after one of those registrations, the
DMAengine devices stay visible until devres unwinds after d40_probe()
returns.

Release the DMAengine registration devres group before freeing the IRQ and
LCLA resources on the probe error path. Keep the managed registrations in
place on successful probe by removing only the temporary group markers.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 145f7105218c..384c2f3805b0 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3511,6 +3511,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	struct d40_base *base;
 	struct resource *res;
 	struct resource res_lcpa;
+	void *dmaenginem_reg_group;
 	int num_reserved_chans;
 	u32 val;
 	int ret;
@@ -3638,20 +3639,29 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	d40_hw_init(base);
 
+	dmaenginem_reg_group = devres_open_group(dev, NULL, GFP_KERNEL);
+	if (!dmaenginem_reg_group) {
+		ret = -ENOMEM;
+		goto destroy_cache;
+	}
+
 	ret = d40_dmaengine_init(base, num_reserved_chans);
 	if (ret)
-		goto destroy_cache;
+		goto release_dmaenginem;
 
 	ret = of_dma_controller_register(np, d40_xlate, NULL);
 	if (ret) {
 		dev_err(dev,
 			"could not register of_dma_controller\n");
-		goto destroy_cache;
+		goto release_dmaenginem;
 	}
+	devres_remove_group(dev, dmaenginem_reg_group);
 
 	dev_info(base->dev, "initialized\n");
 	return 0;
 
+ release_dmaenginem:
+	devres_release_group(dev, dmaenginem_reg_group);
  destroy_cache:
 	if (base->lcla_pool.dma_addr)
 		dma_unmap_single(base->dev, base->lcla_pool.dma_addr,

-- 
2.55.0

Re: [PATCH v2 04/13] dmaengine: ste_dma40: Fix DMA registration unwind

[sashiko-bot]

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

Pre-existing issues:
- [High] The DMA_CYCLIC capability is incorrectly applied to dma_slave instead of dma_both during dma_both setup.
- [High] The channel tasklets are not explicitly killed on the probe error path, which can result in a Use-After-Free if a tasklet is scheduled before the probe fails.
--

--- Patch [4]: [PATCH v2 04/13] dmaengine: ste_dma40: Fix DMA registration unwind ---
commit b477f2730e0cab6c69890c774c76310e0edd988b
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix DMA registration unwind

d40_dmaengine_init() registers DMAengine devices using devres-managed
unregister actions. This commit wraps the registration in a temporary
devres group, releasing it if probe fails to prevent DMAengine devices
from staying visible until devres unwinds.

Closes: https://lore.kernel.org/dmaengine/20260819225215.5EA2F1F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 145f7105218ce..384c2f3805b02 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -3638,20 +3639,29 @@ static int __init d40_probe(struct platform_device *pdev)
>  
>  	d40_hw_init(base);
>  
> +	dmaenginem_reg_group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!dmaenginem_reg_group) {
> +		ret = -ENOMEM;
> +		goto destroy_cache;
> +	}
> +
>  	ret = d40_dmaengine_init(base, num_reserved_chans);

[Severity: High]
This isn't a bug introduced by this patch, but does the configuration of
base->dma_both in d40_dmaengine_init() incorrectly set the DMA_CYCLIC
capability on base->dma_slave instead of base->dma_both?

Looking at d40_dmaengine_init():

drivers/dma/ste_dma40.c:d40_dmaengine_init() {
    ...
    dma_cap_zero(base->dma_both.cap_mask);
    dma_cap_set(DMA_SLAVE, base->dma_both.cap_mask);
    dma_cap_set(DMA_MEMCPY, base->dma_both.cap_mask);
    dma_cap_set(DMA_CYCLIC, base->dma_slave.cap_mask);
    ...
}

Should the last line be setting DMA_CYCLIC on base->dma_both.cap_mask so that
physical channels managed by dma_both advertise cyclic support?

>  	if (ret)
> -		goto destroy_cache;
> +		goto release_dmaenginem;
>  
>  	ret = of_dma_controller_register(np, d40_xlate, NULL);
>  	if (ret) {
>  		dev_err(dev,
>  			"could not register of_dma_controller\n");
> -		goto destroy_cache;
> +		goto release_dmaenginem;
>  	}
> +	devres_remove_group(dev, dmaenginem_reg_group);
>  
>  	dev_info(base->dev, "initialized\n");
>  	return 0;
>  
> + release_dmaenginem:
> +	devres_release_group(dev, dmaenginem_reg_group);

[Severity: High]
This is a pre-existing issue, but does this error path need to explicitly
kill the channel tasklets initialized during probe?

During d40_probe(), tasklets are initialized for all channels in
d40_chan_init(). Then d40_dmaengine_init() exposes the DMA devices via
dmaenginem_async_device_register().

At this point, a client could request a channel and submit a transfer,
causing a hardware interrupt to schedule the tasklet. If probe subsequently
fails here at of_dma_controller_register(), the driver unwinds and returns
an error, causing devm to free the base structure containing the tasklets.

Since tasklet_kill() is never called in the error path, could a scheduled
tasklet execute after the memory is freed, leading to a Use-After-Free crash?

>   destroy_cache:
>  	if (base->lcla_pool.dma_addr)
>  		dma_unmap_single(base->dev, base->lcla_pool.dma_addr,

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

[PATCH v2 05/13] dmaengine: ste_dma40: Fix LCLA allocation order

[Linus Walleij]

d40_lcla_allocate() calculates the number of pages needed for LCLA, but
passes that raw page count as the allocation order to __get_free_pages().
The same value is later passed to free_pages().

Store the allocation order with get_order() instead, and use a separate
byte size for allocation diagnostics, fallback kmalloc() sizing and DMA
mapping.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 384c2f3805b0..804eb5eab06b 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -411,8 +411,8 @@ struct d40_desc {
  * @dma_addr: DMA address, if mapped
  * @base_unaligned: The original kmalloc pointer, if kmalloc is used.
  * This pointer is only there for clean-up on error.
- * @pages: The number of pages needed for all physical channels.
- * Only used later for clean-up on error
+ * @alloc_order: Order used for the LCLA page allocation.
+ * Only used later for clean-up on error.
  * @lock: Lock to protect the content in this struct.
  * @alloc_map: big map over which LCLA entry is own by which job.
  */
@@ -420,7 +420,7 @@ struct d40_lcla_pool {
 	void		*base;
 	dma_addr_t	dma_addr;
 	void		*base_unaligned;
-	int		 pages;
+	unsigned int	 alloc_order;
 	spinlock_t	 lock;
 	struct d40_desc	**alloc_map;
 };
@@ -3372,6 +3372,7 @@ static void __init d40_hw_init(struct d40_base *base)
 static int __init d40_lcla_allocate(struct d40_base *base)
 {
 	struct d40_lcla_pool *pool = &base->lcla_pool;
+	size_t lcla_size = SZ_1K * base->num_phy_chans;
 	unsigned long *page_list;
 	int i, j;
 	int ret;
@@ -3387,20 +3388,20 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	if (!page_list)
 		return -ENOMEM;
 
-	/* Calculating how many pages that are required */
-	base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE;
+	base->lcla_pool.alloc_order = get_order(lcla_size);
 
 	for (i = 0; i < MAX_LCLA_ALLOC_ATTEMPTS; i++) {
 		page_list[i] = __get_free_pages(GFP_KERNEL,
-						base->lcla_pool.pages);
+						base->lcla_pool.alloc_order);
 		if (!page_list[i]) {
 
-			d40_err(base->dev, "Failed to allocate %d pages.\n",
-				base->lcla_pool.pages);
+			d40_err(base->dev, "Failed to allocate %zu bytes.\n",
+				lcla_size);
 			ret = -ENOMEM;
 
 			for (j = 0; j < i; j++)
-				free_pages(page_list[j], base->lcla_pool.pages);
+				free_pages(page_list[j],
+					   base->lcla_pool.alloc_order);
 			goto free_page_list;
 		}
 
@@ -3410,7 +3411,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	}
 
 	for (j = 0; j < i; j++)
-		free_pages(page_list[j], base->lcla_pool.pages);
+		free_pages(page_list[j], base->lcla_pool.alloc_order);
 
 	if (i < MAX_LCLA_ALLOC_ATTEMPTS) {
 		base->lcla_pool.base = (void *)page_list[i];
@@ -3420,10 +3421,9 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 		 * alignment, try with allocating a big buffer.
 		 */
 		dev_warn(base->dev,
-			 "[%s] Failed to get %d pages @ 18 bit align.\n",
-			 __func__, base->lcla_pool.pages);
-		base->lcla_pool.base_unaligned = kmalloc(SZ_1K *
-							 base->num_phy_chans +
+			 "[%s] Failed to get %zu bytes @ 18 bit align.\n",
+			 __func__, lcla_size);
+		base->lcla_pool.base_unaligned = kmalloc(lcla_size +
 							 LCLA_ALIGNMENT,
 							 GFP_KERNEL);
 		if (!base->lcla_pool.base_unaligned) {
@@ -3435,8 +3435,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 						 LCLA_ALIGNMENT);
 	}
 
-	pool->dma_addr = dma_map_single(base->dev, pool->base,
-					SZ_1K * base->num_phy_chans,
+	pool->dma_addr = dma_map_single(base->dev, pool->base, lcla_size,
 					DMA_TO_DEVICE);
 	if (dma_mapping_error(base->dev, pool->dma_addr)) {
 		pool->dma_addr = 0;
@@ -3670,7 +3669,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
 		free_pages((unsigned long)base->lcla_pool.base,
-			   base->lcla_pool.pages);
+			   base->lcla_pool.alloc_order);
 
 	kfree(base->lcla_pool.base_unaligned);
 

-- 
2.55.0

Re: [PATCH v2 05/13] dmaengine: ste_dma40: Fix LCLA allocation order

[sashiko-bot]

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

Pre-existing issues:
- [High] DMA_CYCLIC capability applied to wrong dma_device during initialization.
--

--- Patch [5]: [PATCH v2 05/13] dmaengine: ste_dma40: Fix LCLA allocation order ---
commit d327ef86f7f30a5a539d2fef537e53e5f0f2dc5f
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix LCLA allocation order

This commit modifies d40_lcla_allocate() to use get_order() to determine
the page allocation order, instead of directly passing the calculated
number of pages to __get_free_pages() and free_pages().

Closes: https://lore.kernel.org/dmaengine/20260819225114.AE1511F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 384c2f3805b02..804eb5eab06b4 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -3670,7 +3669,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  
>  	if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
>  		free_pages((unsigned long)base->lcla_pool.base,
> -			   base->lcla_pool.pages);
> +			   base->lcla_pool.alloc_order);
>  
>  	kfree(base->lcla_pool.base_unaligned);
>  

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
d40_dmaengine_init():

drivers/dma/ste_dma40.c:d40_dmaengine_init() {
    ...
	d40_chan_init(base, &base->dma_both, base->phy_chans,
		      0, num_reserved_chans);

	dma_cap_zero(base->dma_both.cap_mask);
	dma_cap_set(DMA_SLAVE, base->dma_both.cap_mask);
	dma_cap_set(DMA_MEMCPY, base->dma_both.cap_mask);
	dma_cap_set(DMA_CYCLIC, base->dma_slave.cap_mask);
    ...
}

Does this code apply the DMA_CYCLIC capability to the wrong dma_device during
initialization?

By setting DMA_CYCLIC on base->dma_slave.cap_mask instead of
base->dma_both.cap_mask, the dma_both device seems to lack this capability.

Will this cause initialization failures for peripherals requesting a cyclic
channel from dma_both?

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

[PATCH v2 06/13] dmaengine: ste_dma40: Fix probe LCLA free

[Linus Walleij]

When LCLA is placed in ESRAM, d40_probe() stores a devm_ioremap()
address in lcla_pool.base and leaves base_unaligned unset. The
destroy_cache error path can then pass the ioremap address to
free_pages().

Only free lcla_pool.base with free_pages() when the driver allocated the
LCLA pool from normal memory. The ESRAM mapping is devm-managed.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 804eb5eab06b..739e47941d0c 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3667,7 +3667,8 @@ static int __init d40_probe(struct platform_device *pdev)
 				 SZ_1K * base->num_phy_chans,
 				 DMA_TO_DEVICE);
 
-	if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
+	if (!base->plat_data->use_esram_lcla &&
+	    !base->lcla_pool.base_unaligned && base->lcla_pool.base)
 		free_pages((unsigned long)base->lcla_pool.base,
 			   base->lcla_pool.alloc_order);
 

-- 
2.55.0

[PATCH v2 07/13] dmaengine: ste_dma40: Fix probe runtime PM disable

[Linus Walleij]

Some d40_probe() error paths jump to destroy_cache before runtime PM has
been enabled for the DMA controller device. The label unconditionally
calls pm_runtime_disable(), which increments disable_depth even though
this probe attempt never enabled runtime PM.

Track whether this probe attempt enabled runtime PM before disabling it on
the error path. This is not about a later deferred-probe retry, since the
driver is registered with platform_driver_probe(); it keeps the probe
unwind balanced.

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 739e47941d0c..0c9094b4c4ad 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3512,6 +3512,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	struct resource res_lcpa;
 	void *dmaenginem_reg_group;
 	int num_reserved_chans;
+	bool runtime_pm_enabled = false;
 	u32 val;
 	int ret;
 
@@ -3633,6 +3634,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;
 
 	dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
 
@@ -3678,7 +3680,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

Re: [PATCH v2 07/13] dmaengine: ste_dma40: Fix probe runtime PM disable

[sashiko-bot]

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

Pre-existing issues:
- [High] request_irq() is called before PM runtime is enabled, which can cause an IRQ storm if an interrupt is pending.
- [High] The driver systematically ignores the return value of pm_runtime_get_sync(), failing to handle resume errors.
--

--- Patch [7]: [PATCH v2 07/13] dmaengine: ste_dma40: Fix probe runtime PM disable ---
commit 95c55fe7afd321bc228d4b1a735de2f4eb7a79fc
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix probe runtime PM disable

This commit fixes an issue where some d40_probe() error paths disable
runtime PM even if it was never enabled for the device. It introduces a
boolean flag to track whether runtime PM was actually enabled, keeping the
disable_depth balanced during probe failures.

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 739e47941d0cc..0c9094b4c4ade 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -3633,6 +3634,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;

[Severity: High]
This is a pre-existing issue, but request_irq() is called before PM
runtime is enabled, which could cause an IRQ storm if an interrupt is
pending.

If an interrupt triggers before runtime PM is enabled, the ISR
d40_handle_interrupt() fails to clear it:

d40_handle_interrupt() {
    ...
    ret = pm_runtime_get_if_active(base->dev);
    if (ret <= 0)
        return IRQ_NONE;
    ...
}

Because pm_runtime_get_if_active() returns an error when runtime PM is
disabled, the handler returns IRQ_NONE without clearing the hardware
interrupt. This causes the interrupt to continually re-trigger.

Could the request_irq() call be moved after pm_runtime_enable() to
prevent this?

[Severity: High]
This is a pre-existing issue, but the driver systematically ignores the
return value of pm_runtime_get_sync().

If the device fails to resume due to PM subsystem issues or memory
pressure, does this cause the driver to access powered-off hardware
registers?

For example, in d40_queue_start():

d40_queue_start() {
    ...
    pm_runtime_get_sync(d40c->base->dev);
    /* hardware accesses follow */
    ...
}

Similar unchecked calls exist in d40_pause(), d40_resume(), and
d40_free_dma(). Could these be updated to check for negative error codes
and abort if the resume fails?

>  
>  	dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
>  
> @@ -3678,7 +3680,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");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=7

[PATCH v2 08/13] dmaengine: ste_dma40: Fix probe IRQ leak

[Linus Walleij]

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 0c9094b4c4ad..8cf73b37f8bf 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3513,6 +3513,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	void *dmaenginem_reg_group;
 	int num_reserved_chans;
 	bool runtime_pm_enabled = false;
+	bool irq_requested = false;
 	u32 val;
 	int ret;
 
@@ -3605,6 +3606,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) {
 
@@ -3680,6 +3682,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

Re: [PATCH v2 08/13] dmaengine: ste_dma40: Fix probe IRQ leak

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware interrupt is requested before hardware initialization and Runtime PM enablement, creating a window for an immediate IRQ storm.
--

--- Patch [8]: [PATCH v2 08/13] dmaengine: ste_dma40: Fix probe IRQ leak ---
commit e1b29c17ec30abafeb3d5100e10a174f78b73842
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix probe IRQ leak

This commit fixes an IRQ leak in the probe error paths of the ste_dma40 driver
by tracking whether the IRQ was successfully requested and freeing it during
failure teardown.

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>

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 0c9094b4c4ade..8cf73b37f8bfd 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -3605,6 +3606,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  		d40_err(dev, "No IRQ defined\n");
>  		goto destroy_cache;
>  	}
> +	irq_requested = true;

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here create a
window for an immediate interrupt storm?

The interrupt is requested in d40_probe() before hardware initialization
(d40_hw_init()) and Runtime PM enablement (pm_runtime_enable()). If the
hardware interrupt is already asserted (for example, if left pending by a
bootloader), it will fire immediately.

Inside d40_handle_interrupt(), the handler checks Runtime PM status:

drivers/dma/ste_dma40.c:d40_handle_interrupt() {
    ...
	ret = pm_runtime_get_if_active(base->dev);
	if (ret <= 0)
		return IRQ_NONE;
    ...
}

Since pm_runtime_set_active() and pm_runtime_enable() have not yet been
executed in d40_probe(), Runtime PM is inactive. This causes the check to
fail, returning IRQ_NONE without servicing or clearing the hardware
interrupt.

For level-triggered interrupts, could this lead to an unhandled interrupt
storm that permanently disables the device's IRQ line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=8

[PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing

[Linus Walleij]

d40_of_probe() validates the memcpy-channels property against
D40_MEMCPY_MAX_CHANS, but reads the property directly into
dma40_memcpy_channels. That array has fewer entries, so a long property
can overwrite adjacent data.

Validate the property length against ARRAY_SIZE(dma40_memcpy_channels)
before reading it into the array.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 8cf73b37f8bf..4e840846466b 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3470,7 +3470,8 @@ static int __init d40_of_probe(struct device *dev,
 	list = of_get_property(np, "memcpy-channels", &num_memcpy);
 	num_memcpy /= sizeof(*list);
 
-	if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) {
+	if (num_memcpy > ARRAY_SIZE(dma40_memcpy_channels) ||
+	    num_memcpy <= 0) {
 		d40_err(dev,
 			"Invalid number of memcpy channels specified (%d)\n",
 			num_memcpy);

-- 
2.55.0

Re: [PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing

[sashiko-bot]

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

Pre-existing issues:
- [High] Out-of-bounds array access in `d40_phy_res_init` due to unvalidated `disabled-channels` property values.
- [Medium] Leaked OF node reference from `of_parse_phandle()` in `d40_probe()`.
- [Medium] Global `dma40_memcpy_channels` array is modified without locking, preventing multiple instances of the device from configuring correctly.
- [Medium] Unvalidated `args_count` in `d40_xlate` leads to reading uninitialized stack memory.
- [Medium] The driver fails to probe if the optional 'memcpy-channels' property is missing from the device tree.
--

--- Patch [9]: [PATCH v2 09/13] dmaengine: ste_dma40: Fix memcpy channel parsing ---
commit 50b8205871db7013fde7fb8ac4433303d26eb570
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix memcpy channel parsing

d40_of_probe() validates the memcpy-channels property against
D40_MEMCPY_MAX_CHANS, but reads the property directly into
dma40_memcpy_channels. That array has fewer entries, so a long property
can overwrite adjacent data.

Validate the property length against ARRAY_SIZE(dma40_memcpy_channels)
before reading it into the array.

Closes: https://lore.kernel.org/dmaengine/20260819225215.5EA2F1F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 8cf73b37f8bfd..4e840846466b8 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3470,7 +3470,8 @@ static int __init d40_of_probe(struct device *dev,
>  	list = of_get_property(np, "memcpy-channels", &num_memcpy);
>  	num_memcpy /= sizeof(*list);
>  
> -	if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) {
> +	if (num_memcpy > ARRAY_SIZE(dma40_memcpy_channels) ||
> +	    num_memcpy <= 0) {

[Severity: Medium]
This is a pre-existing issue, but does this cause the probe to fail if the
"memcpy-channels" property is completely omitted from the device tree? If
of_get_property() returns NULL, num_memcpy would be 0, which triggers this
error path. Should this property be strictly required, or can the driver
continue without it?

[Severity: Medium]
This is also a pre-existing issue, but does reading the "memcpy-channels"
property directly into the global dma40_memcpy_channels array cause problems
if there are multiple instances of this device? It seems probing subsequent
instances would silently overwrite the global array and corrupt the channel
mapping for previously probed controllers.

>  		d40_err(dev,
>  			"Invalid number of memcpy channels specified (%d)\n",
>  			num_memcpy);

[ ... ]

[Severity: Medium]
This isn't a bug introduced by this patch, but does d40_probe() leak an OF
node reference?

drivers/dma/ste_dma40.c:d40_probe() {
    ...
    np_lcpa = of_parse_phandle(np, "sram", 0);
    ...
}

The of_parse_phandle() call increments the node's reference count, but I
don't see a matching of_node_put() to release it. Can this cause a memory
leak when the driver probes?

[Severity: High]
This is a pre-existing issue, but is it possible for malformed device tree
values to cause an out-of-bounds write here?

drivers/dma/ste_dma40.c:d40_phy_res_init() {
    ...
    base->phy_res[chan].allocated_src = D40_ALLOC_PHY;
    ...
}

It appears d40_of_probe() reads the "disabled-channels" property but only
verifies the element count, without validating the actual channel integers.
If those unvalidated values are used as direct indices in d40_phy_res_init(),
could this corrupt the heap array?

[Severity: Medium]
This is a pre-existing issue, but does d40_xlate() read uninitialized memory
if the device tree is misconfigured?

drivers/dma/ste_dma40.c:d40_xlate() {
    ...
    flags = dma_spec->args[2];
    ...
}

If a device tree incorrectly specifies a dma-cells value less than 3, the
args array would only be partially initialized by of_parse_phandle_with_args().
Could unconditionally accessing args[2] without verifying args_count lead to
reading uninitialized stack data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=9

[PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check

[Linus Walleij]

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 4e840846466b..7c777e410c10 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1737,15 +1737,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

Re: [PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check

[sashiko-bot]

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

Pre-existing issues:
- [High] Changing DMA transfer direction after channel allocation corrupts allocation masks and leaks logical channels.
--

--- Patch [10]: [PATCH v2 10/13] dmaengine: ste_dma40: Fix logical channel bounds check ---
commit 77f491852f9c4972a650574c4a045eadf10238f0
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 differently. This
commit validates 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 4e840846466b8..7c777e410c104 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1737,15 +1737,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);

[Severity: High]
This is a pre-existing issue, but does changing the DMA transfer direction
after channel allocation corrupt allocation masks and leak logical channels?

If a DMA client allocates a channel configured for one direction (e.g.,
DMA_MEM_TO_DEV), and then prepares a transfer with the opposite direction,
drivers/dma/ste_dma40.c:d40_set_runtime_config_write() updates the internal
direction:

    cfg->dir = DMA_DEV_TO_MEM;

Later, during channel release, drivers/dma/ste_dma40.c:d40_free_dma() relies
on this mutated direction to determine which allocation mask to free:

    else if (d40c->dma_cfg.dir == DMA_DEV_TO_MEM)
        is_src = true;
    ...
    d40_alloc_mask_free(phy, is_src, chan_is_logical(d40c) ? event : 0);

Could this clear the bit in the wrong resource mask, leaking the originally
allocated resource and inadvertently freeing a mask that may be actively used
by another client?

> +		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;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=10

[PATCH v2 11/13] dmaengine: ste_dma40: Fix event group bounds

[Linus Walleij]

The dev_type validation can allow values whose derived event group has no
matching physical channel pair. d40_allocate_channel() then indexes
phy_res with j + event_group * 2, and __d40_set_prio_rt() uses the same
group to select priority and realtime registers.

Reject dev_type values outside the hardware event-group range, and stop
the physical-channel search before a partial final channel group can index
past phy_res.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 7c777e410c10..e0c694a1fc8b 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1739,6 +1739,10 @@ static int d40_validate_conf(struct d40_chan *d40c,
 	bool is_log = conf->mode == STEDMA40_MODE_LOGICAL;
 	bool invalid_dev_type = conf->dev_type < 0;
 
+	if (!invalid_dev_type &&
+	    D40_TYPE_TO_GROUP(conf->dev_type) >= D40_GROUP_SIZE / 2)
+		invalid_dev_type = true;
+
 	if (!conf->dir) {
 		chan_err(d40c, "Invalid direction.\n");
 		res = -EINVAL;
@@ -1921,8 +1925,12 @@ static int d40_allocate_channel(struct d40_chan *d40c, bool *first_phy_user)
 				}
 			}
 		} else
-			for (j = 0; j < d40c->base->num_phy_chans; j += 8) {
+			for (j = 0; j < d40c->base->num_phy_chans;
+			     j += D40_GROUP_SIZE) {
 				int phy_num = j  + event_group * 2;
+				if (phy_num + 1 >= num_phy_chans)
+					break;
+
 				for (i = phy_num; i < phy_num + 2; i++) {
 					if (d40_alloc_mask_set(&phys[i],
 							       is_src,
@@ -1942,8 +1950,10 @@ static int d40_allocate_channel(struct d40_chan *d40c, bool *first_phy_user)
 		return -EINVAL;
 
 	/* Find logical channel */
-	for (j = 0; j < d40c->base->num_phy_chans; j += 8) {
+	for (j = 0; j < d40c->base->num_phy_chans; j += D40_GROUP_SIZE) {
 		int phy_num = j + event_group * 2;
+		if (phy_num + 1 >= num_phy_chans)
+			break;
 
 		if (d40c->dma_cfg.use_fixed_channel) {
 			i = d40c->dma_cfg.phy_channel;

-- 
2.55.0

Re: [PATCH v2 11/13] dmaengine: ste_dma40: Fix event group bounds

[sashiko-bot]

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

New issues:
- [High] The patch incorrectly rejects valid device types for event group 4, breaking DMA support for those devices on newer hardware (e.g., DB8540).

Pre-existing issues:
- [High] Logical channel allocation fails for fixed channels located outside the first physical channel block (j=0) due to a premature loop abort.
- [High] Missing bounds check on `phy_channel` when allocating fixed physical mem-to-mem channels allows out-of-bounds array access.
- [High] Error paths in `d40_alloc_chan_resources()` drop a PM runtime reference that was never acquired, leading to an unbalanced refcount underflow.
--

--- Patch [11]: [PATCH v2 11/13] dmaengine: ste_dma40: Fix event group bounds ---
commit 409873531e42fe4afcb7eb90c8da7d95a228b5d7
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Fix event group bounds

The dev_type validation can allow values whose derived event group has no
matching physical channel pair. This patch rejects dev_type values outside
the hardware event-group range and stops physical-channel searches before
indexing past phy_res.

Link: https://lore.kernel.org/dmaengine/20260819225008.5F9651F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 7c777e410c104..e0c694a1fc8b0 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1739,6 +1739,10 @@ static int d40_validate_conf(struct d40_chan *d40c,
>  	bool is_log = conf->mode == STEDMA40_MODE_LOGICAL;
>  	bool invalid_dev_type = conf->dev_type < 0;
>  
> +	if (!invalid_dev_type &&
> +	    D40_TYPE_TO_GROUP(conf->dev_type) >= D40_GROUP_SIZE / 2)
> +		invalid_dev_type = true;

[Severity: High]
Does this check incorrectly reject valid device types for event group 4?

Since D40_GROUP_SIZE is 8, D40_GROUP_SIZE / 2 is 4, which means any dev_type
mapped to event_group >= 4 (i.e. dev_type >= 64) is flagged as invalid.

Newer hardware like DB8540 has 5 event groups (0-4). The new out-of-bounds
safety is correctly handled later in d40_allocate_channel():

if (phy_num + 1 >= num_phy_chans)
        break;

Could this dev_type validation cause a regression by breaking valid logical
channels mapped to event group 4 on these devices?

[ ... ]

> @@ -1942,8 +1950,10 @@ static int d40_allocate_channel(struct d40_chan *d40c, bool *first_phy_user)
>  		return -EINVAL;
>  
>  	/* Find logical channel */
> -	for (j = 0; j < d40c->base->num_phy_chans; j += 8) {
> +	for (j = 0; j < d40c->base->num_phy_chans; j += D40_GROUP_SIZE) {
>  		int phy_num = j + event_group * 2;
> +		if (phy_num + 1 >= num_phy_chans)
> +			break;
>  
>  		if (d40c->dma_cfg.use_fixed_channel) {
>  			i = d40c->dma_cfg.phy_channel;

[Severity: High]
This is a pre-existing issue, but does this loop prematurely abort if a fixed
channel isn't in the first physical block?

Looking at d40_allocate_channel(), when iterating over blocks of physical
channels for logical channels with use_fixed_channel == true, it checks if
the requested phy_channel matches phy_num or phy_num + 1 for the current
group j.

If they do not match, it prints an error and returns -EINVAL immediately:

drivers/dma/ste_dma40.c:d40_allocate_channel() {
    ...
    if (d40c->dma_cfg.use_fixed_channel) {
        i = d40c->dma_cfg.phy_channel;
        if ((i != phy_num) && (i != phy_num + 1)) {
            dev_err(chan2dev(d40c), "invalid fixed phy channel %d\n", i);
            return -EINVAL;
        }
    ...
}

Should this use continue instead of returning -EINVAL to allow checking the
remaining physical channel groups?


While reviewing the channel allocation logic, I noticed two other pre-existing
issues in the surrounding code.

[Severity: High]
This is a pre-existing issue, but can an unvalidated phy_channel lead to an
out-of-bounds array access when configuring fixed physical mem-to-mem channels?

In d40_allocate_channel(), when configuring a physical DMA_MEM_TO_MEM channel
with use_fixed_channel, the driver directly assigns phy_channel and uses
it as an index into the phys array:

drivers/dma/ste_dma40.c:d40_allocate_channel() {
    ...
    if (d40c->dma_cfg.dir == DMA_MEM_TO_MEM) {
        if (d40c->dma_cfg.use_fixed_channel) {
            i = d40c->dma_cfg.phy_channel;
            if (d40_alloc_mask_set(&phys[i], is_src, 0, is_log, first_phy_user))
                goto found_phy;
        }
    ...
}

Since phy_channel is not validated against num_phy_chans in
d40_validate_conf() or locally in this block, could a misconfiguration via
device tree access memory outside the bounds of the phys array?


[Severity: High]
This is a pre-existing issue, but do the error paths in
d40_alloc_chan_resources() drop a PM runtime reference that was never
acquired?

If channel allocation fails, the function jumps to the mark_last_busy label,
which unconditionally calls pm_runtime_put_autosuspend(). However,
pm_runtime_get_sync() is only called after a successful allocation:

drivers/dma/ste_dma40.c:d40_alloc_chan_resources() {
    ...
    err = d40_allocate_channel(d40c, &is_free_phy);
    if (err) {
        chan_err(d40c, "Failed to allocate channel\n");
        d40c->configured = false;
        goto mark_last_busy;
    }

    pm_runtime_get_sync(d40c->base->dev);
    ...
mark_last_busy:
    pm_runtime_put_autosuspend(d40c->base->dev);
    ...
}

Could this lead to a refcount underflow and cause the device to suspend
prematurely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=11

[PATCH v2 12/13] dmaengine: ste_dma40: Validate memcpy configuration

[Linus Walleij]

d40_config_memcpy() builds a default memcpy configuration without passing
it through d40_validate_conf(). A dev_type supplied through the
memcpy-channels device tree property can therefore bypass the bounds
checks added for client configurations.

Validate the generated memcpy configuration before deriving logical
channel registers or allocating the channel.

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

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index e0c694a1fc8b..49bca706b638 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2013,11 +2013,16 @@ static int d40_allocate_channel(struct d40_chan *d40c, bool *first_phy_user)
 static int d40_config_memcpy(struct d40_chan *d40c)
 {
 	dma_cap_mask_t cap = d40c->chan.device->cap_mask;
+	int ret;
 
 	if (dma_has_cap(DMA_MEMCPY, cap) && !dma_has_cap(DMA_SLAVE, cap)) {
 		d40c->dma_cfg = dma40_memcpy_conf_log;
 		d40c->dma_cfg.dev_type = dma40_memcpy_channels[d40c->chan.chan_id];
 
+		ret = d40_validate_conf(d40c, &d40c->dma_cfg);
+		if (ret)
+			return ret;
+
 		d40_log_cfg(&d40c->dma_cfg,
 			    &d40c->log_def.lcsp1, &d40c->log_def.lcsp3);
 
@@ -2025,6 +2030,10 @@ static int d40_config_memcpy(struct d40_chan *d40c)
 		   dma_has_cap(DMA_SLAVE, cap)) {
 		d40c->dma_cfg = dma40_memcpy_conf_phy;
 
+		ret = d40_validate_conf(d40c, &d40c->dma_cfg);
+		if (ret)
+			return ret;
+
 		/* Generate interrupt at end of transfer or relink. */
 		d40c->dst_def_cfg |= BIT(D40_SREG_CFG_TIM_POS);
 

-- 
2.55.0

Re: [PATCH v2 12/13] dmaengine: ste_dma40: Validate memcpy configuration

[sashiko-bot]

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

New issues:
- [High] d40_validate_conf() unconditionally rejects memcpy configurations.

Pre-existing issues:
- [High] Error paths in d40_alloc_chan_resources() underflow PM runtime reference count.
--

--- Patch [12]: [PATCH v2 12/13] dmaengine: ste_dma40: Validate memcpy configuration ---
commit 19f873d8b80c96a75bea7cfedde33c6486eb34af
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Validate memcpy configuration

This patch validates the generated memcpy configuration before deriving
logical channel registers or allocating the channel. This ensures that
a dev_type supplied through the memcpy-channels device tree property
cannot bypass the bounds checks.

Closes: https://lore.kernel.org/dmaengine/20260819225008.5F9651F000E9@smtp.kernel.org/

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index e0c694a1fc8b0..49bca706b638f 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -2013,11 +2013,16 @@ static int d40_allocate_channel(struct d40_chan *d40c, bool *first_phy_user)
>  static int d40_config_memcpy(struct d40_chan *d40c)
>  {
>  	dma_cap_mask_t cap = d40c->chan.device->cap_mask;
> +	int ret;
>  
>  	if (dma_has_cap(DMA_MEMCPY, cap) && !dma_has_cap(DMA_SLAVE, cap)) {
>  		d40c->dma_cfg = dma40_memcpy_conf_log;
>  		d40c->dma_cfg.dev_type = dma40_memcpy_channels[d40c->chan.chan_id];
>  
> +		ret = d40_validate_conf(d40c, &d40c->dma_cfg);
> +		if (ret)
> +			return ret;
> +

[Severity: High]
Does this unconditionally reject memcpy configurations?

In the framework, DMA_MEM_TO_MEM has an integer value of 0. When this calls
d40_validate_conf() for the memcpy configurations (which have a direction of
DMA_MEM_TO_MEM), it hits this check in d40_validate_conf():

drivers/dma/ste_dma40.c:d40_validate_conf() {
	...
	if (!conf->dir) {
		chan_err(d40c, "Invalid direction.\n");
		res = -EINVAL;
	}
	...
}

Because conf->dir is 0, this evaluates to true, resulting in validation always
returning -EINVAL and completely breaking memcpy functionality for this
driver.

[Severity: High]
This is a pre-existing issue, but returning an error here uncovers a
PM runtime reference underflow in the caller.

If d40_config_memcpy() fails, d40_alloc_chan_resources() jumps to its error
handler before the PM runtime reference is acquired:

drivers/dma/ste_dma40.c:d40_alloc_chan_resources() {
	...
	if (!d40c->configured) {
		err = d40_config_memcpy(d40c);
		if (err) {
			chan_err(d40c, "Failed to configure memcpy channel\n");
			goto mark_last_busy;
		}
	}
	...
mark_last_busy:
	pm_runtime_put_autosuspend(d40c->base->dev);
	spin_unlock_irqrestore(&d40c->lock, flags);
	return err;
}

The error handler unconditionally calls pm_runtime_put_autosuspend(),
decrementing the reference count without it having been incremented.

Could this lead to the device being prematurely suspended while actively
in use?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-dma40-fixes-v2-0-63238334c707@kernel.org?part=12

[PATCH v2 13/13] dmaengine: Use unique debugfs names

[Linus Walleij]

DMA40 registers multiple dma_device instances for the same platform
device. The DMAengine debugfs code uses dev_name(dma_dev->dev) as the
directory name, so the second and third registrations try to create the
same directory and print duplicate-name warnings.

Keep the existing device-name directory for the first registration, but
use a dev_id-suffixed fallback when that name already exists.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/dma/dmaengine.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 6ffd8bd82154..a1c388def67a 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -33,6 +33,7 @@
 
 #include <linux/acpi.h>
 #include <linux/acpi_dma.h>
+#include <linux/dcache.h>
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
@@ -70,8 +71,24 @@ static struct dentry *rootdir;
 
 static void dmaengine_debug_register(struct dma_device *dma_dev)
 {
-	dma_dev->dbg_dev_root = debugfs_create_dir(dev_name(dma_dev->dev),
-						   rootdir);
+	const char *name = dev_name(dma_dev->dev);
+	struct dentry *dentry;
+	char *uniq;
+
+	dentry = debugfs_lookup(name, rootdir);
+	if (dentry) {
+		dput(dentry);
+
+		uniq = kasprintf(GFP_KERNEL, "%s.%d", name, dma_dev->dev_id);
+		if (!uniq)
+			return;
+
+		dma_dev->dbg_dev_root = debugfs_create_dir(uniq, rootdir);
+		kfree(uniq);
+	} else {
+		dma_dev->dbg_dev_root = debugfs_create_dir(name, rootdir);
+	}
+
 	if (IS_ERR(dma_dev->dbg_dev_root))
 		dma_dev->dbg_dev_root = NULL;
 }

-- 
2.55.0