Re: [PATCH v16 01/10] riscv: kexec_file: Fix crashk_low_res not exclude bug

[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 `crash_prepare_headers()` and `arch_crash_populate_cmem()` can lead to an out-of-bounds array write (Heap Buffer Overflow) if system RAM resources are hotplugged concurrently.
--

commit d18db7747e63171595f6e6caa523eb4a8bcc5bc5
Author: Jinjie Ruan <ruanjinjie@huawei.com>
Subject: riscv: kexec_file: Fix crashk_low_res not exclude bug

This commit ensures the "crashk_low_res" reserved ranges are excluded
from the crash kernel memory in RISC-V kexec_file to prevent them from
being exported through /proc/vmcore. It adds the necessary exclusion
logic and an extra crash_mem range.

Link: https://github.com/chenjh005/kexec-tools/tree/build-test-riscv-v2
Link: https://lore.kernel.org/all/20230726175000.2536220-1-chenjiahao16@huawei.com/

> diff --git a/arch/riscv/kernel/machine_kexec_file.c b/arch/riscv/kernel/machine_kexec_file.c
> index 54e2d9552e930..3f7766057cac3 100644
> --- a/arch/riscv/kernel/machine_kexec_file.c
> +++ b/arch/riscv/kernel/machine_kexec_file.c
> @@ -61,7 +61,7 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>  	unsigned int nr_ranges;
>  	int ret;
>  
> -	nr_ranges = 1; /* For exclusion of crashkernel region */
> +	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);

[Severity: High]
This is a pre-existing issue, but could this multi-pass allocation pattern
lead to an out-of-bounds array write if system RAM resources are hotplugged
concurrently?

Looking at the refactored logic in kernel/crash_core.c and the callbacks in
arch/riscv/kernel/machine_kexec_file.c, there appears to be a Time-of-Check
to Time-of-Use (TOCTOU) gap:

crash_prepare_headers()
   arch_get_system_nr_ranges()
       /* First walk: calculates max_nr_ranges */
   cmem = alloc_cmem(max_nr_ranges)
   arch_crash_populate_cmem(cmem)
       /* Second walk: populates cmem */

Since neither the global resource_lock nor device_hotplug_lock is held across
both passes, a concurrent memory hotplug event (e.g., add_memory()) could
insert new RAM ranges into iomem_resource between the two passes.

If the second pass encounters more ranges than originally allocated,
prepare_elf64_ram_headers_callback() writes directly to the array and
increments the counter:

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

Does this code need a bounds check against cmem->max_nr_ranges in the
callback to prevent a heap buffer overflow in the cmem->ranges array?

[ ... ]

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