* [PATCH] x86/kexec: fix double vfree of image->elf_headers
@ 2023-01-02 10:39 Vlastimil Babka
2023-01-02 11:24 ` Vlastimil Babka
0 siblings, 1 reply; 3+ messages in thread
From: Vlastimil Babka @ 2023-01-02 10:39 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
Cc: x86, H. Peter Anvin, patches, linux-mm, linux-kernel,
Vlastimil Babka, Baoquan He, Dave Young, stable
An investigation of a "Trying to vfree() nonexistent vm area" bug
occurring in arch_kimage_file_post_load_cleanup() doing a
vfree(image->elf_headers) in our 5.14-based kernel yielded the following
double vfree() scenario, also present in mainline:
SYSCALL_DEFINE5(kexec_file_load)
kimage_file_alloc_init()
kimage_file_prepare_segments()
arch_kexec_kernel_image_probe()
kexec_image_load_default()
kexec_bzImage64_ops.load()
bzImage64_load()
crash_load_segments()
prepare_elf_headers(image, &kbuf.buffer, &kbuf.bufsz);
image->elf_headers = kbuf.buffer;
ret = kexec_add_buffer(&kbuf);
if (ret) vfree((void *)image->elf_headers); // first vfree()
if (ret) kimage_file_post_load_cleanup()
vfree(image->elf_headers); // second vfree()
AFAICS the scenario is possible since v5.19 commit b3e34a47f989
("x86/kexec: fix memory leak of elf header buffer") that was marked for
stable and also was backported to our kernel.
Fix the problem by setting the pointer to NULL after the first vfree().
Also set elf_headers_sz to 0, as kimage_file_post_load_cleanup() does.
Fixes: b3e34a47f989 ("x86/kexec: fix memory leak of elf header buffer")
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: <stable@vger.kernel.org>
---
arch/x86/kernel/crash.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9730c88530fc..0d651c05a49e 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -403,6 +403,8 @@ int crash_load_segments(struct kimage *image)
ret = kexec_add_buffer(&kbuf);
if (ret) {
vfree((void *)image->elf_headers);
+ image->elf_headers = NULL;
+ image->elf_headers_sz = 0;
return ret;
}
image->elf_load_addr = kbuf.mem;
--
2.39.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/kexec: fix double vfree of image->elf_headers
2023-01-02 10:39 [PATCH] x86/kexec: fix double vfree of image->elf_headers Vlastimil Babka
@ 2023-01-02 11:24 ` Vlastimil Babka
2023-01-03 10:15 ` Ingo Molnar
0 siblings, 1 reply; 3+ messages in thread
From: Vlastimil Babka @ 2023-01-02 11:24 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, Takashi Iwai
Cc: x86, H. Peter Anvin, patches, linux-mm, linux-kernel, Baoquan He,
Dave Young, stable
On 1/2/23 11:39, Vlastimil Babka wrote:
> An investigation of a "Trying to vfree() nonexistent vm area" bug
> occurring in arch_kimage_file_post_load_cleanup() doing a
> vfree(image->elf_headers) in our 5.14-based kernel yielded the following
> double vfree() scenario, also present in mainline:
>
> SYSCALL_DEFINE5(kexec_file_load)
> kimage_file_alloc_init()
> kimage_file_prepare_segments()
> arch_kexec_kernel_image_probe()
> kexec_image_load_default()
> kexec_bzImage64_ops.load()
> bzImage64_load()
> crash_load_segments()
> prepare_elf_headers(image, &kbuf.buffer, &kbuf.bufsz);
> image->elf_headers = kbuf.buffer;
> ret = kexec_add_buffer(&kbuf);
> if (ret) vfree((void *)image->elf_headers); // first vfree()
> if (ret) kimage_file_post_load_cleanup()
> vfree(image->elf_headers); // second vfree()
>
> AFAICS the scenario is possible since v5.19 commit b3e34a47f989
> ("x86/kexec: fix memory leak of elf header buffer") that was marked for
> stable and also was backported to our kernel.
>
> Fix the problem by setting the pointer to NULL after the first vfree().
> Also set elf_headers_sz to 0, as kimage_file_post_load_cleanup() does.
>
> Fixes: b3e34a47f989 ("x86/kexec: fix memory leak of elf header buffer")
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: <stable@vger.kernel.org>
Takashi told me he sent a slightly different fix already in November:
https://lore.kernel.org/all/20221122115122.13937-1-tiwai@suse.de/
Seems it wasn't picked up? You might pick his then, as Baoquan acked it, and
it's removing code, not adding it.
> ---
> arch/x86/kernel/crash.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9730c88530fc..0d651c05a49e 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -403,6 +403,8 @@ int crash_load_segments(struct kimage *image)
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> vfree((void *)image->elf_headers);
> + image->elf_headers = NULL;
> + image->elf_headers_sz = 0;
> return ret;
> }
> image->elf_load_addr = kbuf.mem;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/kexec: fix double vfree of image->elf_headers
2023-01-02 11:24 ` Vlastimil Babka
@ 2023-01-03 10:15 ` Ingo Molnar
0 siblings, 0 replies; 3+ messages in thread
From: Ingo Molnar @ 2023-01-03 10:15 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Takashi Iwai, x86, H. Peter Anvin, patches, linux-mm,
linux-kernel, Baoquan He, Dave Young, stable
* Vlastimil Babka <vbabka@suse.cz> wrote:
> On 1/2/23 11:39, Vlastimil Babka wrote:
> > An investigation of a "Trying to vfree() nonexistent vm area" bug
> > occurring in arch_kimage_file_post_load_cleanup() doing a
> > vfree(image->elf_headers) in our 5.14-based kernel yielded the following
> > double vfree() scenario, also present in mainline:
> >
> > SYSCALL_DEFINE5(kexec_file_load)
> > kimage_file_alloc_init()
> > kimage_file_prepare_segments()
> > arch_kexec_kernel_image_probe()
> > kexec_image_load_default()
> > kexec_bzImage64_ops.load()
> > bzImage64_load()
> > crash_load_segments()
> > prepare_elf_headers(image, &kbuf.buffer, &kbuf.bufsz);
> > image->elf_headers = kbuf.buffer;
> > ret = kexec_add_buffer(&kbuf);
> > if (ret) vfree((void *)image->elf_headers); // first vfree()
> > if (ret) kimage_file_post_load_cleanup()
> > vfree(image->elf_headers); // second vfree()
> >
> > AFAICS the scenario is possible since v5.19 commit b3e34a47f989
> > ("x86/kexec: fix memory leak of elf header buffer") that was marked for
> > stable and also was backported to our kernel.
> >
> > Fix the problem by setting the pointer to NULL after the first vfree().
> > Also set elf_headers_sz to 0, as kimage_file_post_load_cleanup() does.
> >
> > Fixes: b3e34a47f989 ("x86/kexec: fix memory leak of elf header buffer")
> > Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: <stable@vger.kernel.org>
>
> Takashi told me he sent a slightly different fix already in November:
> https://lore.kernel.org/all/20221122115122.13937-1-tiwai@suse.de/
>
> Seems it wasn't picked up? You might pick his then, as Baoquan acked it, and
> it's removing code, not adding it.
Thanks, indeed we missed that fix - Boris picked up that version in
tip:x86/urgent, via:
d00dd2f2645d ("x86/kexec: Fix double-free of elf header buffer")
Ingo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-03 10:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-02 10:39 [PATCH] x86/kexec: fix double vfree of image->elf_headers Vlastimil Babka
2023-01-02 11:24 ` Vlastimil Babka
2023-01-03 10:15 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox