Re: [PATCH v15 10/23] crash_core: Introduce CRASH_HOTPLUG_SAFETY_PADDING for memory hotplug safety

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless iteration of memblock regions can lead to Use-After-Free during kexec_file_load, which the introduced padding heuristic fails to address.
--

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

crash_core: Introduce CRASH_HOTPLUG_SAFETY_PADDING for memory hotplug safety

This commit introduces CRASH_HOTPLUG_SAFETY_PADDING to allocate extra slots
for the crash memory ranges array. This is intended to mitigate potential
TOCTOU races caused by concurrent memory hotplug events.

> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index c1dee3f971a91..d4762e000098b 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -14,6 +14,12 @@ struct crash_mem {
>  	struct range ranges[] __counted_by(max_nr_ranges);
>  };
>  
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +#define CRASH_HOTPLUG_SAFETY_PADDING 128
[Severity: High]
This is a pre-existing issue, but does this padding actually resolve the
concurrency problems with memory hotplug?

sys_kexec_file_load() holds kexec_lock but does not acquire the memory
hotplug lock (mem_hotplug_begin() or get_online_mems()). Consequently,
functions like arch_crash_populate_cmem() in arch/arm64/kernel/crash.c
and arch/loongarch/kernel/machine_kexec_file.c iterate over
memblock.memory.regions locklessly via for_each_mem_range().

If a concurrent memory hotplug event (e.g., __add_memory()) takes the
hotplug lock and calls memblock_add_node(), it can trigger
memblock_double_array(). This dynamically reallocates the memblock array
and immediately frees the old array via kfree() with no RCU grace period.

If for_each_mem_range() runs concurrently during the kfree(), could it
dereference the freed array, resulting in a direct use-after-free on the
source memblock.memory.regions array?

While the new padding heuristic might prevent out-of-bounds writes to the
destination cmem->ranges array, it appears it would fail to prevent the
use-after-free during the iteration itself.

> +#else
> +#define CRASH_HOTPLUG_SAFETY_PADDING 0
> +#endif
> +
>  #ifdef CONFIG_CRASH_DUMP
>  
>  int crash_shrink_memory(unsigned long new_size);

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