Re: [PATCH v16 05/10] x86: 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 with concurrent memory hotplug can cause an out-of-bounds array write in `prepare_elf64_ram_headers_callback()` due to a missing bounds check.
--

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

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

This commit replaces x86's custom prepare_elf_headers() with the generic
crash_prepare_headers() helper. It implements the required architecture
callbacks arch_get_system_nr_ranges(), arch_crash_populate_cmem(), and
arch_crash_exclude_ranges() to manage crash memory regions.

> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index cd796818d94d9..cb2ce7acfec45 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c

[ ... ]

> -/* Prepare elf headers. Return addr and size */
> -static int prepare_elf_headers(void **addr, unsigned long *sz,
> -			       unsigned long *nr_mem_ranges)
> +int arch_crash_populate_cmem(struct crash_mem *cmem)
>  {
> -	struct crash_mem *cmem;
> -	int ret;
> -
> -	cmem = fill_up_crash_elf_data();
> -	if (!cmem)
> -		return -ENOMEM;
> -
> -	ret = walk_system_ram_res(0, -1, cmem, prepare_elf64_ram_headers_callback);
> -	if (ret)
> -		goto out;
> -
> -	/* Exclude unwanted mem ranges */
> -	ret = elf_header_exclude_ranges(cmem);
> -	if (ret)
> -		goto out;
> -
> -	/* Return the computed number of memory ranges, for hotplug usage */
> -	*nr_mem_ranges = cmem->nr_ranges;
> -
> -	/* By default prepare 64bit headers */
> -	ret = crash_prepare_elf64_headers(cmem, IS_ENABLED(CONFIG_X86_64), addr, sz);
> -
> -out:
> -	vfree(cmem);
> -	return ret;
> +	return walk_system_ram_res(0, -1, cmem, prepare_elf64_ram_headers_callback);
>  }

[Severity: High]
This is a pre-existing issue, but can this sequence lead to an out-of-bounds 
write during concurrent memory hotplug?

The generic crash_prepare_headers() performs two lockless passes over system
RAM resources:

1. arch_get_system_nr_ranges() counts the regions to allocate cmem.
2. arch_crash_populate_cmem() populates them using walk_system_ram_res()
   and prepare_elf64_ram_headers_callback().

Since sys_kexec_file_load() does not hold mem_hotplug_lock, and 
walk_system_ram_res() temporarily drops resource_lock between iterations,
concurrent memory hot-add operations can insert new memory regions between or
during these passes.

If the number of dynamically added RAM regions exceeds the reserved padding, 
the second pass will encounter more regions than cmem->max_nr_ranges.

prepare_elf64_ram_headers_callback() currently does not verify bounds before
appending new regions:

    cmem->ranges[cmem->nr_ranges].start = res->start;
    cmem->ranges[cmem->nr_ranges].end = res->end;
    cmem->nr_ranges++;

Could this allow an out-of-bounds array write on the vmalloc heap if enough
memory ranges are hotplugged concurrently? Would it make sense to add a bounds
check to prepare_elf64_ram_headers_callback() to prevent exceeding 
cmem->max_nr_ranges, or perhaps hold the memory hotplug lock across the passes?

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