Re: [PATCH v16 04/10] arm64: kexec_file: Use crash_prepare_headers() helper to simplify code

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless memblock iteration in `arch_get_system_nr_ranges()` and `arch_crash_populate_cmem()` can lead to TOCTOU buffer overflow and Use-After-Free due to concurrent memory hotplug.
--

commit 3445143d0dcc2150e57d4b72a58250e2bbaf7ea7
Author: Jinjie Ruan <ruanjinjie@huawei.com>

arm64: kexec_file: Use crash_prepare_headers() helper to simplify code

This commit replaces the existing prepare_elf_headers() with the newly
introduced crash_prepare_headers() function. It implements the two required
architecture functions, arch_get_system_nr_ranges() and
arch_crash_populate_cmem(), to traverse and populate memory ranges.

> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index e31fabed378a5..b019b31df48cb 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
[ ... ]
> -static int prepare_elf_headers(void **addr, unsigned long *sz)
> +unsigned int arch_get_system_nr_ranges(void)
>  {
> -	struct crash_mem *cmem;
> -	unsigned int nr_ranges;
> -	int ret;
> -	u64 i;
> +	unsigned int nr_ranges = 2; /* for exclusion of crashkernel region */
>  	phys_addr_t start, end;
> +	u64 i;
>  
> -	nr_ranges = 2; /* for exclusion of crashkernel region */
>  	for_each_mem_range(i, &start, &end)
>  		nr_ranges++;
>  
> -	cmem = kmalloc_flex(*cmem, ranges, nr_ranges);
> -	if (!cmem)
> -		return -ENOMEM;
> +	return nr_ranges;
> +}
> +
> +int arch_crash_populate_cmem(struct crash_mem *cmem)
> +{
> +	phys_addr_t start, end;
> +	u64 i;
>  
> -	cmem->max_nr_ranges = nr_ranges;
> -	cmem->nr_ranges = 0;
>  	for_each_mem_range(i, &start, &end) {
>  		cmem->ranges[cmem->nr_ranges].start = start;
>  		cmem->ranges[cmem->nr_ranges].end = end - 1;
>  		cmem->nr_ranges++;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but could this lockless memblock
iteration lead to a TOCTOU buffer overflow or use-after-free during
concurrent memory hotplug?

arch_get_system_nr_ranges() locklessly calculates the max_nr_ranges used to
allocate cmem. If memory is hot-plugged between this calculation and the
array population here in arch_crash_populate_cmem(), could the second loop
encounter more regions than initially allocated? 

Because there is no bounds check against cmem->max_nr_ranges, could this
result in an out-of-bounds write past the flexible cmem->ranges array?

Additionally, memory hotplug calls memblock_add_node(), which can invoke
memblock_double_array(). This function reallocates memblock.memory.regions
and frees the old array. If this reallocation occurs while
for_each_mem_range() is iterating in either function, could the lockless
iteration dereference a freed regions array?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608073459.3119290-1-ruanjinjie@huawei.com?part=4