Re: [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds write on the static `reserved_mem_array` if dynamic allocation of the reserved memory array fails.
--

commit 716aa35cecc2a52f29b8fd8fd9a0aae87756ea2e
Author: Wandun Chen <chenwandun@lixiang.com>

of: reserved_mem: add config to extend dynamic reserved memory regions
    
Nowadays, the dynamic reserved memory regions is 64 by default, If
the count of reserved memory regions defined in DTS bigger than 64,
only 64 reserved memory can be handled properly.

So add a config to configure the actual dynamic reserved memory
regions count instead of modify the code.

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 056c4179f549c..73d7d7865518e 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -343,7 +343,7 @@ int __init fdt_scan_reserved_mem(void)
>  			count++;
>  
>  		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
> -			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
> +			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d), try expanding CONFIG_OF_RESERVED_MEM_DYNAMIC_REGIONS.\n",
>  				    MAX_RESERVED_REGIONS);
>  			continue;
>  		}

[Severity: High]
This is a pre-existing issue, but there appears to be an out-of-bounds
write on the static reserved_mem_array if dynamic allocation fails.

In fdt_scan_reserved_mem(), the total count of reserved memory regions (and
thus total_reserved_mem_cnt) can exceed MAX_RESERVED_REGIONS because static
regions are not capped by the dynamic_nodes_cnt >= MAX_RESERVED_REGIONS check.

Later, alloc_reserved_mem_array() allocates a dynamically sized array of
total_reserved_mem_cnt elements. If memblock_alloc() fails, it returns early
without resetting total_reserved_mem_cnt to MAX_RESERVED_REGIONS:

drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
	new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
	if (!new_array) {
		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -ENOMEM);
		return;
	}
...
}

When fdt_scan_reserved_mem_late() subsequently initializes the static regions,
it calls fdt_init_reserved_mem_node() for each. The bounds check relies on
the uncapped total_reserved_mem_cnt:

drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];

	if (reserved_mem_count == total_reserved_mem_cnt) {
		pr_err("not enough space for all defined regions.\n");
		return;
	}
...
}

Since total_reserved_mem_cnt wasn't reset on failure, the check allows
reserved_mem_count to exceed MAX_RESERVED_REGIONS. Does this result in
out-of-bounds writes to the static reserved_mem_array?

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