linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y
@ 2019-12-02 18:02 Omar Sandoval
  2019-12-18 10:41 ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Omar Sandoval @ 2019-12-02 18:02 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86; +Cc: linux-kernel

From: Omar Sandoval <osandov@fb.com>

kernel/crash_core.c calls arch_crash_save_vmcoreinfo() to get
arch-specific bits for vmcoreinfo. If it is not defined, then it has a
no-op fallback. kernel/crash_core.c is gated behind CONFIG_CRASH_CORE.
However, x86 defines arch_crash_save_vmcoreinfo() in
arch/x86/kernel/machine_kexec_*.c, which is gated behind
CONFIG_KEXEC_CORE. So, a kernel with CONFIG_CRASH_CORE=y and
CONFIG_KEXEC_CORE=n uses the fallback and gets incomplete vmcoreinfo
data, which can confuse applications reading vmcoreinfo from
/proc/kcore.

Fix it by moving arch_crash_save_vmcoreinfo() into two new
arch/x86/kernel/crash_core_*.c files, which are gated behind
CONFIG_CRASH_CORE.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Based on Linus's tree.

 arch/x86/kernel/Makefile           |  1 +
 arch/x86/kernel/crash_core_32.c    | 17 +++++++++++++++++
 arch/x86/kernel/crash_core_64.c    | 24 ++++++++++++++++++++++++
 arch/x86/kernel/machine_kexec_32.c | 12 ------------
 arch/x86/kernel/machine_kexec_64.c | 19 -------------------
 5 files changed, 42 insertions(+), 31 deletions(-)
 create mode 100644 arch/x86/kernel/crash_core_32.c
 create mode 100644 arch/x86/kernel/crash_core_64.c

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 6175e370ee4a..9b294c13809a 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_FUNCTION_TRACER)	+= ftrace_$(BITS).o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
 obj-$(CONFIG_FTRACE_SYSCALLS)	+= ftrace.o
 obj-$(CONFIG_X86_TSC)		+= trace_clock.o
+obj-$(CONFIG_CRASH_CORE)	+= crash_core_$(BITS).o
 obj-$(CONFIG_KEXEC_CORE)	+= machine_kexec_$(BITS).o
 obj-$(CONFIG_KEXEC_CORE)	+= relocate_kernel_$(BITS).o crash.o
 obj-$(CONFIG_KEXEC_FILE)	+= kexec-bzimage64.o
diff --git a/arch/x86/kernel/crash_core_32.c b/arch/x86/kernel/crash_core_32.c
new file mode 100644
index 000000000000..c0159a7bca6d
--- /dev/null
+++ b/arch/x86/kernel/crash_core_32.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/crash_core.h>
+
+#include <asm/pgtable.h>
+#include <asm/setup.h>
+
+void arch_crash_save_vmcoreinfo(void)
+{
+#ifdef CONFIG_NUMA
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+#ifdef CONFIG_X86_PAE
+	VMCOREINFO_CONFIG(X86_PAE);
+#endif
+}
diff --git a/arch/x86/kernel/crash_core_64.c b/arch/x86/kernel/crash_core_64.c
new file mode 100644
index 000000000000..845a57eb4eb7
--- /dev/null
+++ b/arch/x86/kernel/crash_core_64.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/crash_core.h>
+
+#include <asm/pgtable.h>
+#include <asm/setup.h>
+
+void arch_crash_save_vmcoreinfo(void)
+{
+	u64 sme_mask = sme_me_mask;
+
+	VMCOREINFO_NUMBER(phys_base);
+	VMCOREINFO_SYMBOL(init_top_pgt);
+	vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
+			      pgtable_l5_enabled());
+
+#ifdef CONFIG_NUMA
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+	vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
+	VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
+	VMCOREINFO_NUMBER(sme_mask);
+}
diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
index 7b45e8daad22..02bddfc122a4 100644
--- a/arch/x86/kernel/machine_kexec_32.c
+++ b/arch/x86/kernel/machine_kexec_32.c
@@ -250,15 +250,3 @@ void machine_kexec(struct kimage *image)
 
 	__ftrace_enabled_restore(save_ftrace_enabled);
 }
-
-void arch_crash_save_vmcoreinfo(void)
-{
-#ifdef CONFIG_NUMA
-	VMCOREINFO_SYMBOL(node_data);
-	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
-#endif
-#ifdef CONFIG_X86_PAE
-	VMCOREINFO_CONFIG(X86_PAE);
-#endif
-}
-
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 16e125a50b33..ad5cdd6a5f23 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -398,25 +398,6 @@ void machine_kexec(struct kimage *image)
 	__ftrace_enabled_restore(save_ftrace_enabled);
 }
 
-void arch_crash_save_vmcoreinfo(void)
-{
-	u64 sme_mask = sme_me_mask;
-
-	VMCOREINFO_NUMBER(phys_base);
-	VMCOREINFO_SYMBOL(init_top_pgt);
-	vmcoreinfo_append_str("NUMBER(pgtable_l5_enabled)=%d\n",
-			pgtable_l5_enabled());
-
-#ifdef CONFIG_NUMA
-	VMCOREINFO_SYMBOL(node_data);
-	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
-#endif
-	vmcoreinfo_append_str("KERNELOFFSET=%lx\n",
-			      kaslr_offset());
-	VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
-	VMCOREINFO_NUMBER(sme_mask);
-}
-
 /* arch-dependent functionality related to kexec file-based syscall */
 
 #ifdef CONFIG_KEXEC_FILE
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y
  2019-12-02 18:02 [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y Omar Sandoval
@ 2019-12-18 10:41 ` Borislav Petkov
  2019-12-19 20:28   ` Omar Sandoval
  0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2019-12-18 10:41 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel

On Mon, Dec 02, 2019 at 10:02:29AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> kernel/crash_core.c calls arch_crash_save_vmcoreinfo() to get
> arch-specific bits for vmcoreinfo. If it is not defined, then it has a
> no-op fallback. kernel/crash_core.c is gated behind CONFIG_CRASH_CORE.
> However, x86 defines arch_crash_save_vmcoreinfo() in
> arch/x86/kernel/machine_kexec_*.c, which is gated behind
> CONFIG_KEXEC_CORE. So, a kernel with CONFIG_CRASH_CORE=y and
> CONFIG_KEXEC_CORE=n

How does that even happen?

Symbol: KEXEC_CORE [=y]
Type  : bool
  Defined at arch/Kconfig:17
  Selects: CRASH_CORE [=y]
  Selected by [y]:
  - KEXEC [=y]
  - KEXEC_FILE [=y] && X86_64 [=y] && CRYPTO [=y]=y && CRYPTO_SHA256 [=y]=y

In order to do crash dumps, you need to select KEXEC, which selects
KEXEC_CORE, which selects CRASH_CORE...

Or are you talking about the PROC_KCORE use angle where it selects
CRASH_CORE and the crash_save_vmcoreinfo_init() initcall is then
supposed to save arch-specific crash info?

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y
  2019-12-18 10:41 ` Borislav Petkov
@ 2019-12-19 20:28   ` Omar Sandoval
  2019-12-20 10:05     ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Omar Sandoval @ 2019-12-19 20:28 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel

On Wed, Dec 18, 2019 at 11:41:13AM +0100, Borislav Petkov wrote:
> On Mon, Dec 02, 2019 at 10:02:29AM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > kernel/crash_core.c calls arch_crash_save_vmcoreinfo() to get
> > arch-specific bits for vmcoreinfo. If it is not defined, then it has a
> > no-op fallback. kernel/crash_core.c is gated behind CONFIG_CRASH_CORE.
> > However, x86 defines arch_crash_save_vmcoreinfo() in
> > arch/x86/kernel/machine_kexec_*.c, which is gated behind
> > CONFIG_KEXEC_CORE. So, a kernel with CONFIG_CRASH_CORE=y and
> > CONFIG_KEXEC_CORE=n
> 
> How does that even happen?
> 
> Symbol: KEXEC_CORE [=y]
> Type  : bool
>   Defined at arch/Kconfig:17
>   Selects: CRASH_CORE [=y]
>   Selected by [y]:
>   - KEXEC [=y]
>   - KEXEC_FILE [=y] && X86_64 [=y] && CRYPTO [=y]=y && CRYPTO_SHA256 [=y]=y
> 
> In order to do crash dumps, you need to select KEXEC, which selects
> KEXEC_CORE, which selects CRASH_CORE...
> 
> Or are you talking about the PROC_KCORE use angle where it selects
> CRASH_CORE and the crash_save_vmcoreinfo_init() initcall is then
> supposed to save arch-specific crash info?

Yes, I'm talking about reading VMCOREINFO from /proc/kcore at runtime,
no kdump involved. crash [1] and my own tool, drgn [2], use this for
live debugging.

I can set CONFIG_PROC_KCORE=y, which selects CONFIG_CRASH_CORE=y, but if
CONFIG_KEXEC_CORE=n, the VMCOREINFO is incomplete.

1: https://github.com/crash-utility/crash/commit/60a42d709280cdf38ab06327a5b4fa9d9208ef86
2: https://github.com/osandov/drgn/blob/73dd7def1217e24cc83d8ca95c995decbd9ba24c/libdrgn/program.c#L385

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y
  2019-12-19 20:28   ` Omar Sandoval
@ 2019-12-20 10:05     ` Borislav Petkov
  0 siblings, 0 replies; 4+ messages in thread
From: Borislav Petkov @ 2019-12-20 10:05 UTC (permalink / raw)
  To: Omar Sandoval; +Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel

On Thu, Dec 19, 2019 at 12:28:07PM -0800, Omar Sandoval wrote:
> Yes, I'm talking about reading VMCOREINFO from /proc/kcore at runtime,
> no kdump involved. crash [1] and my own tool, drgn [2], use this for
> live debugging.

Ok, please state the gist of the use cases in the commit message so that
it is clear why you're doing this.

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-12-20 10:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 18:02 [PATCH] x86: define arch_crash_save_vmcoreinfo() if CONFIG_CRASH_CORE=y Omar Sandoval
2019-12-18 10:41 ` Borislav Petkov
2019-12-19 20:28   ` Omar Sandoval
2019-12-20 10:05     ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).