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