[PATCH v2] proc/vmcore: fix potential memory leak in vmcore_init()

2022-09-13 Thread Jianglei Nie
elfcorehdr_alloc() allocates a memory chunk for elfcorehdr_addr with
kzalloc(). If is_vmcore_usable() returns false, elfcorehdr_addr is a
predefined value. If parse_crash_elf_headers() gets some error and
returns a negetive value, the elfcorehdr_addr should be released with
elfcorehdr_free().

Fix it by calling elfcorehdr_free() when parse_crash_elf_headers() fails.

Acked-by: Baoquan He 
Signed-off-by: Jianglei Nie 
---
 fs/proc/vmcore.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index f2aa86c421f2..74747571d58e 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1567,6 +1567,7 @@ static int __init vmcore_init(void)
return rc;
rc = parse_crash_elf_headers();
if (rc) {
+   elfcorehdr_free(elfcorehdr_addr);
pr_warn("Kdump: vmcore not initialized\n");
return rc;
}
-- 
2.25.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v12 7/7] x86/crash: Add x86 crash hotplug support

2022-09-13 Thread Eric DeVolder

Boris,
Thanks for the feedback! Inline responses below.
eric

On 9/12/22 01:52, Borislav Petkov wrote:

On Fri, Sep 09, 2022 at 05:05:09PM -0400, Eric DeVolder wrote:

For x86_64, when CPU or memory is hot un/plugged, the crash
elfcorehdr, which describes the CPUs and memory in the system,
must also be updated.

When loading the crash kernel via kexec_load or kexec_file_load,


Please end function names with parentheses. Check the whole patch pls.

Done.




the elfcorehdr is identified at run time in
crash_core:handle_hotplug_event().

To update the elfcorehdr for x86_64, a new elfcorehdr must be
generated from the available CPUs and memory. The new elfcorehdr
is prepared into a buffer, and then installed over the top of
the existing elfcorehdr.

In the patch 'kexec: exclude elfcorehdr from the segment digest'
the need to update purgatory due to the change in elfcorehdr was
eliminated.  As a result, no changes to purgatory or boot_params
(as the elfcorehdr= kernel command line parameter pointer
remains unchanged and correct) are needed, just elfcorehdr.

To accommodate a growing number of resources via hotplug, the
elfcorehdr segment must be sufficiently large enough to accommodate
changes, see the CRASH_MAX_MEMORY_RANGES configure item.

With this change, crash hotplug for kexec_file_load syscall
is supported.


Redundant sentence.

Removed.




The kexec_load is also supported, but also
requires a corresponding change to userspace kexec-tools.


Ditto.

Removed.




Signed-off-by: Eric DeVolder 
Acked-by: Baoquan He 
---
  arch/x86/Kconfig |  11 
  arch/x86/include/asm/kexec.h |  20 +++
  arch/x86/kernel/crash.c  | 102 +++
  3 files changed, 133 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f9920f1341c8..cdfc9b2fdf98 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2056,6 +2056,17 @@ config CRASH_DUMP
  (CONFIG_RELOCATABLE=y).
  For more details see Documentation/admin-guide/kdump/kdump.rst
  
+config CRASH_MAX_MEMORY_RANGES

+   depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU || MEMORY_HOTPLUG)
+   int
+   default 32768
+   help
+ For the kexec_file_load path, specify the maximum number of
+ memory regions, eg. as represented by the 'System RAM' entries
+ in /proc/iomem, that the elfcorehdr buffer/segment can accommodate.
+ This value is combined with NR_CPUS and multiplied by Elf64_Phdr
+ size to determine the final buffer size.


If I'm purely a user, I'm left wondering how to determine what to
specify. Do you have a guidance text somewhere you can point to from
here?


This topic was discussed previously https://lkml.org/lkml/2022/3/3/372.
David points out that terminology is tricky here due to differing behaviors.
And perhaps that is your point in asking for guidance text. It can be
complicated, but it all comes down to System RAM entries.

I could perhaps offer an overly simplified example such that for 1GiB block
size, for example, the CRASH_MAX_MEMORY_RANGES of 32768 would allow for 32TiB
of memory?




+
  config KEXEC_JUMP
bool "kexec jump"
depends on KEXEC && HIBERNATION
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index a3760ca796aa..432073385b2d 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -212,6 +212,26 @@ typedef void crash_vmclear_fn(void);
  extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
  extern void kdump_nmi_shootdown_cpus(void);
  
+void *arch_map_crash_pages(unsigned long paddr, unsigned long size);

+#define arch_map_crash_pages arch_map_crash_pages
+
+void arch_unmap_crash_pages(void **ptr);
+#define arch_unmap_crash_pages arch_unmap_crash_pages
+
+void arch_crash_handle_hotplug_event(struct kimage *image,
+   unsigned int hp_action);
+#define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
+
+#ifdef CONFIG_HOTPLUG_CPU
+static inline int crash_hotplug_cpu_support(void) { return 1; }
+#define crash_hotplug_cpu_support crash_hotplug_cpu_support
+#endif
+
+#ifdef CONFIG_MEMORY_HOTPLUG
+static inline int crash_hotplug_memory_support(void) { return 1; }
+#define crash_hotplug_memory_support crash_hotplug_memory_support
+#endif
+
  #endif /* __ASSEMBLY__ */
  
  #endif /* _ASM_X86_KEXEC_H */

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9ceb93c176a6..8fc7d678ac72 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 

  #include 
@@ -397,7 +398,18 @@ int crash_load_segments(struct kimage *image)
image->elf_headers = kbuf.buffer;
image->elf_headers_sz = kbuf.bufsz;
  
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)

+   /* Ensure elfcorehdr segment large enough for hotplug changes */
+   kbuf.memsz =
+   (CONFIG_NR_CPUS_DEFAULT + 

Re: [PATCH] proc/vmcore: fix potential memory leak in vmcore_init()

2022-09-13 Thread Baoquan He
On 09/13/22 at 07:35am, Matthew Wilcox wrote:
> On Tue, Sep 13, 2022 at 02:25:01PM +0800, Jianglei Nie wrote:
> > }
> > -   elfcorehdr_free(elfcorehdr_addr);
> > elfcorehdr_addr = ELFCORE_ADDR_ERR;
> >  
> > proc_vmcore = proc_create("vmcore", S_IRUSR, NULL, _proc_ops);
> > if (proc_vmcore)
> > proc_vmcore->size = vmcore_size;
> > -   return 0;
> > +
> > +fail:
> > +   elfcorehdr_free(elfcorehdr_addr);
> > +   return rc;
> >  }
> 
> Did you test this?  It looks like you now call
> elfcorehdr_free(ELFCORE_ADDR_ERR) if 'rc' is 0.

Right, that will cause problem. It's my fault since I suggested
the current change.

Jianglei, please use your v1 change and post again. Sorry for the
incorrect suggestion.


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] proc/vmcore: fix potential memory leak in vmcore_init()

2022-09-13 Thread Matthew Wilcox
On Tue, Sep 13, 2022 at 02:25:01PM +0800, Jianglei Nie wrote:
>   }
> - elfcorehdr_free(elfcorehdr_addr);
>   elfcorehdr_addr = ELFCORE_ADDR_ERR;
>  
>   proc_vmcore = proc_create("vmcore", S_IRUSR, NULL, _proc_ops);
>   if (proc_vmcore)
>   proc_vmcore->size = vmcore_size;
> - return 0;
> +
> +fail:
> + elfcorehdr_free(elfcorehdr_addr);
> + return rc;
>  }

Did you test this?  It looks like you now call
elfcorehdr_free(ELFCORE_ADDR_ERR) if 'rc' is 0.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH] proc/vmcore: fix potential memory leak in vmcore_init()

2022-09-13 Thread Jianglei Nie
elfcorehdr_alloc() allocates a memory chunk for elfcorehdr_addr with
kzalloc(). If is_vmcore_usable() returns false, elfcorehdr_addr is a
predefined value. If parse_crash_elf_headers() gets some error and
returns a negetive value, the elfcorehdr_addr should be released with
elfcorehdr_free().

Fix it by calling elfcorehdr_free() when parse_crash_elf_headers() fails.

Acked-by: Baoquan He 
Signed-off-by: Jianglei Nie 
---
 fs/proc/vmcore.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index f2aa86c421f2..163cb266f5fd 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1568,15 +1568,17 @@ static int __init vmcore_init(void)
rc = parse_crash_elf_headers();
if (rc) {
pr_warn("Kdump: vmcore not initialized\n");
-   return rc;
+   goto fail;
}
-   elfcorehdr_free(elfcorehdr_addr);
elfcorehdr_addr = ELFCORE_ADDR_ERR;
 
proc_vmcore = proc_create("vmcore", S_IRUSR, NULL, _proc_ops);
if (proc_vmcore)
proc_vmcore->size = vmcore_size;
-   return 0;
+
+fail:
+   elfcorehdr_free(elfcorehdr_addr);
+   return rc;
 }
 fs_initcall(vmcore_init);
 
-- 
2.25.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec