Re: [PATCH v16 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] A time-of-check to time-of-use (TOCTOU) race condition exists between arch_get_system_nr_ranges() and arch_crash_populate_cmem() against concurrent memory hotplug, potentially causing a buffer overflow or Use-After-Free.
--

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

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

This commit replaces prepare_elf_headers() with the generic
crash_prepare_headers() helper to simplify crash memory handling. It implements
arch_get_system_nr_ranges() to count memory ranges and
arch_crash_populate_cmem() to fill them.

> diff --git a/arch/loongarch/kernel/machine_kexec_file.c b/arch/loongarch/kernel/machine_kexec_file.c
> index 5584b798ba464..5412aa9f35687 100644
> --- a/arch/loongarch/kernel/machine_kexec_file.c
> +++ b/arch/loongarch/kernel/machine_kexec_file.c
[ ... ]
> +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, but is there a risk of a time-of-check to
time-of-use (TOCTOU) race condition here against concurrent memory hotplug?

Because sys_kexec_file_load() acquires kexec_mutex but does not hold memory
hotplug locks (e.g., mem_hotplug_begin()) or cpus_read_lock(), concurrent
memory hotplug events like add_memory_resource() could occur.

The generic crash_prepare_headers() separates counting and population:

kernel/crash_core.c:crash_prepare_headers() {
    ...
    max_nr_ranges = arch_get_system_nr_ranges();
    if (!max_nr_ranges)
        return -ENOMEM;
    
    cmem = alloc_cmem(max_nr_ranges);
    ...
    ret = arch_crash_populate_cmem(cmem);
    ...
}

If memblock regions are added dynamically between the time max_nr_ranges is
calculated and the cmem->ranges array is populated in
arch_crash_populate_cmem(), won't the loop above iterate more times than
max_nr_ranges and blindly write out-of-bounds to the flexible ranges array?

Additionally, could a concurrent memblock_add_node() trigger a
memblock_double_array(), which reallocates the regions array and frees the
old one, resulting in a use-after-free while for_each_mem_range() is
traversing it?

> +	return 0;
>  }
>  
>  /*

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