Re: [PATCH v13 04/15] arm64: kexec_file: Fix potential buffer overflow in prepare_elf_headers()

Jinjie Ruan <[email protected]> Tue, 19 May 2026 20:33:26 +0800
Newsgroups dev.linux.lists.sashiko,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>

On 5/12/2026 8:45 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Memory leak of image->elf_headers on error paths in load_other_segments().
> --
> 
> commit 176ac9ad5382c8a7611ce5e111bc472c392e772b
> Author: Jinjie Ruan <[email protected]>
> 
> arm64: kexec_file: Fix potential buffer overflow in prepare_elf_headers()
> 
> This commit fixes a race condition between the kexec_load() system call and
> memory hotplug operations that can lead to a buffer overflow. It adds an
> explicit bounds check to prevent out-of-bounds access when populating
> the memory ranges array.
> 
> Closes: https://sashiko.dev/#/patchset/20260323072745.2481719-1-ruanjinjie%40huawei.com
> 
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index fba260ad87a96..df52ac4474c96 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -59,6 +59,11 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
>>  	cmem->max_nr_ranges = nr_ranges;
>>  	cmem->nr_ranges = 0;
>>  	for_each_mem_range(i, &start, &end) {
>> +		if (cmem->nr_ranges >= cmem->max_nr_ranges) {
>> +			ret = -ENOMEM;
>> +			goto out;
>> +		}
>> +
>>  		cmem->ranges[cmem->nr_ranges].start = start;
>>  		cmem->ranges[cmem->nr_ranges].end = end - 1;
>>  		cmem->nr_ranges++;
> 
> While reviewing this commit, I noticed a pre-existing issue in the
> surrounding code.
> 
> Could the error paths in load_other_segments() leak the memory
> allocated by prepare_elf_headers()?
> 
> If a failure occurs later in load_other_segments(), for example when
> adding the initrd or dtb, the function jumps to out_err:
> 
> arch/arm64/kernel/machine_kexec_file.c:load_other_segments() {
> 	[ ... ]
> 		ret = kexec_add_buffer(&kbuf);
> 		if (ret) {
> 			vfree(headers);
> 			goto out_err;
> 		}
> 		image->elf_headers = headers;
> 	[ ... ]
> 	if (initrd) {
> 	[ ... ]
> 		ret = kexec_add_buffer(&kbuf);
> 		if (ret)
> 			goto out_err;
> 	[ ... ]
> out_err:
> 	image->nr_segments = orig_segments;
> 	kvfree(dtb);
> 	return ret;
> }
> 
> The out_err path returns an error but does not free image->elf_headers.
> 
> This seems problematic because the caller in kexec_image.c
> handles placement failures by erasing the segment and retrying in a loop:
> 
> arch/arm64/kernel/kexec_image.c:image_load() {
> 	[ ... ]
> 	while ((ret = kexec_add_buffer(&kbuf)) == 0) {
> 		kernel_segment = &image->segment[kernel_segment_number];
> 		ret = load_other_segments(image, kernel_segment->mem, ...);
> 		if (!ret)
> 			break;
> 	[ ... ]
> }
> 
> Does this retry loop unconditionally allocate new ELF headers on each
> iteration and permanently leak the previously allocated buffers?

It seems this is a bug, and I plan to fix this issue as follows, and
would appreciate your feedback on whether this is reasonable.

Decouple the ELF header allocation from the target-seeking
retry loop. Since the contents and size of ELF headers only depend on
the host memory layout and do not change with the kernel's physical
placement, move prepare_elf_headers() completely outside and prior to
the while retry loop in image_load().

diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 892e5bebda95..cc2f36b1b0d4 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -127,7 +127,8 @@ int arch_kimage_file_post_load_cleanup(struct kimage
*image);
 extern int load_other_segments(struct kimage *image,
 		unsigned long kernel_load_addr, unsigned long kernel_size,
 		char *initrd, unsigned long initrd_len,
-		char *cmdline);
+		char *cmdline, void *headers, unsigned long headers_size);
+extern int prepare_elf_headers(void **addr, unsigned long *sz);
 #endif

 #endif /* __ASSEMBLER__ */
diff --git a/arch/arm64/kernel/kexec_image.c
b/arch/arm64/kernel/kexec_image.c
index b70f4df15a1a..79efeaeb71e9 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -44,6 +44,11 @@ static void *image_load(struct kimage *image,
 	struct kexec_buf kbuf = {};
 	unsigned long text_offset, kernel_segment_number;
 	struct kexec_segment *kernel_segment;
+#ifdef CONFIG_CRASH_DUMP
+	/* load elf core header */
+	unsigned long headers_sz;
+	void *headers;
+#endif
 	int ret;

 	/*
@@ -89,6 +94,18 @@ static void *image_load(struct kimage *image,

 	kernel_segment_number = image->nr_segments;

+#ifdef CONFIG_CRASH_DUMP
+	if (image->type == KEXEC_TYPE_CRASH) {
+		ret = prepare_elf_headers(&headers, &headers_sz);
+		if (ret) {
+			pr_err("Preparing elf core header failed\n");
+			return ERR_PTR(ret);
+		}
+		image->elf_headers = headers;
+		image->elf_headers_sz = headers_sz;
+	}
+#endif
+
 	/*
 	 * The location of the kernel segment may make it impossible to satisfy
 	 * the other segment requirements, so we try repeatedly to find a
@@ -99,7 +116,8 @@ static void *image_load(struct kimage *image,
 		kernel_segment = &image->segment[kernel_segment_number];
 		ret = load_other_segments(image, kernel_segment->mem,
 					  kernel_segment->memsz, initrd,
-					  initrd_len, cmdline);
+					  initrd_len, cmdline,
+					  headers, headers_sz);
 		if (!ret)
 			break;

@@ -107,7 +125,7 @@ static void *image_load(struct kimage *image,
 		 * We couldn't find space for the other segments; erase the
 		 * kernel segment and try the next available hole.
 		 */
-		image->nr_segments -= 1;
+		image->nr_segments = kernel_segment_number;
 		kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
 		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 	}
diff --git a/arch/arm64/kernel/machine_kexec_file.c
b/arch/arm64/kernel/machine_kexec_file.c
index e31fabed378a..daf81a873bbd 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -40,7 +40,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage
*image)
 }

 #ifdef CONFIG_CRASH_DUMP
-static int prepare_elf_headers(void **addr, unsigned long *sz)
+int prepare_elf_headers(void **addr, unsigned long *sz)
 {
 	struct crash_mem *cmem;
 	unsigned int nr_ranges;
@@ -92,7 +92,8 @@ int load_other_segments(struct kimage *image,
 			unsigned long kernel_load_addr,
 			unsigned long kernel_size,
 			char *initrd, unsigned long initrd_len,
-			char *cmdline)
+			char *cmdline, void *headers,
+			unsigned long headers_sz)
 {
 	struct kexec_buf kbuf = {};
 	void *dtb = NULL;
@@ -105,16 +106,7 @@ int load_other_segments(struct kimage *image,
 	kbuf.buf_min = kernel_load_addr + kernel_size;

 #ifdef CONFIG_CRASH_DUMP
-	/* load elf core header */
-	void *headers;
-	unsigned long headers_sz;
 	if (image->type == KEXEC_TYPE_CRASH) {
-		ret = prepare_elf_headers(&headers, &headers_sz);
-		if (ret) {
-			pr_err("Preparing elf core header failed\n");
-			goto out_err;
-		}
-
 		kbuf.buffer = headers;
 		kbuf.bufsz = headers_sz;
 		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -128,9 +120,7 @@ int load_other_segments(struct kimage *image,
 			vfree(headers);
 			goto out_err;
 		}
-		image->elf_headers = headers;
 		image->elf_load_addr = kbuf.mem;
-		image->elf_headers_sz = headers_sz;

 		kexec_dprintk("Loaded elf core header at 0x%lx bufsz=0x%lx
memsz=0x%lx\n",
 			      image->elf_load_addr, kbuf.bufsz, kbuf.memsz);

>