Re: [PATCH v17 07/10] LoongArch: 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] Concurrent memory hotplug can modify `memblock.memory` during `kexec_file_load()`, causing an out-of-bounds write on `cmem->ranges[]` and use-after-free on `memblock.memory.regions`.
--

commit 144c95098f9114fc6eb269028a7cf5e5b2d79839
Author: Jinjie Ruan <ruanjinjie@huawei.com>

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

This commit simplifies the kexec_file code by replacing the existing
prepare_elf_headers() with the generic crash_prepare_headers() helper.
It implements the architecture-specific arch_get_system_nr_ranges()
and arch_crash_populate_cmem() functions using for_each_mem_range.

> diff --git a/arch/loongarch/kernel/machine_kexec_file.c b/arch/loongarch/kernel/machine_kexec_file.c
> --- a/arch/loongarch/kernel/machine_kexec_file.c
> +++ b/arch/loongarch/kernel/machine_kexec_file.c

[ ... ]

> -static int prepare_elf_headers(void **addr, unsigned long *sz)
> +unsigned int arch_get_system_nr_ranges(void)
>  {
> -	int ret, nr_ranges;
> -	uint64_t i;
> +	int nr_ranges = 2; /* for exclusion of crashkernel region */
>  	phys_addr_t start, end;
> -	struct crash_mem *cmem;
> +	uint64_t 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;
> +	uint64_t 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 is a pre-existing issue, and was not introduced by the patch under
review, but could this loop overflow cmem->ranges[]?

The crash_prepare_headers() function first counts the memblock memory ranges
via arch_get_system_nr_ranges() to allocate the cmem array, and then
populates it using arch_crash_populate_cmem().

Neither of these functions appears to hold the mem_hotplug_lock via
get_online_mems(). If a concurrent memory hotplug event adds a new memory
range via memblock_add_node() between the counting and population steps,
does the loop here blindly increment cmem->nr_ranges past cmem->max_nr_ranges
and cause a heap out-of-bounds write?

Furthermore, if the hotplug event triggers memblock_double_array(), the
underlying memblock.memory.regions array might be reallocated and freed.
Could this lead to a use-after-free when for_each_mem_range() iterates
over the stale array pointer?

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