The memory leak is reported by kmemleak detector, has been existing
for very long time. It could 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, 3, clean up is done to remove unnecessary elf header
buffer freeing and unneeded arch_kexec_kernel_image_load().
Baoquan He (3):
x86/kexec: fix memory leak of elf header buffer
x86/kexec: remove incorrect elf header buffer freeing
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.17.2
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.
Signed-off-by: Baoquan He <[email protected]>
---
arch/x86/kernel/machine_kexec_64.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 131f30fdcfbd..fd8223fa2de5 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -511,6 +511,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.17.2
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.
So remove it.
Signed-off-by: Baoquan He <[email protected]>
---
arch/x86/kernel/machine_kexec_64.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index fd8223fa2de5..dc8b17568784 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -373,9 +373,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);
--
2.17.2
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 <[email protected]>
---
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 dc8b17568784..6339ae7e6e79 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -371,17 +371,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 33400ff051a8..0f2eed939f1f 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.17.2
Hi Baoquan,
On 10/29/21 at 03:24pm, Baoquan He wrote:
> The memory leak is reported by kmemleak detector, has been existing
> for very long time. It could 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, 3, clean up is done to remove unnecessary elf header
> buffer freeing and unneeded arch_kexec_kernel_image_load().
>
> Baoquan He (3):
> x86/kexec: fix memory leak of elf header buffer
> x86/kexec: remove incorrect elf header buffer freeing
> 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.17.2
>
Acked-by: Dave Young <[email protected]>
nitpick: the first two patches can be merged togeter, but I'm also fine if
they are in two patches.
Thanks
Dave
On 10/29/21 at 03:24pm, Baoquan He wrote:
> 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.
>
> Signed-off-by: Baoquan He <[email protected]>
It deserves to add a Fixes tag, and this drawback exists since
kexec_file_load added. It wastes one page of memory, or severa pages
depending on the cpu numbers, not sure if it deserves to cc stable.
Fixes: cb1052581e2b ("kexec: implementation of new syscall kexec_file_load")
> ---
> arch/x86/kernel/machine_kexec_64.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 131f30fdcfbd..fd8223fa2de5 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -511,6 +511,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.17.2
>