[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[Baoquan He]

The memory leak is reported by kmemleak detector, has been existing
for very long time. It casue much memory loss on large machine
with huge memory hotplug which will trigger kdump kernel reloading
many times, with kexec_file_load interface.

And in patch 2, clean up is done to remove unneeded
arch_kexec_kernel_image_load() and rename functions.

V1 post can be found here:
https://lore.kernel.org/all/20211029072424.9109-1-bhe at redhat.com/T/#u

v1->v2:
 No code change. The v1 post has been acked by Dave Young but not
merged. In v2, just merging the old patch 1 and 2 as per Dave's concern
in v1 post.

Baoquan He (2):
  x86/kexec: fix memory leak of elf header buffer
  kexec_file: clean up arch_kexec_kernel_image_load

 arch/x86/kernel/machine_kexec_64.c | 23 +++++++++--------------
 include/linux/kexec.h              |  1 -
 kernel/kexec_file.c                |  9 ++-------
 3 files changed, 11 insertions(+), 22 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] x86/kexec: fix memory leak of elf header buffer

[Baoquan He]

This is reported by kmemleak detector:

unreferenced object 0xffffc900002a9000 (size 4096):
  comm "kexec", pid 14950, jiffies 4295110793 (age 373.951s)
  hex dump (first 32 bytes):
    7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
    04 00 3e 00 01 00 00 00 00 00 00 00 00 00 00 00  ..>.............
  backtrace:
    [<0000000016a8ef9f>] __vmalloc_node_range+0x101/0x170
    [<000000002b66b6c0>] __vmalloc_node+0xb4/0x160
    [<00000000ad40107d>] crash_prepare_elf64_headers+0x8e/0xcd0
    [<0000000019afff23>] crash_load_segments+0x260/0x470
    [<0000000019ebe95c>] bzImage64_load+0x814/0xad0
    [<0000000093e16b05>] arch_kexec_kernel_image_load+0x1be/0x2a0
    [<000000009ef2fc88>] kimage_file_alloc_init+0x2ec/0x5a0
    [<0000000038f5a97a>] __do_sys_kexec_file_load+0x28d/0x530
    [<0000000087c19992>] do_syscall_64+0x3b/0x90
    [<0000000066e063a4>] entry_SYSCALL_64_after_hwframe+0x44/0xae

In crash_prepare_elf64_headers(), a buffer is allocated via vmalloc() to
store elf headers. While it's not freed back to system correctly when
kdump kernel is reloaded or unloaded. Then memory leak is caused. Fix it
by introducing x86 specific function arch_kimage_file_post_load_cleanup(),
and freeing the buffer there.

And also remove the incorrect elf header buffer freeing code. Before
calling arch specific kexec_file loading function, the image instance
has been initialized. So 'image->elf_headers' must be NULL. It doesn't
make sense to free the elf header buffer in the place.

Signed-off-by: Baoquan He <bhe@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/kernel/machine_kexec_64.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index f5da4a18070a..1f0eb0eed546 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -374,9 +374,6 @@ void machine_kexec(struct kimage *image)
 #ifdef CONFIG_KEXEC_FILE
 void *arch_kexec_kernel_image_load(struct kimage *image)
 {
-	vfree(image->elf_headers);
-	image->elf_headers = NULL;
-
 	if (!image->fops || !image->fops->load)
 		return ERR_PTR(-ENOEXEC);
 
@@ -512,6 +509,15 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 	       (int)ELF64_R_TYPE(rel[i].r_info), value);
 	return -ENOEXEC;
 }
+
+int arch_kimage_file_post_load_cleanup(struct kimage *image)
+{
+	vfree(image->elf_headers);
+	image->elf_headers = NULL;
+	image->elf_headers_sz = 0;
+
+	return kexec_image_post_load_cleanup_default(image);
+}
 #endif /* CONFIG_KEXEC_FILE */
 
 static int
-- 
2.31.1

[PATCH v2 2/2] kexec_file: clean up arch_kexec_kernel_image_load

[Baoquan He]

Function arch_kexec_kernel_image_load() has a common weak version which
is a wrapper of kexec_image_load_default() , and has only one arch
dependent version in x86_64. Now the x86_64 dependent function is not
needed any more. So clean it up.

And also rename kexec_image_load_default() to kexec_kernel_image_load() for
better reflecting its functionality.

Signed-off-by: Baoquan He <bhe@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/kernel/machine_kexec_64.c | 11 -----------
 include/linux/kexec.h              |  1 -
 kernel/kexec_file.c                |  9 ++-------
 3 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 1f0eb0eed546..6b14d9274def 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -372,17 +372,6 @@ void machine_kexec(struct kimage *image)
 /* arch-dependent functionality related to kexec file-based syscall */
 
 #ifdef CONFIG_KEXEC_FILE
-void *arch_kexec_kernel_image_load(struct kimage *image)
-{
-	if (!image->fops || !image->fops->load)
-		return ERR_PTR(-ENOEXEC);
-
-	return image->fops->load(image, image->kernel_buf,
-				 image->kernel_buf_len, image->initrd_buf,
-				 image->initrd_buf_len, image->cmdline_buf,
-				 image->cmdline_buf_len);
-}
-
 /*
  * Apply purgatory relocations.
  *
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0c994ae37729..47b70402c0a4 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -186,7 +186,6 @@ void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
 /* Architectures may override the below functions */
 int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
 				  unsigned long buf_len);
-void *arch_kexec_kernel_image_load(struct kimage *image);
 int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 				     Elf_Shdr *section,
 				     const Elf_Shdr *relsec,
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 8347fc158d2b..b6a1347f546e 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -60,7 +60,7 @@ int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
 	return kexec_image_probe_default(image, buf, buf_len);
 }
 
-static void *kexec_image_load_default(struct kimage *image)
+static void *kexec_kernel_image_load(struct kimage *image)
 {
 	if (!image->fops || !image->fops->load)
 		return ERR_PTR(-ENOEXEC);
@@ -71,11 +71,6 @@ static void *kexec_image_load_default(struct kimage *image)
 				 image->cmdline_buf_len);
 }
 
-void * __weak arch_kexec_kernel_image_load(struct kimage *image)
-{
-	return kexec_image_load_default(image);
-}
-
 int kexec_image_post_load_cleanup_default(struct kimage *image)
 {
 	if (!image->fops || !image->fops->cleanup)
@@ -279,7 +274,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	ima_add_kexec_buffer(image);
 
 	/* Call arch image load handlers */
-	ldata = arch_kexec_kernel_image_load(image);
+	ldata = kexec_kernel_image_load(image);
 
 	if (IS_ERR(ldata)) {
 		ret = PTR_ERR(ldata);
-- 
2.31.1

[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[Baoquan He]

Hi Andrew,

On 02/23/22 at 07:32pm, Baoquan He wrote:
> The memory leak is reported by kmemleak detector, has been existing
> for very long time. It casue much memory loss on large machine
> with huge memory hotplug which will trigger kdump kernel reloading
> many times, with kexec_file_load interface.

Could you merge these two patches? Or should I ping x86 maintainers to
take them? 

Thanks
Baoquan

> 
> And in patch 2, clean up is done to remove unneeded
> arch_kexec_kernel_image_load() and rename functions.
> 
> V1 post can be found here:
> https://lore.kernel.org/all/20211029072424.9109-1-bhe at redhat.com/T/#u
> 
> v1->v2:
>  No code change. The v1 post has been acked by Dave Young but not
> merged. In v2, just merging the old patch 1 and 2 as per Dave's concern
> in v1 post.
> 
> Baoquan He (2):
>   x86/kexec: fix memory leak of elf header buffer
>   kexec_file: clean up arch_kexec_kernel_image_load
> 
>  arch/x86/kernel/machine_kexec_64.c | 23 +++++++++--------------
>  include/linux/kexec.h              |  1 -
>  kernel/kexec_file.c                |  9 ++-------
>  3 files changed, 11 insertions(+), 22 deletions(-)
> 
> -- 
> 2.31.1
> 

[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[Andrew Morton]

On Thu, 31 Mar 2022 19:32:17 +0800 Baoquan He <bhe@redhat.com> wrote:

> Hi Andrew,
> 
> On 02/23/22 at 07:32pm, Baoquan He wrote:
> > The memory leak is reported by kmemleak detector, has been existing
> > for very long time. It casue much memory loss on large machine
> > with huge memory hotplug which will trigger kdump kernel reloading
> > many times, with kexec_file_load interface.
> 
> Could you merge these two patches? Or should I ping x86 maintainers to
> take them? 

Ah, sorry, I tend to fall asleep if there's "x86" in the subject. 
Poking a sleeping Andrew is always the right thing to do.  Shall look
at them.

[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[Baoquan He]

On 04/12/22 at 07:27pm, Andrew Morton wrote:
> On Thu, 31 Mar 2022 19:32:17 +0800 Baoquan He <bhe@redhat.com> wrote:
> 
> > Hi Andrew,
> > 
> > On 02/23/22 at 07:32pm, Baoquan He wrote:
> > > The memory leak is reported by kmemleak detector, has been existing
> > > for very long time. It casue much memory loss on large machine
> > > with huge memory hotplug which will trigger kdump kernel reloading
> > > many times, with kexec_file_load interface.
> > 
> > Could you merge these two patches? Or should I ping x86 maintainers to
> > take them? 
> 
> Ah, sorry, I tend to fall asleep if there's "x86" in the subject. 
> Poking a sleeping Andrew is always the right thing to do.  Shall look
> at them.

Thanks, Andrew.

Since it's also MM related, so ping you and x86 maintainers to see who
can help pick them.

[PATCH v2 0/2] x86/kexec: fix memory leak of elf header buffer

[Borislav Petkov]

On Wed, Apr 13, 2022 at 10:47:49AM +0800, Baoquan He wrote:
> Since it's also MM related, so ping you and x86 maintainers to see who
> can help pick them.

I'd leave that decision to akpm. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette