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

[Wandun <chenwandun1@gmail.com>]

On 5/20/26 18:37, sashiko-bot@kernel.org wrote:
> 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.
Yes, there is indeed an out-of-bounds issue if the device tree lacks a
/reserved-memory node.

At the same time, it will also cause inaccurate counting for
total_reserved_mem_cnt.

Will fix both in v3.
>
> 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++;
>> +}
> [ ... ]
>