Re: [PATCH v8 06/11] x86: kexec_file: Use the common segment helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] x86/crash: unset elfcorehdr_index in crash_load_segments()
  breaks hotplug
--

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

x86: kexec_file: Use the common segment helpers

This commit replaces open-coded ELF core header and initrd segment loading
in x86 kexec_file with the common kexec_prepare_elf_core_header(),
kexec_add_elf_core_header(), and kexec_add_initrd() helpers.

> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index e6f23933a6df2..8d56d39ebbd53 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -354,14 +354,12 @@ int crash_load_segments(struct kimage *image)
>  				  .buf_max = ULONG_MAX, .top_down = false };
>  
>  	/* Prepare elf headers and add a segment */
> -	ret = crash_prepare_headers(IS_ENABLED(CONFIG_X86_64), &kbuf.buffer,
> -				    &kbuf.bufsz, &pnum);
> +	ret = kexec_prepare_elf_core_header(image, IS_ENABLED(CONFIG_X86_64),
> +					    &pnum);
>  	if (ret)
>  		return ret;
>  
> -	image->elf_headers	= kbuf.buffer;
> -	image->elf_headers_sz	= kbuf.bufsz;
> -	kbuf.memsz		= kbuf.bufsz;
> +	kbuf.memsz = image->elf_headers_sz;
>  
>  #ifdef CONFIG_CRASH_HOTPLUG
>  	/*
> @@ -374,28 +372,15 @@ int crash_load_segments(struct kimage *image)
>  		pnum += 2 + CONFIG_NR_CPUS;
>  
>  	if (pnum < (unsigned long)PN_XNUM) {
> -		kbuf.memsz = pnum * sizeof(Elf64_Phdr);
> -		kbuf.memsz += sizeof(Elf64_Ehdr);
> -
> -		image->elfcorehdr_index = image->nr_segments;
> -
> -		/* Mark as usable to crash kernel, else crash kernel fails on boot */
> -		image->elf_headers_sz = kbuf.memsz;
> +		kbuf.memsz = pnum * sizeof(Elf64_Phdr) + sizeof(Elf64_Ehdr);
>  	} else {
>  		pr_err("number of Phdrs %lu exceeds max\n", pnum);
>  	}
>  #endif
>  
>  	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> -	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> -	ret = kexec_add_buffer(&kbuf);
> -	if (ret)
> -		return ret;
> -	image->elf_load_addr = kbuf.mem;
> -	kexec_dprintk("Loaded ELF headers at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> -		      image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
>  
> -	return ret;
> +	return kexec_add_elf_core_header(image, &kbuf);
>  }

[Severity: High]
Can this leave image->elfcorehdr_index unset (-1) when CONFIG_CRASH_HOTPLUG
is enabled?

kexec_prepare_elf_core_header() -> crash_prepare_headers() ->
crash_prepare_elf64_headers() aligns image->elf_headers_sz up to
ELF_CORE_HEADER_ALIGN (4096), whereas crash_load_segments() computes an
unaligned kbuf.memsz for the hotplug reservation.  In
kexec_add_elf_core_header(), image->elfcorehdr_index is only updated when
memsz strictly exceeds image->elf_headers_sz:

kernel/kexec_file.c:kexec_add_elf_core_header() {
	...
	/* The crash hotplug reservation can exceed the header size. */
	if (memsz > image->elf_headers_sz) {
		image->elf_headers_sz = memsz;
#ifdef CONFIG_CRASH_HOTPLUG
		image->elfcorehdr_index = image->nr_segments - 1;
#endif
	}
	...
}

Whenever the unaligned hotplug kbuf.memsz fits within the same 4096-byte
multiple as the initial header (for example, when CONFIG_MEMORY_HOTPLUG is
disabled and CONFIG_NR_CPUS equals num_possible_cpus()), memsz <=
image->elf_headers_sz and image->elfcorehdr_index remains -1.
kexec_calculate_store_digests() then includes the elfcorehdr segment in the
purgatory SHA-256 digest, so any later CPU or memory hotplug update to
elfcorehdr causes purgatory digest verification to fail on crash.

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