[PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[Rosen Penev]

Convert the separately-offset phy_chans pointer to a C99 flexible array
member at the end of struct d40_base, and switch the allocation to
struct_size(). The log_chans and memcpy_chans slots continue to live
in the same allocation immediately after phy_chans, indexed via
base->log_chans. This removes the hand-rolled pointer fixup that
recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).

Assisted-by: Claude:Opus-4.7
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ste_dma40.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 9b803c0aec25..d3e3c4cd43f1 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -602,7 +602,6 @@ struct d40_base {
 	struct dma_device		  dma_both;
 	struct dma_device		  dma_slave;
 	struct dma_device		  dma_memcpy;
-	struct d40_chan			 *phy_chans;
 	struct d40_chan			 *log_chans;
 	struct d40_chan			**lookup_log_chans;
 	struct d40_chan			**lookup_phy_chans;
@@ -621,6 +620,7 @@ struct d40_base {
 	u32				 *regs_interrupt;
 	u16				  gcc_pwr_off_mask;
 	struct d40_gen_dmac		  gen_dmac;
+	struct d40_chan			 phy_chans[];
 };
 
 static struct device *chan2dev(struct d40_chan *d40c)
@@ -3128,6 +3128,7 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 	struct clk *clk;
 	void __iomem *virtbase;
 	struct d40_base *base;
+	size_t alloc_size;
 	int num_log_chans;
 	int num_phy_chans;
 	int num_memcpy_chans;
@@ -3197,10 +3198,9 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 		 "hardware rev: %d with %d physical and %d logical channels\n",
 		 rev, num_phy_chans, num_log_chans);
 
-	base = devm_kzalloc(dev,
-		ALIGN(sizeof(struct d40_base), 4) +
-		(num_phy_chans + num_log_chans + num_memcpy_chans) *
-		sizeof(struct d40_chan), GFP_KERNEL);
+	alloc_size = struct_size(base, phy_chans, num_phy_chans);
+	alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
+	base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
 
 	if (!base)
 		return -ENOMEM;
@@ -3213,7 +3213,6 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 	base->virtbase = virtbase;
 	base->plat_data = plat_data;
 	base->dev = dev;
-	base->phy_chans = ((void *)base) + ALIGN(sizeof(struct d40_base), 4);
 	base->log_chans = &base->phy_chans[num_phy_chans];
 
 	if (base->plat_data->num_of_phy_chans == 14) {
-- 
2.54.0

Re: [PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[sashiko-bot]

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

New issues:
- [High] Adding directly to the saturated result of `struct_size()` bypasses integer overflow protection.

Pre-existing issues:
- [High] Missing `free_irq()` on the error path in `d40_probe()` leads to a resource leak and use-after-free.
--

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

dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

Convert the separately-offset phy_chans pointer to a C99 flexible array
member at the end of struct d40_base, and switch the allocation to
struct_size().

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 9b803c0aec254..d3e3c4cd43f17 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c

[ ... ]

> @@ -3197,10 +3198,9 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
>  		 "hardware rev: %d with %d physical and %d logical channels\n",
>  		 rev, num_phy_chans, num_log_chans);
>  
> -	base = devm_kzalloc(dev,
> -		ALIGN(sizeof(struct d40_base), 4) +
> -		(num_phy_chans + num_log_chans + num_memcpy_chans) *
> -		sizeof(struct d40_chan), GFP_KERNEL);
> +	alloc_size = struct_size(base, phy_chans, num_phy_chans);
> +	alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);

[Severity: High]
Can this unprotected addition bypass the integer overflow protection provided
by struct_size()?

If struct_size() saturates and returns SIZE_MAX, the subsequent addition
will wrap around to a small value. This could allow devm_kzalloc() to
successfully allocate an undersized buffer, which might result in a heap
buffer overflow when the channel arrays are written out of bounds. The
manual multiplication also appears to lack overflow protection.

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential resource leak and
use-after-free in d40_probe().

If the probe function fails after requesting the interrupt (for example,
during regulator_get() or d40_dmaengine_init()) and jumps to the
destroy_cache label, it appears to return an error without freeing the
requested interrupt.

Because the base structure is automatically freed by the devres framework
upon probe failure, if an interrupt triggers subsequently, could the
interrupt handler d40_handle_interrupt() execute and access the freed
base structure?

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

Re: [PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[Linus Walleij]

Hi Rosen,

thanks for your patch!

On Tue, May 26, 2026 at 10:16 PM Rosen Penev <rosenp@gmail.com> wrote:

> Convert the separately-offset phy_chans pointer to a C99 flexible array
> member at the end of struct d40_base, and switch the allocation to
> struct_size(). The log_chans and memcpy_chans slots continue to live
> in the same allocation immediately after phy_chans, indexed via
> base->log_chans. This removes the hand-rolled pointer fixup that
> recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).
>
> Assisted-by: Claude:Opus-4.7
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

OK!

Please add

unsigned int num_phy_chans

> +       struct d40_chan                  phy_chans[];

and

phy_chans[] __counted_by(num_phy_chans);


> -       base = devm_kzalloc(dev,
> -               ALIGN(sizeof(struct d40_base), 4) +
> -               (num_phy_chans + num_log_chans + num_memcpy_chans) *
> -               sizeof(struct d40_chan), GFP_KERNEL);
> +       alloc_size = struct_size(base, phy_chans, num_phy_chans);
> +       alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
> +       base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);

Please describe exactly how the ALIGN(sizeof(struct d40_base), 4) requirement
is met by the new code?

The phy_chans will be read by hardware which depends on this specific
alignment otherwise the data will be corrupted.

Yours,
Linus Walleij

Re: [PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[Rosen Penev]

On Wed, May 27, 2026 at 3:14 AM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Rosen,
>
> thanks for your patch!
>
> On Tue, May 26, 2026 at 10:16 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> > Convert the separately-offset phy_chans pointer to a C99 flexible array
> > member at the end of struct d40_base, and switch the allocation to
> > struct_size(). The log_chans and memcpy_chans slots continue to live
> > in the same allocation immediately after phy_chans, indexed via
> > base->log_chans. This removes the hand-rolled pointer fixup that
> > recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).
> >
> > Assisted-by: Claude:Opus-4.7
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> OK!
>
> Please add
>
> unsigned int num_phy_chans
>
> > +       struct d40_chan                  phy_chans[];
>
> and
>
> phy_chans[] __counted_by(num_phy_chans);
Not possible here. The allocation allocates space for both phy_chans
and log_chans. To do this I would need to split up allocations into
two. Not a fan of that as two kfrees and two allocs would be needed.
>
>
> > -       base = devm_kzalloc(dev,
> > -               ALIGN(sizeof(struct d40_base), 4) +
> > -               (num_phy_chans + num_log_chans + num_memcpy_chans) *
> > -               sizeof(struct d40_chan), GFP_KERNEL);
> > +       alloc_size = struct_size(base, phy_chans, num_phy_chans);
> > +       alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
> > +       base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
>
> Please describe exactly how the ALIGN(sizeof(struct d40_base), 4) requirement
> is met by the new code?
Will do.
>
> The phy_chans will be read by hardware which depends on this specific
> alignment otherwise the data will be corrupted.
>
> Yours,
> Linus Walleij