Re: [PATCH v16 06/10] riscv: 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] A Time-of-Check to Time-of-Use (TOCTOU) race condition in kexec segment preparation can cause a heap buffer overflow if memory hotplug occurs.
--

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

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

This patch simplifies the kexec_file code for RISC-V by using the newly
introduced crash_prepare_headers() helper. It replaces the architecture
specific prepare_elf_headers() with arch_get_system_nr_ranges() and
arch_crash_populate_cmem() to reduce code duplication.

> diff --git a/arch/riscv/kernel/machine_kexec_file.c b/arch/riscv/kernel/machine_kexec_file.c
> index 3f7766057cac3..439cbc50dfa69 100644
> --- a/arch/riscv/kernel/machine_kexec_file.c
> +++ b/arch/riscv/kernel/machine_kexec_file.c
[ ... ]
> @@ -55,41 +64,9 @@ static int prepare_elf64_ram_headers_callback(struct resource *res, void *arg)
>  	return 0;
>  }
>  
> -static int prepare_elf_headers(void **addr, unsigned long *sz)
> +int arch_crash_populate_cmem(struct crash_mem *cmem)
>  {
> -	struct crash_mem *cmem;
> -	unsigned int nr_ranges;
> -	int ret;
> -
> -	nr_ranges = 2; /* For exclusion of crashkernel region */
> -	walk_system_ram_res(0, -1, &nr_ranges, get_nr_ram_ranges_callback);
> -
> -	cmem = kmalloc_flex(*cmem, ranges, nr_ranges);
> -	if (!cmem)
> -		return -ENOMEM;
> -
> -	cmem->max_nr_ranges = nr_ranges;
> -	cmem->nr_ranges = 0;
> -	ret = walk_system_ram_res(0, -1, cmem, prepare_elf64_ram_headers_callback);
> -	if (ret)
> -		goto out;
> -
[ ... ]
> +	return walk_system_ram_res(0, -1, cmem, prepare_elf64_ram_headers_callback);
>  }

[Severity: High]
This is a pre-existing issue, but can this code suffer from a Time-of-Check
to Time-of-Use race condition during memory hotplug?

The crash_prepare_headers() helper in kernel/crash_core.c uses a lockless
two-pass sequence:

kernel/crash_core.c:crash_prepare_headers() {
    ...
    max_nr_ranges = arch_get_system_nr_ranges();
    ...
    cmem = alloc_cmem(max_nr_ranges);
    ...
    ret = arch_crash_populate_cmem(cmem);
    ...
}

Since the kexec syscall does not hold mem_hotplug_lock or use
get_online_mems(), could a concurrent memory hotplug event call add_memory()
and insert a new resource into the system RAM resource tree between the two
passes?

If that happens, the second walk_system_ram_res() pass in
arch_crash_populate_cmem() would encounter the newly inserted memory range.
This would cause it to execute prepare_elf64_ram_headers_callback() more
times than initially counted.

Because prepare_elf64_ram_headers_callback() unconditionally writes to
cmem->ranges[cmem->nr_ranges] without verifying that
cmem->nr_ranges < cmem->max_nr_ranges, would this result in an out-of-bounds
write that corrupts adjacent heap memory?

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