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

[sashiko-bot@kernel.org]

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