git: 2310894c1021 - stable/13 - kerneldump: Inline dump_savectx() into its callers

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Wed, 22 Mar 2023 13:17:23 UTC
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=2310894c1021f49b6c3003ba215ca978eee2be75

commit 2310894c1021f49b6c3003ba215ca978eee2be75
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-03-20 18:16:00 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-03-22 13:04:19 +0000

    kerneldump: Inline dump_savectx() into its callers
    
    The callers of dump_savectx() (i.e., doadump() and livedump_start())
    subsequently call dumpsys()/minidumpsys(), which dump the calling
    thread's stack when writing the dump.  If dump_savectx() gets its own
    stack frame, that frame might be clobbered when its caller later calls
    dumpsys()/minidumpsys(), making it difficult for debuggers to unwind the
    stack.
    
    Fix this by making dump_savectx() a macro, so that savectx() is always
    called directly by the function which subsequently calls
    dumpsys()/minidumpsys().
    
    This fixes stack unwinding for the panicking thread from arm64
    minidumps.  The same happened to work on amd64, but kgdb reports the
    dump_savectx() calls as coming from dumpsys(), so in that case it
    appears to work by accident.
    
    Fixes:  c9114f9f86f9 ("Add new vnode dumper to support live minidumps")
    Reviewed by:    mhorne, jhb
    MFC after:      3 days
    Differential Revision:  https://reviews.freebsd.org/D39151
    
    (cherry picked from commit c3179891f897d840f578a5139839fcacb587c96d)
---
 sys/kern/kern_shutdown.c    | 15 ++-------------
 sys/kern/kern_vnodedumper.c |  1 +
 sys/sys/conf.h              | 16 +++++++++++++++-
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c
index ee6faa222206..cf9411420304 100644
--- a/sys/kern/kern_shutdown.c
+++ b/sys/kern/kern_shutdown.c
@@ -239,8 +239,8 @@ MTX_SYSINIT(dumper_configs, &dumpconf_list_lk, "dumper config list", MTX_DEF);
 static TAILQ_HEAD(dumpconflist, dumperinfo) dumper_configs =
     TAILQ_HEAD_INITIALIZER(dumper_configs);
 
-/* Context information for dump-debuggers. */
-static struct pcb dumppcb;		/* Registers. */
+/* Context information for dump-debuggers, saved by the dump_savectx() macro. */
+struct pcb dumppcb;			/* Registers. */
 lwpid_t dumptid;			/* Thread ID. */
 
 static struct cdevsw reroot_cdevsw = {
@@ -381,17 +381,6 @@ print_uptime(void)
 	printf("%lds\n", (long)ts.tv_sec);
 }
 
-/*
- * Set up a context that can be extracted from the dump.
- */
-void
-dump_savectx(void)
-{
-
-	savectx(&dumppcb);
-	dumptid = curthread->td_tid;
-}
-
 int
 doadump(boolean_t textdump)
 {
diff --git a/sys/kern/kern_vnodedumper.c b/sys/kern/kern_vnodedumper.c
index 26154af20372..0104369b9d67 100644
--- a/sys/kern/kern_vnodedumper.c
+++ b/sys/kern/kern_vnodedumper.c
@@ -44,6 +44,7 @@
 #include <sys/sysctl.h>
 #include <sys/vnode.h>
 
+#include <machine/pcb.h>
 #include <machine/vmparam.h>
 
 static dumper_start_t vnode_dumper_start;
diff --git a/sys/sys/conf.h b/sys/sys/conf.h
index 9f60dcaf62fa..b75af5a7705f 100644
--- a/sys/sys/conf.h
+++ b/sys/sys/conf.h
@@ -361,7 +361,21 @@ struct dumperinfo {
 
 extern int dumping;		/* system is dumping */
 
-void dump_savectx(void);
+/*
+ * Save registers for later extraction from a kernel dump.
+ *
+ * This must be inlined into the caller, which in turn must be the function that
+ * calls (mini)dumpsys().  Otherwise, the saved frame pointer will reference a
+ * stack frame that may be clobbered by subsequent function calls.
+ */
+#define	dump_savectx() do {		\
+	extern struct pcb dumppcb;	\
+	extern lwpid_t dumptid;		\
+					\
+	savectx(&dumppcb);		\
+	dumptid = curthread->td_tid;	\
+} while (0)
+
 int doadump(boolean_t);
 struct diocskerneldump_arg;
 int dumper_create(const struct dumperinfo *di_template, const char *devname,