x86/dumpstack: Fix partial register dumps

The show_regs_safe() logic is wrong.  When there's an iret stack frame,
it prints the entire pt_regs -- most of which is random stack data --
instead of just the five registers at the end.

show_regs_safe() is also poorly named: the on_stack() checks aren't for
safety.  Rename the function to show_regs_if_on_stack() and add a
comment to explain why the checks are needed.

These issues were introduced with the "partial register dump" feature of
the following commit:

  b02fcf9ba121 ("x86/unwinder: Handle stack overflows more gracefully")

That patch had gone through a few iterations of development, and the
above issues were artifacts from a previous iteration of the patch where
'regs' pointed directly to the iret frame rather than to the (partially
empty) pt_regs.

Tested-by: Alexander Tsoy <alexander@tsoy.me>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Toralf Förster <toralf.foerster@gmx.de>
Cc: stable@vger.kernel.org
Fixes: b02fcf9ba121 ("x86/unwinder: Handle stack overflows more gracefully")
Link: http://lkml.kernel.org/r/5b05b8b344f59db2d3d50dbdeba92d60f2304c54.1514736742.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index c1688c2d0a128f063053697dc60bcbfbca509765..1f86e1b0a5cdc1afeb4667e8c770ec81456e9fb4 100644 (file)
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -56,18 +56,27 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,
 
 #if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
 /*
- * WARNING: The entire pt_regs may not be safe to dereference.  In some cases,
- * only the iret frame registers are accessible.  Use with caution!
+ * If 'partial' returns true, only the iret frame registers are valid.
  */
-static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
+static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
+                                                   bool *partial)
 {
        if (unwind_done(state))
                return NULL;
 
+       if (partial) {
+#ifdef CONFIG_UNWINDER_ORC
+               *partial = !state->full_regs;
+#else
+               *partial = false;
+#endif
+       }
+
        return state->regs;
 }
 #else
-static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
+static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
+                                                   bool *partial)
 {
        return NULL;
 }

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 5fa110699ed275fe81dc9d8f678a2a2f8902017e..d0bb176a7261ae85c09580ec4dd103923ae0348a 100644 (file)
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -76,12 +76,23 @@ void show_iret_regs(struct pt_regs *regs)
                regs->sp, regs->flags);
 }
 
-static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)
+static void show_regs_if_on_stack(struct stack_info *info, struct pt_regs *regs,
+                                 bool partial)
 {
-       if (on_stack(info, regs, sizeof(*regs)))
+       /*
+        * These on_stack() checks aren't strictly necessary: the unwind code
+        * has already validated the 'regs' pointer.  The checks are done for
+        * ordering reasons: if the registers are on the next stack, we don't
+        * want to print them out yet.  Otherwise they'll be shown as part of
+        * the wrong stack.  Later, when show_trace_log_lvl() switches to the
+        * next stack, this function will be called again with the same regs so
+        * they can be printed in the right context.
+        */
+       if (!partial && on_stack(info, regs, sizeof(*regs))) {
                __show_regs(regs, 0);
-       else if (on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
-                         IRET_FRAME_SIZE)) {
+
+       } else if (partial && on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
+                                      IRET_FRAME_SIZE)) {
                /*
                 * When an interrupt or exception occurs in entry code, the
                 * full pt_regs might not have been saved yet.  In that case
@@ -98,6 +109,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
        struct stack_info stack_info = {0};
        unsigned long visit_mask = 0;
        int graph_idx = 0;
+       bool partial;
 
        printk("%sCall Trace:\n", log_lvl);
 
@@ -140,7 +152,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
                        printk("%s <%s>\n", log_lvl, stack_name);
 
                if (regs)
-                       show_regs_safe(&stack_info, regs);
+                       show_regs_if_on_stack(&stack_info, regs, partial);
 
                /*
                 * Scan the stack, printing any text addresses we find.  At the
@@ -164,7 +176,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 
                        /*
                         * Don't print regs->ip again if it was already printed
-                        * by show_regs_safe() below.
+                        * by show_regs_if_on_stack().
                         */
                        if (regs && stack == &regs->ip)
                                goto next;
@@ -199,9 +211,9 @@ next:
                        unwind_next_frame(&state);
 
                        /* if the frame has entry regs, print them */
-                       regs = unwind_get_entry_regs(&state);
+                       regs = unwind_get_entry_regs(&state, &partial);
                        if (regs)
-                               show_regs_safe(&stack_info, regs);
+                               show_regs_if_on_stack(&stack_info, regs, partial);
                }
 
                if (stack_name)

diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 8dabd7bf16730918f1a8c91fcd5df503d73b056a..60244bfaf88f69c91732929ddda712767e1ff030 100644 (file)
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -98,7 +98,7 @@ static int __save_stack_trace_reliable(struct stack_trace *trace,
        for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
             unwind_next_frame(&state)) {
 
-               regs = unwind_get_entry_regs(&state);
+               regs = unwind_get_entry_regs(&state, NULL);
                if (regs) {
                        /*
                         * Kernel mode registers on the stack indicate an