Re: [PATCH v5 8/8] crash_core: skip MEMBLOCK_NODUMP regions when building vmcore ELF header

[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 over memblock regions without bounds checks can lead to Use-After-Free and Heap Buffer Overflow during concurrent memory hotplug.
--

--- Patch [8]: [PATCH v5 8/8] crash_core: skip MEMBLOCK_NODUMP regions when building vmcore ELF header ---
commit 7fdffadebf6d8803dc1491f6f55262a163ba9f05
Author: Wandun Chen <chenwandun@lixiang.com>

crash_core: skip MEMBLOCK_NODUMP regions when building vmcore ELF header

Reserve regions (for GPU, DSP, ...) carry no data useful for crash
analysis. Exclude MEMBLOCK_NODUMP regions from the vmcore ELF header to
reduce vmcore size and capture time. crash_should_skip_region() now also
returns true for MEMBLOCK_NODUMP.

> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index fb78b7814f188..8cc9d666bc541 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -269,10 +269,10 @@ int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>  	return 0;
>  }
>  
> -/* Exclude NOMAP regions from the vmcore. */
> +/* Exclude NOMAP and NODUMP regions from the vmcore. */
>  static bool crash_should_skip_region(struct memblock_region *reg)
>  {
> -	return memblock_is_nomap(reg);
> +	return memblock_is_nomap(reg) || memblock_is_nodump(reg);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call this
function during a lockless memblock iteration?

The caller arch_get_system_nr_ranges() uses for_each_mem_region(reg) without
taking locks. If a privileged user initiates memory hotplug concurrently
while loading a crash kernel, memblock_add_node() could trigger
memblock_double_array(), which reallocates and frees the
memblock.memory.regions array mid-iteration.

Could this result in a use-after-free on the memblock array?

kernel/crash_core.c:arch_get_system_nr_ranges() {
	...
	for_each_mem_region(reg) {
		if (crash_should_skip_region(reg))
			continue;
		nr_ranges++;
	}
	...
}

Additionally, there appears to be a time-of-check to time-of-use window
in crash_prepare_headers() between sizing and populating. If memory is
hot-added during this window, the second iteration could encounter more
regions than originally counted:

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);
	...
}

Because arch_crash_populate_cmem() writes to cmem->ranges without bounds
checking, could this race cause a heap buffer overflow on the cmem->ranges
array?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806093519.2907771-1-chenwandun1@gmail.com?part=8