Re: [PATCH v15 23/23] arm64: crash: Add crash hotplug support

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] During `kexec_file_load`, the system will read out of bounds from the `vzalloc` allocated `image->elf_headers` buffer, potentially causing a kernel panic by hitting a vmap guard page.

Pre-existing issues:
- [High] Lockless iteration of `memblock.memory` via `for_each_mem_range` during `kexec_file_load` races with memory hotplug, potentially causing a Use-After-Free.
--

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

arm64: crash: Add crash hotplug support

This commit adds crash hotplug support for arm64. It allows the architecture
to selectively update the kdump image components (like elfcorehdr) during
memory hotplug events without reloading the entire kdump image.

Link: https://lore.kernel.org/all/20240529133446.28446-1-Jonathan.Cameron@huawei.com/

> diff --git a/arch/arm64/kernel/crash.c b/arch/arm64/kernel/crash.c
> new file mode 100644
> index 0000000000000..5882b9b5a90e8
> --- /dev/null
> +++ b/arch/arm64/kernel/crash.c

[ ... ]

> +int arch_crash_populate_cmem(struct crash_mem *cmem)
> +{
> +	phys_addr_t start, end;
> +	u64 i;
> +
> +	for_each_mem_range(i, &start, &end) {

[Severity: High]
This is a pre-existing issue, but does this lockless iteration of
memblock ranges race with memory hotplug?

When invoked via the sys_kexec_file_load() path, it appears no locks
protecting memory hotplug (such as mem_hotplug_lock) are held.

If a concurrent memory hotplug event occurs, couldn't memblock_add_node()
dynamically resize the memblock.memory.regions array via
memblock_double_array() (which calls kfree() on the old array), resulting
in a use-after-free here?

[ ... ]

> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index 93c36a3aa618e..21f38de7a8b6a 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c

[ ... ]

> @@ -92,16 +93,32 @@ static void *image_load(struct kimage *image,
>  #ifdef CONFIG_CRASH_DUMP
>  	if (image->type == KEXEC_TYPE_CRASH) {
>  		/* load elf core header */
> -		unsigned long headers_sz;
> +		unsigned long headers_sz, pnum = 0;
>  		void *headers;
>  
> -		ret = crash_prepare_headers(true, &headers, &headers_sz, NULL);
> +		ret = crash_prepare_headers(true, &headers, &headers_sz, &pnum);
>  		if (ret) {
>  			pr_err("Preparing elf core header failed\n");
>  			return ERR_PTR(ret);
>  		}
>  		image->elf_headers = headers;
>  		image->elf_headers_sz = headers_sz;
> +
> +#ifdef CONFIG_CRASH_HOTPLUG
> +		/*
> +		 * The elfcorehdr segment size accounts for VMCOREINFO, kernel_map
> +		 * maximum CPUs and maximum memory ranges.
> +		 */
> +		if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
> +			pnum = 2 + num_possible_cpus() + CONFIG_CRASH_MAX_MEMORY_RANGES;
> +		else
> +			pnum += 2 + num_possible_cpus();
> +
> +		if (pnum < (unsigned long)PN_XNUM)
> +			image->elf_headers_sz = max(pnum_hdr_sz(pnum), headers_sz);

[ ... ]

> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index d0f73eb3f856f..0016001f4d005 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c

[ ... ]

> @@ -98,6 +66,12 @@ int load_other_segments(struct kimage *image,
>  		kbuf.bufsz = image->elf_headers_sz;
>  		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>  		kbuf.memsz = image->elf_headers_sz;

[Severity: Critical]
Will this cause an out-of-bounds read during the segment copy?

In arch/arm64/kernel/kexec_image.c:image_load(), image->elf_headers_sz
is artificially inflated beyond the actual allocated size (headers_sz) to
accommodate future hotpluggable CPUs and memory regions.

By setting kbuf.bufsz to this inflated size here, when the generic
kimage_load_segment() executes, it will use segment->bufsz to determine
how many bytes to copy from the source buffer.

Since the copy size now exceeds the vzalloc'd allocation size, won't this
cause the kernel to read into unmapped vmap guard pages and trigger a
panic?

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