Re: [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added bounds check for `dynamic_nodes_cnt` is placed outside the block that evaluates whether a node is actually a dynamically-placed region, leading to spurious error logs.
--

commit 6a79f526de5e1185b7c79fb469545f4094fb402f
Author: Wandun Chen <chenwandun@lixiang.com>

of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()

This commit adds bounds checking on dynamic_nodes_cnt to prevent a
potential stack buffer overflow in fdt_scan_reserved_mem().

Link: https://sashiko.dev/#/patchset/20260604015332.3669384-1-chenwandun1%40gmail.com?part=1 [1]

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c

[ ... ]

> @@ -341,6 +341,12 @@ int __init fdt_scan_reserved_mem(void)
>  		err = __reserved_mem_reserve_reg(child, uname);
>  		if (!err)
>  			count++;
> +
> +		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
> +			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
> +				    MAX_RESERVED_REGIONS);
> +			continue;
> +		}

[Severity: Medium]
Will this bounds check trigger spurious error logs for statically-placed
regions?

Because the check is placed outside the block that evaluates whether a node
is actually a dynamically-placed region, it evaluates for all subsequent nodes
once the limit is reached.

If the number of dynamically-placed regions exactly reaches
MAX_RESERVED_REGIONS, and a statically-placed region is processed
afterwards, won't it incorrectly trigger the warning claiming the dynamic
nodes limit was exceeded?

>  		/*
>  		 * Save the nodes for the dynamically-placed regions
>  		 * into an array which will be used for allocation right
>  		 * after all the statically-placed regions are reserved
>  		 * or marked as no-map. This is done to avoid dynamically
>  		 * allocating from one of the statically-placed regions.
>  		 */
>  		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
>  			dynamic_nodes[dynamic_nodes_cnt] = child;
>  			dynamic_nodes_cnt++;
>  		}

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