* [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
@ 2026-05-31 2:08 Rosen Penev
2026-05-31 2:21 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Rosen Penev @ 2026-05-31 2:08 UTC (permalink / raw)
To: dmaengine
Cc: Linus Walleij, Vinod Koul, Frank Li,
moderated list:ARM/NOMADIK/Ux500 ARCHITECTURES, open list
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>
---
v3: add min() calls
v2: added ALIGN description
drivers/dma/ste_dma40.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 9b803c0aec25..0d9ffa3e2663 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;
@@ -3185,22 +3186,24 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
else
num_phy_chans = 4 * (readl(virtbase + D40_DREG_ICFG) & 0x7) + 4;
+ num_phy_chans = min(num_phy_chans, STEDMA40_MAX_PHYS);
+
/* The number of channels used for memcpy */
if (plat_data->num_of_memcpy_chans)
num_memcpy_chans = plat_data->num_of_memcpy_chans;
else
num_memcpy_chans = ARRAY_SIZE(dma40_memcpy_channels);
+ num_memcpy_chans = min(num_memcpy_chans, D40_MEMCPY_MAX_CHANS);
num_log_chans = num_phy_chans * D40_MAX_LOG_CHAN_PER_PHY;
dev_info(dev,
"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 +3216,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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
2026-05-31 2:08 [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array Rosen Penev
@ 2026-05-31 2:21 ` sashiko-bot
2026-06-02 9:00 ` Linus Walleij
2026-06-08 12:12 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-05-31 2:21 UTC (permalink / raw)
To: Rosen Penev; +Cc: Frank.Li, vkoul, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds read and write on `dma40_memcpy_channels` due to incorrect array size and clamping limit.
--
commit f80ed233c1567c0d1ee4933ba428492ac35dc443
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..0d9ffa3e26639 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -3185,22 +3186,24 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
> else
> num_phy_chans = 4 * (readl(virtbase + D40_DREG_ICFG) & 0x7) + 4;
>
> + num_phy_chans = min(num_phy_chans, STEDMA40_MAX_PHYS);
> +
> /* The number of channels used for memcpy */
> if (plat_data->num_of_memcpy_chans)
> num_memcpy_chans = plat_data->num_of_memcpy_chans;
> else
> num_memcpy_chans = ARRAY_SIZE(dma40_memcpy_channels);
>
> + num_memcpy_chans = min(num_memcpy_chans, D40_MEMCPY_MAX_CHANS);
[Severity: High]
This isn't a bug introduced by this patch, but does this clamping limit allow
out-of-bounds accesses on the dma40_memcpy_channels array?
The dma40_memcpy_channels array has exactly 6 elements, but
D40_MEMCPY_MAX_CHANS is 8.
When d40_of_probe() is called, it permits num_memcpy up to 8 and writes
them into the array:
drivers/dma/ste_dma40.c:d40_of_probe() {
...
of_property_read_u32_array(np, "memcpy-channels",
dma40_memcpy_channels,
num_memcpy);
...
}
Can this cause an out-of-bounds write corrupting adjacent memory?
Additionally, because num_memcpy_chans is clamped to 8 here in
d40_hw_detect_init(), it allows up to 8 memcpy channels to be registered.
When clients allocate these channels, d40_config_memcpy() indexes the array:
drivers/dma/ste_dma40.c:d40_config_memcpy() {
...
d40c->dma_cfg.dev_type = dma40_memcpy_channels[d40c->chan.chan_id];
...
}
If chan_id is 6 or 7, won't this result in an out-of-bounds read?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260531020843.594892-1-rosenp@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
2026-05-31 2:08 [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array Rosen Penev
2026-05-31 2:21 ` sashiko-bot
@ 2026-06-02 9:00 ` Linus Walleij
2026-06-08 12:12 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2026-06-02 9:00 UTC (permalink / raw)
To: Rosen Penev
Cc: dmaengine, Vinod Koul, Frank Li,
moderated list:ARM/NOMADIK/Ux500 ARCHITECTURES, open list
On Sun, May 31, 2026 at 4:09 AM 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).
>
> 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>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
2026-05-31 2:08 [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array Rosen Penev
2026-05-31 2:21 ` sashiko-bot
2026-06-02 9:00 ` Linus Walleij
@ 2026-06-08 12:12 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2026-06-08 12:12 UTC (permalink / raw)
To: dmaengine, Rosen Penev
Cc: Linus Walleij, Frank Li, linux-arm-kernel, linux-kernel
On Sat, 30 May 2026 19:08:43 -0700, Rosen Penev 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).
>
> [...]
Applied, thanks!
[1/1] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array
commit: cc4fea19daeb0460fe3569e0a2d523f427b2bac1
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-08 12:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 2:08 [PATCHv3] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array Rosen Penev
2026-05-31 2:21 ` sashiko-bot
2026-06-02 9:00 ` Linus Walleij
2026-06-08 12:12 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox