Re: [PATCH v17 05/10] x86/crash: 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 TOCTOU race condition with concurrent memory hotplug events can cause an out-of-bounds array write in `prepare_elf64_ram_headers_callback()`.
--

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

x86/crash: Use crash_prepare_headers() helper to simplify code
    
This commit simplifies the x86 crash code by replacing the existing
prepare_elf_headers() with the newly introduced crash_prepare_headers()
helper function. It implements the three required architecture-specific
callbacks to pre-count, collect, and exclude memory ranges.

> 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

[ ... ]

> @@ -177,49 +169,16 @@ static struct crash_mem *fill_up_crash_elf_data(void)
>  	 * But in order to lest the low 1M could be changed in the future,
>  	 * (e.g. [start, 1M]), add a extra slot.
>  	 */
> -	nr_ranges += 3 + crashk_cma_cnt;
> -	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> -	if (!cmem)
> -		return NULL;
> -
> -	cmem->max_nr_ranges = nr_ranges;
> +	unsigned int nr_ranges = 3 + crashk_cma_cnt;
>  
> -	return cmem;
> +	walk_system_ram_res(0, -1, &nr_ranges, get_nr_ram_ranges_callback);
> +	return nr_ranges;
>  }

[ ... ]

> @@ -233,35 +192,9 @@ static int prepare_elf64_ram_headers_callback(struct resource *res, void *arg)
>  	return 0;
>  }
>  
> -/* 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)
>  {
> +	return walk_system_ram_res(0, -1, cmem, prepare_elf64_ram_headers_callback);
> +}

[Severity: High]
This is a pre-existing issue, but could concurrent memory hotplug events
cause an out-of-bounds array write here?

In kernel/crash_core.c:crash_prepare_headers(), we first count the ranges
and allocate memory, then later populate it:

	max_nr_ranges = arch_get_system_nr_ranges();
	if (!max_nr_ranges)
		return -ENOMEM;

	cmem = alloc_cmem(max_nr_ranges);
	if (!cmem)
		return -ENOMEM;

	ret = arch_crash_populate_cmem(cmem);

Since mem_hotplug_lock isn't held during this sequence, a concurrent memory
hotplug event (like ACPI memory hot-add) can execute add_memory_resource()
and insert new System RAM resources into the iomem resource tree between the
count and the population steps.

If the number of newly added resources exceeds the padding added in
arch_get_system_nr_ranges(), prepare_elf64_ram_headers_callback()
can blindly write past the allocated bounds of cmem->ranges:

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

It doesn't appear to check against cmem->max_nr_ranges before incrementing
cmem->nr_ranges. Does this need an explicit bounds check or synchronization
to prevent potential heap corruption?

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