Re: [PATCH v2 4/8] of: reserved_mem: save /memreserve/ entries into the reserved_mem array

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds write to the statically allocated `reserved_mem_array` due to inflated `total_reserved_mem_cnt` without guaranteed array reallocation.
--

commit cac834a48f61205ebaa2a6eaf1bc2f735c0a6c46
Author: Wandun Chen <chenwandun1@gmail.com>

of: reserved_mem: save /memreserve/ entries into the reserved_mem array

This commit saves /memreserve/ entries into the reserved_mem array so they
can be excluded from kdump vmcores. Overlapping entries are marked as dumpable
to ensure useful crash analysis data is retained.

> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 82f7327c59ea9..d2bcaf149fe81 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
[ ... ]
> @@ -516,7 +517,9 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  		if (!size)
>  			break;
>  		memblock_reserve(base, size);
> +		nr_memreserve++;
>  	}
> +	fdt_reserved_mem_account_memreserve(nr_memreserve);
>  }
[ ... ]
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 313cbc57aa453..6dfe9e03c535d 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
[ ... ]
> @@ -241,6 +241,43 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +static void __init fdt_reserved_mem_add_memreserve(phys_addr_t base,
> +						   phys_addr_t size)
> +{
> +	struct reserved_mem *rmem;
> +	bool dumpable = false;
> +	int i;
> +
> +	if (reserved_mem_count == total_reserved_mem_cnt) {
> +		pr_err("not enough space for memreserve regions.\n");
> +		return;
> +	}

[Severity: High]
Could this lead to an out-of-bounds write to the statically allocated
reserved_mem_array during early boot?

fdt_reserved_mem_account_memreserve() increments total_reserved_mem_cnt
by the number of /memreserve/ entries. However, if the device tree lacks
a /reserved-memory node, fdt_scan_reserved_mem_late() returns early and
alloc_reserved_mem_array() is bypassed.

If this happens, the reserved_mem pointer continues to point to the
statically allocated array with a fixed capacity of MAX_RESERVED_REGIONS.

Since the check above compares against the inflated total_reserved_mem_cnt, it
appears we could write past the end of the static array if the device tree
contains 64 or more /memreserve/ entries?

> +
> +	for (i = 0; i < reserved_mem_count; i++) {
> +		rmem = &reserved_mem[i];
> +
> +		if (!rmem->dumpable)
> +			continue;
> +
> +		if (base < rmem->base + rmem->size && rmem->base < base + size) {
> +			dumpable = true;
> +			break;
> +		}
> +	}
> +
> +	rmem = &reserved_mem[reserved_mem_count];
> +	rmem->base = base;
> +	rmem->size = size;
> +	rmem->dumpable = dumpable;
> +
> +	reserved_mem_count++;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520091844.592753-1-chenwandun@lixiang.com?part=4