[PATCH 0/3] x86/unwind: silence entry-related warnings

[PATCH 0/3] x86/unwind: silence entry-related warnings

[Josh Poimboeuf]

This fixes a rare unwinder warning which is seen when unwinding from an
interrupt which occurred in entry code.  The first two patches are
cleanups which are needed for the fix in the third patch.

Josh Poimboeuf (3):
  x86/unwind: move common code into update_stack_state()
  x86/unwind: read stack return address in update_stack_state()
  x86/unwind: silence entry-related warnings

 arch/x86/include/asm/unwind.h  |   2 +
 arch/x86/kernel/unwind_frame.c | 176 ++++++++++++++++++++++-------------------
 2 files changed, 95 insertions(+), 83 deletions(-)

-- 
2.7.4

[PATCH 1/3] x86/unwind: move common code into update_stack_state()

[Josh Poimboeuf]

The __unwind_start() and unwind_next_frame() functions have some
duplicated functionality.  They both call decode_frame_pointer() and set
state->regs and state->bp accordingly.  Move that functionality to a
common place in update_stack_state().

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/unwind_frame.c | 119 +++++++++++++++++++----------------------
 1 file changed, 55 insertions(+), 64 deletions(-)

diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 0833926..9098ef1 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -135,26 +135,59 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 	return (struct pt_regs *)(regs & ~0x1);
 }
 
-static bool update_stack_state(struct unwind_state *state, void *addr,
-			       size_t len)
+static bool update_stack_state(struct unwind_state *state,
+			       unsigned long *next_bp)
 {
 	struct stack_info *info = &state->stack_info;
-	enum stack_type orig_type = info->type;
+	enum stack_type prev_type = info->type;
+	struct pt_regs *regs;
+	unsigned long *frame, *prev_frame_end;
+	size_t len;
+
+	if (state->regs)
+		prev_frame_end = (void *)state->regs + regs_size(state->regs);
+	else
+		prev_frame_end = (void *)state->bp + FRAME_HEADER_SIZE;
+
+	/* Is the next frame pointer an encoded pointer to pt_regs? */
+	regs = decode_frame_pointer(next_bp);
+	if (regs) {
+		frame = (unsigned long *)regs;
+		len = regs_size(regs);
+	} else {
+		frame = next_bp;
+		len = FRAME_HEADER_SIZE;
+	}
 
 	/*
-	 * If addr isn't on the current stack, switch to the next one.
+	 * If the next bp isn't on the current stack, switch to the next one.
 	 *
 	 * We may have to traverse multiple stacks to deal with the possibility
-	 * that 'info->next_sp' could point to an empty stack and 'addr' could
-	 * be on a subsequent stack.
+	 * that info->next_sp could point to an empty stack and the next bp
+	 * could be on a subsequent stack.
 	 */
-	while (!on_stack(info, addr, len))
+	while (!on_stack(info, frame, len))
 		if (get_stack_info(info->next_sp, state->task, info,
 				   &state->stack_mask))
 			return false;
 
-	if (!state->orig_sp || info->type != orig_type)
-		state->orig_sp = addr;
+	/* Make sure it only unwinds up and doesn't overlap the prev frame: */
+	if (state->orig_sp && state->stack_info.type == prev_type &&
+	    frame < prev_frame_end)
+		return false;
+
+	/* Move state to the next frame: */
+	if (regs) {
+		state->regs = regs;
+		state->bp = NULL;
+	} else {
+		state->bp = next_bp;
+		state->regs = NULL;
+	}
+
+	/* Save the original stack pointer for unwind_dump(): */
+	if (!state->orig_sp || info->type != prev_type)
+		state->orig_sp = frame;
 
 	return true;
 }
@@ -162,14 +195,12 @@ static bool update_stack_state(struct unwind_state *state, void *addr,
 bool unwind_next_frame(struct unwind_state *state)
 {
 	struct pt_regs *regs;
-	unsigned long *next_bp, *next_frame;
-	size_t next_len;
-	enum stack_type prev_type = state->stack_info.type;
+	unsigned long *next_bp;
 
 	if (unwind_done(state))
 		return false;
 
-	/* have we reached the end? */
+	/* Have we reached the end? */
 	if (state->regs && user_mode(state->regs))
 		goto the_end;
 
@@ -200,24 +231,14 @@ bool unwind_next_frame(struct unwind_state *state)
 		return true;
 	}
 
-	/* get the next frame pointer */
+	/* Get the next frame pointer: */
 	if (state->regs)
 		next_bp = (unsigned long *)state->regs->bp;
 	else
-		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task,*state->bp);
+		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);
 
-	/* is the next frame pointer an encoded pointer to pt_regs? */
-	regs = decode_frame_pointer(next_bp);
-	if (regs) {
-		next_frame = (unsigned long *)regs;
-		next_len = sizeof(*regs);
-	} else {
-		next_frame = next_bp;
-		next_len = FRAME_HEADER_SIZE;
-	}
-
-	/* make sure the next frame's data is accessible */
-	if (!update_stack_state(state, next_frame, next_len)) {
+	/* Move to the next frame if it's safe: */
+	if (!update_stack_state(state, next_bp)) {
 		/*
 		 * Don't warn on bad regs->bp.  An interrupt in entry code
 		 * might cause a false positive warning.
@@ -228,24 +249,6 @@ bool unwind_next_frame(struct unwind_state *state)
 		goto bad_address;
 	}
 
-	/* Make sure it only unwinds up and doesn't overlap the last frame: */
-	if (state->stack_info.type == prev_type) {
-		if (state->regs && (void *)next_frame < (void *)state->regs + regs_size(state->regs))
-			goto bad_address;
-
-		if (state->bp && (void *)next_frame < (void *)state->bp + FRAME_HEADER_SIZE)
-			goto bad_address;
-	}
-
-	/* move to the next frame */
-	if (regs) {
-		state->regs = regs;
-		state->bp = NULL;
-	} else {
-		state->bp = next_bp;
-		state->regs = NULL;
-	}
-
 	return true;
 
 bad_address:
@@ -263,13 +266,13 @@ bool unwind_next_frame(struct unwind_state *state)
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",
 			state->regs, state->task->comm,
-			state->task->pid, next_frame);
+			state->task->pid, next_bp);
 		unwind_dump(state, (unsigned long *)state->regs);
 	} else {
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack frame pointer at %p in %s:%d has bad value %p\n",
 			state->bp, state->task->comm,
-			state->task->pid, next_frame);
+			state->task->pid, next_bp);
 		unwind_dump(state, state->bp);
 	}
 the_end:
@@ -281,35 +284,23 @@ EXPORT_SYMBOL_GPL(unwind_next_frame);
 void __unwind_start(struct unwind_state *state, struct task_struct *task,
 		    struct pt_regs *regs, unsigned long *first_frame)
 {
-	unsigned long *bp, *frame;
-	size_t len;
+	unsigned long *bp;
 
 	memset(state, 0, sizeof(*state));
 	state->task = task;
 
-	/* don't even attempt to start from user mode regs */
+	/* Don't even attempt to start from user mode regs: */
 	if (regs && user_mode(regs)) {
 		state->stack_info.type = STACK_TYPE_UNKNOWN;
 		return;
 	}
 
-	/* set up the starting stack frame */
 	bp = get_frame_pointer(task, regs);
-	regs = decode_frame_pointer(bp);
-	if (regs) {
-		state->regs = regs;
-		frame = (unsigned long *)regs;
-		len = sizeof(*regs);
-	} else {
-		state->bp = bp;
-		frame = bp;
-		len = FRAME_HEADER_SIZE;
-	}
 
-	/* initialize stack info and make sure the frame data is accessible */
-	get_stack_info(frame, state->task, &state->stack_info,
+	/* Initialize stack info and make sure the frame data is accessible: */
+	get_stack_info(bp, state->task, &state->stack_info,
 		       &state->stack_mask);
-	update_stack_state(state, frame, len);
+	update_stack_state(state, bp);
 
 	/*
 	 * The caller can provide the address of the first frame directly
-- 
2.7.4

[PATCH 2/3] x86/unwind: read stack return address in update_stack_state()

[Josh Poimboeuf]

Instead of reading the return address when unwind_get_return_address()
is called, read it from update_stack_state() and store it in the unwind
state.  This enables the next patch to check the return address from
unwind_next_frame() so it can detect an entry code frame.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/unwind.h  |  1 +
 arch/x86/kernel/unwind_frame.c | 25 +++++++++++++------------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 6fa75b1..5663425 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -14,6 +14,7 @@ struct unwind_state {
 #ifdef CONFIG_FRAME_POINTER
 	unsigned long *bp, *orig_sp;
 	struct pt_regs *regs;
+	unsigned long ip;
 #else
 	unsigned long *sp;
 #endif
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 9098ef1..c67a059 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -56,20 +56,10 @@ static void unwind_dump(struct unwind_state *state, unsigned long *sp)
 
 unsigned long unwind_get_return_address(struct unwind_state *state)
 {
-	unsigned long addr;
-	unsigned long *addr_p = unwind_get_return_address_ptr(state);
-
 	if (unwind_done(state))
 		return 0;
 
-	if (state->regs && user_mode(state->regs))
-		return 0;
-
-	addr = READ_ONCE_TASK_STACK(state->task, *addr_p);
-	addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, addr,
-				     addr_p);
-
-	return __kernel_text_address(addr) ? addr : 0;
+	return __kernel_text_address(state->ip) ? state->ip : 0;
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
@@ -141,7 +131,7 @@ static bool update_stack_state(struct unwind_state *state,
 	struct stack_info *info = &state->stack_info;
 	enum stack_type prev_type = info->type;
 	struct pt_regs *regs;
-	unsigned long *frame, *prev_frame_end;
+	unsigned long *frame, *prev_frame_end, *addr_p, addr;
 	size_t len;
 
 	if (state->regs)
@@ -185,6 +175,16 @@ static bool update_stack_state(struct unwind_state *state,
 		state->regs = NULL;
 	}
 
+	/* Save the return address: */
+	if (state->regs && user_mode(state->regs))
+		state->ip = 0;
+	else {
+		addr_p = unwind_get_return_address_ptr(state);
+		addr = READ_ONCE_TASK_STACK(state->task, *addr_p);
+		state->ip = ftrace_graph_ret_addr(state->task, &state->graph_idx,
+						  addr, addr_p);
+	}
+
 	/* Save the original stack pointer for unwind_dump(): */
 	if (!state->orig_sp || info->type != prev_type)
 		state->orig_sp = frame;
@@ -228,6 +228,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		 */
 		state->regs = regs;
 		state->bp = NULL;
+		state->ip = 0;
 		return true;
 	}
 
-- 
2.7.4

[PATCH 3/3] x86/unwind: silence entry-related warnings

[Josh Poimboeuf]

A few people have reported unwinder warnings like the following:

  WARNING: kernel stack frame pointer at ffffc90000fe7ff0 in rsync:1157 has bad value           (null)
  unwind stack type:0 next_sp:          (null) mask:2 graph_idx:0
  ffffc90000fe7f98: ffffc90000fe7ff0 (0xffffc90000fe7ff0)
  ffffc90000fe7fa0: ffffffffb7000f56 (trace_hardirqs_off_thunk+0x1a/0x1c)
  ffffc90000fe7fa8: 0000000000000246 (0x246)
  ffffc90000fe7fb0: 0000000000000000 ...
  ffffc90000fe7fc0: 00007ffe3af639bc (0x7ffe3af639bc)
  ffffc90000fe7fc8: 0000000000000006 (0x6)
  ffffc90000fe7fd0: 00007f80af433fc5 (0x7f80af433fc5)
  ffffc90000fe7fd8: 00007ffe3af638e0 (0x7ffe3af638e0)
  ffffc90000fe7fe0: 00007ffe3af638e0 (0x7ffe3af638e0)
  ffffc90000fe7fe8: 00007ffe3af63970 (0x7ffe3af63970)
  ffffc90000fe7ff0: 0000000000000000 ...
  ffffc90000fe7ff8: ffffffffb7b74b9a (entry_SYSCALL_64_after_swapgs+0x17/0x4f)

This warning can happen when unwinding a code path where an interrupt
occurred in x86 entry code before it set up the first stack frame.
Silently ignore any warnings for this case.

Fixes: c32c47c68a0a ("x86/unwind: Warn on bad frame pointer")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/unwind.h  |  1 +
 arch/x86/kernel/unwind_frame.c | 36 +++++++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 5663425..9b10dcd 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -12,6 +12,7 @@ struct unwind_state {
 	struct task_struct *task;
 	int graph_idx;
 #ifdef CONFIG_FRAME_POINTER
+	bool got_irq;
 	unsigned long *bp, *orig_sp;
 	struct pt_regs *regs;
 	unsigned long ip;
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index c67a059..c461135 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -1,6 +1,8 @@
 #include <linux/sched.h>
 #include <linux/sched/task.h>
 #include <linux/sched/task_stack.h>
+#include <linux/interrupt.h>
+#include <asm/sections.h>
 #include <asm/ptrace.h>
 #include <asm/bitops.h>
 #include <asm/stacktrace.h>
@@ -72,6 +74,21 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
+static bool in_entry_code(unsigned long ip)
+{
+	char *addr = (char *)ip;
+
+	if (addr >= __entry_text_start && addr < __entry_text_end)
+		return true;
+
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
+	if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
+		return true;
+#endif
+
+	return false;
+}
+
 #ifdef CONFIG_X86_32
 #define GCC_REALIGN_WORDS 3
 #else
@@ -144,6 +161,7 @@ static bool update_stack_state(struct unwind_state *state,
 	if (regs) {
 		frame = (unsigned long *)regs;
 		len = regs_size(regs);
+		state->got_irq = true;
 	} else {
 		frame = next_bp;
 		len = FRAME_HEADER_SIZE;
@@ -239,16 +257,8 @@ bool unwind_next_frame(struct unwind_state *state)
 		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);
 
 	/* Move to the next frame if it's safe: */
-	if (!update_stack_state(state, next_bp)) {
-		/*
-		 * Don't warn on bad regs->bp.  An interrupt in entry code
-		 * might cause a false positive warning.
-		 */
-		if (state->regs)
-			goto the_end;
-
+	if (!update_stack_state(state, next_bp))
 		goto bad_address;
-	}
 
 	return true;
 
@@ -263,6 +273,13 @@ bool unwind_next_frame(struct unwind_state *state)
 	if (state->task != current)
 		goto the_end;
 
+	/*
+	 * Don't warn if the unwinder got lost due to an interrupt in entry
+	 * code before the stack was set up:
+	 */
+	if (state->got_irq && in_entry_code(state->ip))
+		goto the_end;
+
 	if (state->regs) {
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",
@@ -289,6 +306,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task,
 
 	memset(state, 0, sizeof(*state));
 	state->task = task;
+	state->got_irq = (regs);
 
 	/* Don't even attempt to start from user mode regs: */
 	if (regs && user_mode(regs)) {
-- 
2.7.4

[tip:x86/asm] x86/unwind: Move common code into update_stack_state()

[tip-bot for Josh Poimboeuf]

Commit-ID:  5ed8d8bb38c5dcd78de540182cedb0fb19399aab
Gitweb:     http://git.kernel.org/tip/5ed8d8bb38c5dcd78de540182cedb0fb19399aab
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Wed, 12 Apr 2017 13:47:10 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 14 Apr 2017 10:19:49 +0200

x86/unwind: Move common code into update_stack_state()

The __unwind_start() and unwind_next_frame() functions have some
duplicated functionality.  They both call decode_frame_pointer() and set
state->regs and state->bp accordingly.  Move that functionality to a
common place in update_stack_state().

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Dave Jones <davej@codemonkey.org.uk>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/a2ee4801113f6d2300d58f08f6b69f85edf4eb43.1492020577.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/unwind_frame.c | 119 +++++++++++++++++++----------------------
 1 file changed, 55 insertions(+), 64 deletions(-)

diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 0833926..9098ef1 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -135,26 +135,59 @@ static struct pt_regs *decode_frame_pointer(unsigned long *bp)
 	return (struct pt_regs *)(regs & ~0x1);
 }
 
-static bool update_stack_state(struct unwind_state *state, void *addr,
-			       size_t len)
+static bool update_stack_state(struct unwind_state *state,
+			       unsigned long *next_bp)
 {
 	struct stack_info *info = &state->stack_info;
-	enum stack_type orig_type = info->type;
+	enum stack_type prev_type = info->type;
+	struct pt_regs *regs;
+	unsigned long *frame, *prev_frame_end;
+	size_t len;
+
+	if (state->regs)
+		prev_frame_end = (void *)state->regs + regs_size(state->regs);
+	else
+		prev_frame_end = (void *)state->bp + FRAME_HEADER_SIZE;
+
+	/* Is the next frame pointer an encoded pointer to pt_regs? */
+	regs = decode_frame_pointer(next_bp);
+	if (regs) {
+		frame = (unsigned long *)regs;
+		len = regs_size(regs);
+	} else {
+		frame = next_bp;
+		len = FRAME_HEADER_SIZE;
+	}
 
 	/*
-	 * If addr isn't on the current stack, switch to the next one.
+	 * If the next bp isn't on the current stack, switch to the next one.
 	 *
 	 * We may have to traverse multiple stacks to deal with the possibility
-	 * that 'info->next_sp' could point to an empty stack and 'addr' could
-	 * be on a subsequent stack.
+	 * that info->next_sp could point to an empty stack and the next bp
+	 * could be on a subsequent stack.
 	 */
-	while (!on_stack(info, addr, len))
+	while (!on_stack(info, frame, len))
 		if (get_stack_info(info->next_sp, state->task, info,
 				   &state->stack_mask))
 			return false;
 
-	if (!state->orig_sp || info->type != orig_type)
-		state->orig_sp = addr;
+	/* Make sure it only unwinds up and doesn't overlap the prev frame: */
+	if (state->orig_sp && state->stack_info.type == prev_type &&
+	    frame < prev_frame_end)
+		return false;
+
+	/* Move state to the next frame: */
+	if (regs) {
+		state->regs = regs;
+		state->bp = NULL;
+	} else {
+		state->bp = next_bp;
+		state->regs = NULL;
+	}
+
+	/* Save the original stack pointer for unwind_dump(): */
+	if (!state->orig_sp || info->type != prev_type)
+		state->orig_sp = frame;
 
 	return true;
 }
@@ -162,14 +195,12 @@ static bool update_stack_state(struct unwind_state *state, void *addr,
 bool unwind_next_frame(struct unwind_state *state)
 {
 	struct pt_regs *regs;
-	unsigned long *next_bp, *next_frame;
-	size_t next_len;
-	enum stack_type prev_type = state->stack_info.type;
+	unsigned long *next_bp;
 
 	if (unwind_done(state))
 		return false;
 
-	/* have we reached the end? */
+	/* Have we reached the end? */
 	if (state->regs && user_mode(state->regs))
 		goto the_end;
 
@@ -200,24 +231,14 @@ bool unwind_next_frame(struct unwind_state *state)
 		return true;
 	}
 
-	/* get the next frame pointer */
+	/* Get the next frame pointer: */
 	if (state->regs)
 		next_bp = (unsigned long *)state->regs->bp;
 	else
-		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task,*state->bp);
+		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);
 
-	/* is the next frame pointer an encoded pointer to pt_regs? */
-	regs = decode_frame_pointer(next_bp);
-	if (regs) {
-		next_frame = (unsigned long *)regs;
-		next_len = sizeof(*regs);
-	} else {
-		next_frame = next_bp;
-		next_len = FRAME_HEADER_SIZE;
-	}
-
-	/* make sure the next frame's data is accessible */
-	if (!update_stack_state(state, next_frame, next_len)) {
+	/* Move to the next frame if it's safe: */
+	if (!update_stack_state(state, next_bp)) {
 		/*
 		 * Don't warn on bad regs->bp.  An interrupt in entry code
 		 * might cause a false positive warning.
@@ -228,24 +249,6 @@ bool unwind_next_frame(struct unwind_state *state)
 		goto bad_address;
 	}
 
-	/* Make sure it only unwinds up and doesn't overlap the last frame: */
-	if (state->stack_info.type == prev_type) {
-		if (state->regs && (void *)next_frame < (void *)state->regs + regs_size(state->regs))
-			goto bad_address;
-
-		if (state->bp && (void *)next_frame < (void *)state->bp + FRAME_HEADER_SIZE)
-			goto bad_address;
-	}
-
-	/* move to the next frame */
-	if (regs) {
-		state->regs = regs;
-		state->bp = NULL;
-	} else {
-		state->bp = next_bp;
-		state->regs = NULL;
-	}
-
 	return true;
 
 bad_address:
@@ -263,13 +266,13 @@ bad_address:
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",
 			state->regs, state->task->comm,
-			state->task->pid, next_frame);
+			state->task->pid, next_bp);
 		unwind_dump(state, (unsigned long *)state->regs);
 	} else {
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack frame pointer at %p in %s:%d has bad value %p\n",
 			state->bp, state->task->comm,
-			state->task->pid, next_frame);
+			state->task->pid, next_bp);
 		unwind_dump(state, state->bp);
 	}
 the_end:
@@ -281,35 +284,23 @@ EXPORT_SYMBOL_GPL(unwind_next_frame);
 void __unwind_start(struct unwind_state *state, struct task_struct *task,
 		    struct pt_regs *regs, unsigned long *first_frame)
 {
-	unsigned long *bp, *frame;
-	size_t len;
+	unsigned long *bp;
 
 	memset(state, 0, sizeof(*state));
 	state->task = task;
 
-	/* don't even attempt to start from user mode regs */
+	/* Don't even attempt to start from user mode regs: */
 	if (regs && user_mode(regs)) {
 		state->stack_info.type = STACK_TYPE_UNKNOWN;
 		return;
 	}
 
-	/* set up the starting stack frame */
 	bp = get_frame_pointer(task, regs);
-	regs = decode_frame_pointer(bp);
-	if (regs) {
-		state->regs = regs;
-		frame = (unsigned long *)regs;
-		len = sizeof(*regs);
-	} else {
-		state->bp = bp;
-		frame = bp;
-		len = FRAME_HEADER_SIZE;
-	}
 
-	/* initialize stack info and make sure the frame data is accessible */
-	get_stack_info(frame, state->task, &state->stack_info,
+	/* Initialize stack info and make sure the frame data is accessible: */
+	get_stack_info(bp, state->task, &state->stack_info,
 		       &state->stack_mask);
-	update_stack_state(state, frame, len);
+	update_stack_state(state, bp);
 
 	/*
 	 * The caller can provide the address of the first frame directly

[tip:x86/asm] x86/unwind: Read stack return address in update_stack_state()

[tip-bot for Josh Poimboeuf]

Commit-ID:  6bcdf9d51b9892dd5c6892d50cf5e9628efb8062
Gitweb:     http://git.kernel.org/tip/6bcdf9d51b9892dd5c6892d50cf5e9628efb8062
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Wed, 12 Apr 2017 13:47:11 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 14 Apr 2017 10:19:59 +0200

x86/unwind: Read stack return address in update_stack_state()

Instead of reading the return address when unwind_get_return_address()
is called, read it from update_stack_state() and store it in the unwind
state.  This enables the next patch to check the return address from
unwind_next_frame() so it can detect an entry code frame.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Dave Jones <davej@codemonkey.org.uk>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/af0c5e4560c49c0343dca486ea26c4fa92bc4e35.1492020577.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/unwind.h  |  1 +
 arch/x86/kernel/unwind_frame.c | 25 +++++++++++++------------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 6fa75b1..5663425 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -14,6 +14,7 @@ struct unwind_state {
 #ifdef CONFIG_FRAME_POINTER
 	unsigned long *bp, *orig_sp;
 	struct pt_regs *regs;
+	unsigned long ip;
 #else
 	unsigned long *sp;
 #endif
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 9098ef1..c67a059 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -56,20 +56,10 @@ static void unwind_dump(struct unwind_state *state, unsigned long *sp)
 
 unsigned long unwind_get_return_address(struct unwind_state *state)
 {
-	unsigned long addr;
-	unsigned long *addr_p = unwind_get_return_address_ptr(state);
-
 	if (unwind_done(state))
 		return 0;
 
-	if (state->regs && user_mode(state->regs))
-		return 0;
-
-	addr = READ_ONCE_TASK_STACK(state->task, *addr_p);
-	addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, addr,
-				     addr_p);
-
-	return __kernel_text_address(addr) ? addr : 0;
+	return __kernel_text_address(state->ip) ? state->ip : 0;
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
@@ -141,7 +131,7 @@ static bool update_stack_state(struct unwind_state *state,
 	struct stack_info *info = &state->stack_info;
 	enum stack_type prev_type = info->type;
 	struct pt_regs *regs;
-	unsigned long *frame, *prev_frame_end;
+	unsigned long *frame, *prev_frame_end, *addr_p, addr;
 	size_t len;
 
 	if (state->regs)
@@ -185,6 +175,16 @@ static bool update_stack_state(struct unwind_state *state,
 		state->regs = NULL;
 	}
 
+	/* Save the return address: */
+	if (state->regs && user_mode(state->regs))
+		state->ip = 0;
+	else {
+		addr_p = unwind_get_return_address_ptr(state);
+		addr = READ_ONCE_TASK_STACK(state->task, *addr_p);
+		state->ip = ftrace_graph_ret_addr(state->task, &state->graph_idx,
+						  addr, addr_p);
+	}
+
 	/* Save the original stack pointer for unwind_dump(): */
 	if (!state->orig_sp || info->type != prev_type)
 		state->orig_sp = frame;
@@ -228,6 +228,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		 */
 		state->regs = regs;
 		state->bp = NULL;
+		state->ip = 0;
 		return true;
 	}
 

[tip:x86/asm] x86/unwind: Silence entry-related warnings

[tip-bot for Josh Poimboeuf]

Commit-ID:  a8b7a92318b6d7779f6d8e9aa6ba0e3de01a8943
Gitweb:     http://git.kernel.org/tip/a8b7a92318b6d7779f6d8e9aa6ba0e3de01a8943
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Wed, 12 Apr 2017 13:47:12 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 14 Apr 2017 10:20:06 +0200

x86/unwind: Silence entry-related warnings

A few people have reported unwinder warnings like the following:

  WARNING: kernel stack frame pointer at ffffc90000fe7ff0 in rsync:1157 has bad value           (null)
  unwind stack type:0 next_sp:          (null) mask:2 graph_idx:0
  ffffc90000fe7f98: ffffc90000fe7ff0 (0xffffc90000fe7ff0)
  ffffc90000fe7fa0: ffffffffb7000f56 (trace_hardirqs_off_thunk+0x1a/0x1c)
  ffffc90000fe7fa8: 0000000000000246 (0x246)
  ffffc90000fe7fb0: 0000000000000000 ...
  ffffc90000fe7fc0: 00007ffe3af639bc (0x7ffe3af639bc)
  ffffc90000fe7fc8: 0000000000000006 (0x6)
  ffffc90000fe7fd0: 00007f80af433fc5 (0x7f80af433fc5)
  ffffc90000fe7fd8: 00007ffe3af638e0 (0x7ffe3af638e0)
  ffffc90000fe7fe0: 00007ffe3af638e0 (0x7ffe3af638e0)
  ffffc90000fe7fe8: 00007ffe3af63970 (0x7ffe3af63970)
  ffffc90000fe7ff0: 0000000000000000 ...
  ffffc90000fe7ff8: ffffffffb7b74b9a (entry_SYSCALL_64_after_swapgs+0x17/0x4f)

This warning can happen when unwinding a code path where an interrupt
occurred in x86 entry code before it set up the first stack frame.
Silently ignore any warnings for this case.

Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: c32c47c68a0a ("x86/unwind: Warn on bad frame pointer")
Link: http://lkml.kernel.org/r/dbd6838826466a60dc23a52098185bc973ce2f1e.1492020577.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/unwind.h  |  1 +
 arch/x86/kernel/unwind_frame.c | 36 +++++++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 5663425..9b10dcd 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -12,6 +12,7 @@ struct unwind_state {
 	struct task_struct *task;
 	int graph_idx;
 #ifdef CONFIG_FRAME_POINTER
+	bool got_irq;
 	unsigned long *bp, *orig_sp;
 	struct pt_regs *regs;
 	unsigned long ip;
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index c67a059..c461135 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -1,6 +1,8 @@
 #include <linux/sched.h>
 #include <linux/sched/task.h>
 #include <linux/sched/task_stack.h>
+#include <linux/interrupt.h>
+#include <asm/sections.h>
 #include <asm/ptrace.h>
 #include <asm/bitops.h>
 #include <asm/stacktrace.h>
@@ -72,6 +74,21 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
+static bool in_entry_code(unsigned long ip)
+{
+	char *addr = (char *)ip;
+
+	if (addr >= __entry_text_start && addr < __entry_text_end)
+		return true;
+
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
+	if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
+		return true;
+#endif
+
+	return false;
+}
+
 #ifdef CONFIG_X86_32
 #define GCC_REALIGN_WORDS 3
 #else
@@ -144,6 +161,7 @@ static bool update_stack_state(struct unwind_state *state,
 	if (regs) {
 		frame = (unsigned long *)regs;
 		len = regs_size(regs);
+		state->got_irq = true;
 	} else {
 		frame = next_bp;
 		len = FRAME_HEADER_SIZE;
@@ -239,16 +257,8 @@ bool unwind_next_frame(struct unwind_state *state)
 		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);
 
 	/* Move to the next frame if it's safe: */
-	if (!update_stack_state(state, next_bp)) {
-		/*
-		 * Don't warn on bad regs->bp.  An interrupt in entry code
-		 * might cause a false positive warning.
-		 */
-		if (state->regs)
-			goto the_end;
-
+	if (!update_stack_state(state, next_bp))
 		goto bad_address;
-	}
 
 	return true;
 
@@ -263,6 +273,13 @@ bad_address:
 	if (state->task != current)
 		goto the_end;
 
+	/*
+	 * Don't warn if the unwinder got lost due to an interrupt in entry
+	 * code before the stack was set up:
+	 */
+	if (state->got_irq && in_entry_code(state->ip))
+		goto the_end;
+
 	if (state->regs) {
 		printk_deferred_once(KERN_WARNING
 			"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",
@@ -289,6 +306,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task,
 
 	memset(state, 0, sizeof(*state));
 	state->task = task;
+	state->got_irq = (regs);
 
 	/* Don't even attempt to start from user mode regs: */
 	if (regs && user_mode(regs)) {