[PATCH] dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

[PATCH] dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

[Rosen Penev]

D40_MEMCPY_MAX_CHANS is defined as 8, but the dma40_memcpy_channels[]
array only has 6 elements. This mismatch causes an out-of-bounds
issue:

1. d40_of_probe() accepts up to 8 memcpy channels from DT
   (num_memcpy > D40_MEMCPY_MAX_CHANS allows 7-8), then writes them
   into the 6-element dma40_memcpy_channels[], corrupting adjacent
   stack memory.

Fix by defining D40_MEMCPY_MAX_CHANS as 6 to match the array size.

Fixes: a7dacb68b35a ("dmaengine: ste_dma40: Allow memcpy channels to be configured from DT")
Assisted-by: Opencode:Big-Pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ste_dma40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..c45643b7f415 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -86,7 +86,7 @@ struct stedma40_platform_data {
 #define D40_ALLOC_PHY		BIT(30)
 #define D40_ALLOC_LOG_FREE	0
 
-#define D40_MEMCPY_MAX_CHANS	8
+#define D40_MEMCPY_MAX_CHANS	6
 
 /* Reserved event lines for memcpy only. */
 #define DB8500_DMA_MEMCPY_EV_0	51
-- 
2.54.0

Re: [PATCH] dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

[sashiko-bot]

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

New issues:
- [Low] The commit message incorrectly claims that the out-of-bounds write corrupts 'adjacent stack memory'.

Pre-existing issues:
- [High] Missing `free_irq()` and `tasklet_kill()` in the `d40_probe()` error path (`destroy_cache`) leads to a severe Use-After-Free and IRQ leak.
--

commit d80dea89461b0f1c87cf7d871360b4cb6985bcdb
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

The commit fixes an out-of-bounds array access in the ste_dma40 driver by
correcting D40_MEMCPY_MAX_CHANS to match the actual size of the
dma40_memcpy_channels array.

> 1. d40_of_probe() accepts up to 8 memcpy channels from DT
>    (num_memcpy > D40_MEMCPY_MAX_CHANS allows 7-8), then writes them
>    into the 6-element dma40_memcpy_channels[], corrupting adjacent
>    stack memory.

[Severity: Low]
Is the description of "stack memory" corruption accurate here?

Looking at dma40_memcpy_channels, it is defined as a static global array,
meaning the out-of-bounds write would corrupt the .data or .bss segments
rather than adjacent stack memory.

Could the commit message be updated to reflect that this is global memory
corruption rather than stack corruption?


> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 9b803c0aec254..9dfacffa0548f 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -86,7 +86,7 @@ struct stedma40_platform_data {
>  #define D40_ALLOC_PHY		BIT(30)
>  #define D40_ALLOC_LOG_FREE	0
>  
> -#define D40_MEMCPY_MAX_CHANS	8
> +#define D40_MEMCPY_MAX_CHANS	6

[Severity: High]
This is a pre-existing issue, but I noticed a missing cleanup step in the
probe function's error handling. Does the destroy_cache error path in
d40_probe() leak the IRQ and fail to kill tasklets?

drivers/dma/ste_dma40.c:d40_probe() {
    ...
	ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
	if (ret) {
		d40_err(dev, "No IRQ defined\n");
		goto destroy_cache;
	}
    ...
	ret = of_dma_controller_register(np, d40_xlate, NULL);
	if (ret) {
		dev_err(dev,
			"could not register of_dma_controller\n");
		goto destroy_cache;
	}
    ...
 destroy_cache:
	if (base->lcla_pool.dma_addr)
		dma_unmap_single(base->dev, base->lcla_pool.dma_addr,
    ...
}

Because request_irq() is used instead of a devm_ variant, shouldn't free_irq()
and tasklet_kill() be called before returning?

If probe fails late, the devm_kzalloc() allocated base structure is
automatically freed. If the leaked IRQ fires or a scheduled tasklet executes
afterwards, they will access the freed memory, leading to a Use-After-Free.

>  
>  /* Reserved event lines for memcpy only. */
>  #define DB8500_DMA_MEMCPY_EV_0	51

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

Re: [PATCH] dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

[Linus Walleij]

On Sun, May 31, 2026 at 11:08 PM Rosen Penev <rosenp@gmail.com> wrote:

> D40_MEMCPY_MAX_CHANS is defined as 8, but the dma40_memcpy_channels[]
> array only has 6 elements. This mismatch causes an out-of-bounds
> issue:
>
> 1. d40_of_probe() accepts up to 8 memcpy channels from DT
>    (num_memcpy > D40_MEMCPY_MAX_CHANS allows 7-8), then writes them
>    into the 6-element dma40_memcpy_channels[], corrupting adjacent
>    stack memory.
>
> Fix by defining D40_MEMCPY_MAX_CHANS as 6 to match the array size.
>
> Fixes: a7dacb68b35a ("dmaengine: ste_dma40: Allow memcpy channels to be configured from DT")
> Assisted-by: Opencode:Big-Pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Excellent find Rosen!
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH] dmaengine: ste_dma40: fix out-of-bounds access from D40_MEMCPY_MAX_CHANS

[Rosen Penev]

On Tue, Jun 2, 2026 at 1:48 AM Linus Walleij <linusw@kernel.org> wrote:
>
> On Sun, May 31, 2026 at 11:08 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> > D40_MEMCPY_MAX_CHANS is defined as 8, but the dma40_memcpy_channels[]
> > array only has 6 elements. This mismatch causes an out-of-bounds
> > issue:
> >
> > 1. d40_of_probe() accepts up to 8 memcpy channels from DT
> >    (num_memcpy > D40_MEMCPY_MAX_CHANS allows 7-8), then writes them
> >    into the 6-element dma40_memcpy_channels[], corrupting adjacent
> >    stack memory.
> >
> > Fix by defining D40_MEMCPY_MAX_CHANS as 6 to match the array size.
> >
> > Fixes: a7dacb68b35a ("dmaengine: ste_dma40: Allow memcpy channels to be configured from DT")
> > Assisted-by: Opencode:Big-Pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> Excellent find Rosen!
All credit is to our robot overl-i mean sashiko. I'm under the
impression nothing can get merged unless it's happy.
> Reviewed-by: Linus Walleij <linusw@kernel.org>
>
> Yours,
> Linus Walleij