Commit 519fb5c3 authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Ingo Molnar

x86/unwind: Add end-of-stack check for ftrace handlers

Dave Jones and Steven Rostedt reported unwinder warnings like the
following:

  WARNING: kernel stack frame pointer at ffff8800bda0ff30 in sshd:1090 has bad value 000055b32abf1fa8

In both cases, the unwinder was attempting to unwind from an ftrace
handler into entry code.  The callchain was something like:

  syscall entry code
    C function
      ftrace handler
        save_stack_trace()

The problem is that the unwinder's end-of-stack logic gets confused by
the way ftrace lays out the stack frame (with fentry enabled).

I was able to recreate this warning with:

  echo call_usermodehelper_exec_async:stacktrace > /sys/kernel/debug/tracing/set_ftrace_filter
  (exit login session)

I considered fixing this by changing the ftrace code to rewrite the
stack to make the unwinder happy.  But that seemed too intrusive after I
implemented it.  Instead, just add another check to the unwinder's
end-of-stack logic to detect this special case.

Side note: We could probably get rid of these end-of-stack checks by
encoding the frame pointer for syscall entry just like we do for
interrupt entry.  That would be simpler, but it would also be a lot more
intrusive since it would slightly affect the performance of every
syscall.
Reported-by: default avatarDave Jones <davej@codemonkey.org.uk>
Reported-by: default avatarSteven Rostedt <rostedt@goodmis.org>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Acked-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: live-patching@vger.kernel.org
Fixes: c32c47c6 ("x86/unwind: Warn on bad frame pointer")
Link: http://lkml.kernel.org/r/671ba22fbc0156b8f7e0cfa5ab2a795e08bc37e1.1495553739.git.jpoimboe@redhat.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent ebd57499
...@@ -104,6 +104,11 @@ static inline unsigned long *last_frame(struct unwind_state *state) ...@@ -104,6 +104,11 @@ static inline unsigned long *last_frame(struct unwind_state *state)
return (unsigned long *)task_pt_regs(state->task) - 2; return (unsigned long *)task_pt_regs(state->task) - 2;
} }
static bool is_last_frame(struct unwind_state *state)
{
return state->bp == last_frame(state);
}
#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
#define GCC_REALIGN_WORDS 3 #define GCC_REALIGN_WORDS 3
#else #else
...@@ -115,16 +120,15 @@ static inline unsigned long *last_aligned_frame(struct unwind_state *state) ...@@ -115,16 +120,15 @@ static inline unsigned long *last_aligned_frame(struct unwind_state *state)
return last_frame(state) - GCC_REALIGN_WORDS; return last_frame(state) - GCC_REALIGN_WORDS;
} }
static bool is_last_task_frame(struct unwind_state *state) static bool is_last_aligned_frame(struct unwind_state *state)
{ {
unsigned long *last_bp = last_frame(state); unsigned long *last_bp = last_frame(state);
unsigned long *aligned_bp = last_aligned_frame(state); unsigned long *aligned_bp = last_aligned_frame(state);
/* /*
* We have to check for the last task frame at two different locations * GCC can occasionally decide to realign the stack pointer and change
* because gcc can occasionally decide to realign the stack pointer and * the offset of the stack frame in the prologue of a function called
* change the offset of the stack frame in the prologue of a function * by head/entry code. Examples:
* called by head/entry code. Examples:
* *
* <start_secondary>: * <start_secondary>:
* push %edi * push %edi
...@@ -141,11 +145,38 @@ static bool is_last_task_frame(struct unwind_state *state) ...@@ -141,11 +145,38 @@ static bool is_last_task_frame(struct unwind_state *state)
* push %rbp * push %rbp
* mov %rsp,%rbp * mov %rsp,%rbp
* *
* Note that after aligning the stack, it pushes a duplicate copy of * After aligning the stack, it pushes a duplicate copy of the return
* the return address before pushing the frame pointer. * address before pushing the frame pointer.
*/
return (state->bp == aligned_bp && *(aligned_bp + 1) == *(last_bp + 1));
}
static bool is_last_ftrace_frame(struct unwind_state *state)
{
unsigned long *last_bp = last_frame(state);
unsigned long *last_ftrace_bp = last_bp - 3;
/*
* When unwinding from an ftrace handler of a function called by entry
* code, the stack layout of the last frame is:
*
* bp
* parent ret addr
* bp
* function ret addr
* parent ret addr
* pt_regs
* -----------------
*/ */
return (state->bp == last_bp || return (state->bp == last_ftrace_bp &&
(state->bp == aligned_bp && *(aligned_bp+1) == *(last_bp+1))); *state->bp == *(state->bp + 2) &&
*(state->bp + 1) == *(state->bp + 4));
}
static bool is_last_task_frame(struct unwind_state *state)
{
return is_last_frame(state) || is_last_aligned_frame(state) ||
is_last_ftrace_frame(state);
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment