Commit 5c99b692 authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Ingo Molnar

x86/unwind: Use MSB for frame pointer encoding on 32-bit

On x86-32, Tetsuo Handa and Fengguang Wu reported unwinder warnings
like:

  WARNING: kernel stack regs at f60bb9c8 in swapper:1 has bad 'bp' value 0ba00000

And also there were some stack dumps with a bunch of unreliable '?'
symbols after an apic_timer_interrupt symbol, meaning the unwinder got
confused when it tried to read the regs.

The cause of those issues is that, with GCC 4.8 (and possibly older),
there are cases where GCC misaligns the stack pointer in a leaf function
for no apparent reason:

  c124a388 <acpi_rs_move_data>:
  c124a388:       55                      push   %ebp
  c124a389:       89 e5                   mov    %esp,%ebp
  c124a38b:       57                      push   %edi
  c124a38c:       56                      push   %esi
  c124a38d:       89 d6                   mov    %edx,%esi
  c124a38f:       53                      push   %ebx
  c124a390:       31 db                   xor    %ebx,%ebx
  c124a392:       83 ec 03                sub    $0x3,%esp
  ...
  c124a3e3:       83 c4 03                add    $0x3,%esp
  c124a3e6:       5b                      pop    %ebx
  c124a3e7:       5e                      pop    %esi
  c124a3e8:       5f                      pop    %edi
  c124a3e9:       5d                      pop    %ebp
  c124a3ea:       c3                      ret

If an interrupt occurs in such a function, the regs on the stack will be
unaligned, which breaks the frame pointer encoding assumption.  So on
32-bit, use the MSB instead of the LSB to encode the regs.

This isn't an issue on 64-bit, because interrupts align the stack before
writing to it.
Reported-and-tested-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-and-tested-by: default avatarFengguang Wu <fengguang.wu@intel.com>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: LKP <lkp@01.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/279a26996a482ca716605c7dbc7f2db9d8d91e81.1507597785.git.jpoimboe@redhat.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 62dd86ac
...@@ -176,7 +176,7 @@ ...@@ -176,7 +176,7 @@
/* /*
* This is a sneaky trick to help the unwinder find pt_regs on the stack. The * This is a sneaky trick to help the unwinder find pt_regs on the stack. The
* frame pointer is replaced with an encoded pointer to pt_regs. The encoding * frame pointer is replaced with an encoded pointer to pt_regs. The encoding
* is just setting the LSB, which makes it an invalid stack address and is also * is just clearing the MSB, which makes it an invalid stack address and is also
* a signal to the unwinder that it's a pt_regs pointer in disguise. * a signal to the unwinder that it's a pt_regs pointer in disguise.
* *
* NOTE: This macro must be used *after* SAVE_ALL because it corrupts the * NOTE: This macro must be used *after* SAVE_ALL because it corrupts the
...@@ -185,7 +185,7 @@ ...@@ -185,7 +185,7 @@
.macro ENCODE_FRAME_POINTER .macro ENCODE_FRAME_POINTER
#ifdef CONFIG_FRAME_POINTER #ifdef CONFIG_FRAME_POINTER
mov %esp, %ebp mov %esp, %ebp
orl $0x1, %ebp andl $0x7fffffff, %ebp
#endif #endif
.endm .endm
......
...@@ -174,6 +174,7 @@ static bool is_last_task_frame(struct unwind_state *state) ...@@ -174,6 +174,7 @@ static bool is_last_task_frame(struct unwind_state *state)
* This determines if the frame pointer actually contains an encoded pointer to * This determines if the frame pointer actually contains an encoded pointer to
* pt_regs on the stack. See ENCODE_FRAME_POINTER. * pt_regs on the stack. See ENCODE_FRAME_POINTER.
*/ */
#ifdef CONFIG_X86_64
static struct pt_regs *decode_frame_pointer(unsigned long *bp) static struct pt_regs *decode_frame_pointer(unsigned long *bp)
{ {
unsigned long regs = (unsigned long)bp; unsigned long regs = (unsigned long)bp;
...@@ -183,6 +184,17 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp) ...@@ -183,6 +184,17 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
return (struct pt_regs *)(regs & ~0x1); return (struct pt_regs *)(regs & ~0x1);
} }
#else
static struct pt_regs *decode_frame_pointer(unsigned long *bp)
{
unsigned long regs = (unsigned long)bp;
if (regs & 0x80000000)
return NULL;
return (struct pt_regs *)(regs | 0x80000000);
}
#endif
#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
#define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long)) #define KERNEL_REGS_SIZE (sizeof(struct pt_regs) - 2*sizeof(long))
......
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