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

[PATCHv2] 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).

The ALIGN(sizeof(struct d40_base), 4) requirement is met implicitly by the
C compiler when using a flexible array member. With struct d40_chan
phy_chans[] as the last member, the C standard guarantees
sizeof(struct d40_base) includes trailing padding to satisfy the alignment
of the flexible array element type (struct d40_chan). Since struct d40_chan
contains members like spinlock_t, pointers, and struct dma_chan — all with
alignment ≥ 4 — the compiler ensures sizeof(struct d40_base) is already a
multiple of _Alignof(struct d40_chan) >= 4. The struct_size() macro then
computes sizeof(struct d40_base) + sizeof(struct d40_chan) * num_phy_chans,
so phy_chans[0] lands at a properly aligned offset without needing the manual
ALIGN.

Assisted-by: Claude:Opus-4.7
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: added ALIGN description
 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: [PATCHv2] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Defeating struct_size() saturation arithmetic with unchecked addition can lead to a heap buffer overflow.
--

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

dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

This commit converts the separately-offset phy_chans pointer to a C99 flexible
array member at the end of struct d40_base, switching the allocation to
struct_size(). The log_chans and memcpy_chans slots continue to live in the
same allocation immediately after phy_chans.

> 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 unchecked addition defeat the saturation arithmetic of struct_size()
and lead to a heap buffer overflow?

If struct_size() detects an overflow and saturates to SIZE_MAX, this
unconditional addition will wrap alloc_size around to a small integer value.

As a result, devm_kzalloc() might succeed in allocating an undersized buffer,
causing an out-of-bounds write when the channels are later initialized or
accessed.

Since phy_chans, log_chans, and memcpy_chans all use the same type and are
allocated contiguously, should the counts be summed inside a single
struct_size() call like this:

    alloc_size = struct_size(base, phy_chans,
                             num_phy_chans + num_log_chans + num_memcpy_chans);

Alternatively, the size_add() and size_mul() macros could be used to preserve
the saturation protection.

> +	base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
>  
>  	if (!base)
>  		return -ENOMEM;

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

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

[Rosen Penev]

On Wed, May 27, 2026 at 1:48 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Defeating struct_size() saturation arithmetic with unchecked addition can lead to a heap buffer overflow.
> --
>
> commit e3bf7e1250eb198cd9bf4617a8f0c3d6cc4d796b
> Author: Rosen Penev <rosenp@gmail.com>
>
> dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
>
> This commit converts the separately-offset phy_chans pointer to a C99 flexible
> array member at the end of struct d40_base, switching the allocation to
> struct_size(). The log_chans and memcpy_chans slots continue to live in the
> same allocation immediately after phy_chans.
>
> > 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 unchecked addition defeat the saturation arithmetic of struct_size()
> and lead to a heap buffer overflow?
>
> If struct_size() detects an overflow and saturates to SIZE_MAX, this
> unconditional addition will wrap alloc_size around to a small integer value.
>
> As a result, devm_kzalloc() might succeed in allocating an undersized buffer,
> causing an out-of-bounds write when the channels are later initialized or
> accessed.
>
> Since phy_chans, log_chans, and memcpy_chans all use the same type and are
> allocated contiguously, should the counts be summed inside a single
> struct_size() call like this:
>
>     alloc_size = struct_size(base, phy_chans,
>                              num_phy_chans + num_log_chans + num_memcpy_chans);
>
> Alternatively, the size_add() and size_mul() macros could be used to preserve
> the saturation protection.
This is completely unreachable. I can add min() calls if needed.
>
> > +     base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
> >
> >       if (!base)
> >               return -ENOMEM;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260527203126.7053-1-rosenp@gmail.com?part=1