x86/unwind: add some unwinder warnings

PATCH In review Josh Poimboeuf 3 participants 11 messages Oct 26, 2016 – Oct 28, 2016

Subsystems (2)

x86 x86 stack unwinding

lore

Share & tools

lore

Josh Poimboeuf

author

2016-10-26 15:41 UTC #0 lore raw

cover x86/unwind: add some unwinder warnings

Detect some stack error conditions in the unwinder and warn about them.

These warnings may help with debugging stack corruption issues.

Josh Poimboeuf (4):
x86/unwind: warn on bad frame pointer
x86/unwind: ensure stack grows down
x86/dumpstack: warn on stack recursion
x86/unwind: detect bad stack return address

 arch/x86/kernel/dumpstack_32.c |  5 +++-
 arch/x86/kernel/dumpstack_64.c |  5 +++-
 arch/x86/kernel/unwind_frame.c | 56 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 61 insertions(+), 5 deletions(-)

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-10-26 15:41 UTC #1 lore raw

PATCH 1/4 x86/unwind: warn on bad frame pointer

Detect situations in the unwinder where the frame pointer refers to a bad address, and print an appropriate warning.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

arch/x86/kernel/unwind_frame.c +16 -2

index 5795427..9be9a8f 100644
@@ -123,8 +123,17 @@ bool unwind_next_frame(struct unwind_state *state)
 	}
 
 	/* make sure the next frame's data is accessible */
-	if (!update_stack_state(state, next_frame, next_len))
-		return false;
+	if (!update_stack_state(state, next_frame, next_len)) {
+		/*
+		 * Don't warn on bad regs->bp.  An interrupt in entry code
+		 * might cause a false positive warning.
+		 */
+		if (state->regs)
+			goto the_end;
+
+		goto bad_address;
+	}
+
 	/* move to the next frame */
 	if (regs) {
 		state->regs = regs;

@@ -136,6 +145,11 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	return true;
 
+bad_address:
+	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_bp);
 the_end:
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-10-27 07:40 UTC #2 lore raw

[tip:x86/asm] x86/unwind: Warn on bad frame pointer

x86/unwind: Warn on bad frame pointer

Commit-ID:  c32c47c68a0ae701088c5b2c3798856ed16746ae
Gitweb:     http://git.kernel.org/tip/c32c47c68a0ae701088c5b2c3798856ed16746ae
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 26 Oct 2016 10:41:48 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 27 Oct 2016 08:32:37 +0200

x86/unwind: Warn on bad frame pointer

Detect situations in the unwinder where the frame pointer refers to a bad address, and print an appropriate warning.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/03c888f6f7414d54fa56b393ea25482be6899b5f.1477496147.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

arch/x86/kernel/unwind_frame.c +16 -2

index 5795427..9be9a8f 100644
@@ -123,8 +123,17 @@ bool unwind_next_frame(struct unwind_state *state)
 	}
 
 	/* make sure the next frame's data is accessible */
-	if (!update_stack_state(state, next_frame, next_len))
-		return false;
+	if (!update_stack_state(state, next_frame, next_len)) {
+		/*
+		 * Don't warn on bad regs->bp.  An interrupt in entry code
+		 * might cause a false positive warning.
+		 */
+		if (state->regs)
+			goto the_end;
+
+		goto bad_address;
+	}
+
 	/* move to the next frame */
 	if (regs) {
 		state->regs = regs;

@@ -136,6 +145,11 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	return true;
 
+bad_address:
+	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_bp);
 the_end:
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;

Josh Poimboeuf

author

2016-10-26 15:43 UTC #3 lore raw

PATCH 2/4 x86/unwind: ensure stack grows down

In reply to Josh Poimboeuf

Add a sanity check to ensure the stack only grows down, and print a warning if the check fails.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

arch/x86/kernel/unwind_frame.c +32 -4

index 9be9a8f..0d8d2f2 100644
@@ -24,6 +24,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
+static size_t regs_size(struct pt_regs *regs)
+{
+	/* x86_32 regs from kernel mode are two words shorter */
+	if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
+		return sizeof(*regs) - (2*sizeof(long));
+
+	return sizeof(*regs);
+}
+
 static bool is_last_task_frame(struct unwind_state *state)
 {
 	unsigned long bp = (unsigned long)state->bp;

@@ -71,6 +80,7 @@ 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;
 
 	if (unwind_done(state))
 		return false;

@@ -134,6 +144,18 @@ 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;

@@ -146,10 +168,16 @@ bool unwind_next_frame(struct unwind_state *state)
 	return true;
 
 bad_address:
-	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_bp);
+	if (state->regs)
+		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);
+	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);
 the_end:
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;

Signature · 1 line

2.7.4

Ingo Molnar

2016-10-27 06:30 UTC #4 lore raw

Re: [PATCH 2/4] x86/unwind: ensure stack grows down

Quoted 13 lines · Josh Poimboeuf

Add a sanity check to ensure the stack only grows down, and print a warning if the check fails.
Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/unwind_frame.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)
arch/x86/kernel/unwind_frame.c +0 -0
index 9be9a8f..0d8d2f2 100644

arch/x86/kernel/unwind_frame.c unsigned long unwind_get_return_address(struct unwind_state *state) · line 31

Quoted 8 lines

arch/x86/kernel/unwind_frame.c +5 -0

@@ -24,6 +24,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
+static size_t regs_size(struct pt_regs *regs)
+{
+	/* x86_32 regs from kernel mode are two words shorter */
+	if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
+		return sizeof(*regs) - (2*sizeof(long));

Unnecessary parentheses.

Quoted 16 lines

arch/x86/kernel/unwind_frame.c +5 -0

+
+	return sizeof(*regs);
+}
+
 static bool is_last_task_frame(struct unwind_state *state)
 {
 	unsigned long bp = (unsigned long)state->bp;

@@ -71,6 +80,7 @@ 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;
 
 	if (unwind_done(state))
 		return false;

arch/x86/kernel/unwind_frame.c bool unwind_next_frame(struct unwind_state *state) · line 147

Quoted 4 lines

arch/x86/kernel/unwind_frame.c +1 -0

@@ -134,6 +144,18 @@ bool unwind_next_frame(struct unwind_state *state)
 		goto bad_address;
 	}
 
+	/* make sure it only unwinds up and doesn't overlap the last frame */

Please capitalize comments - and if the comment refers to a control block please also add ':', i.e. something like:

	/* Make sure it only unwinds up and doesn't overlap the last frame: */

This also applies to other places in this patch series.

arch/x86/kernel/unwind_frame.c bool unwind_next_frame(struct unwind_state *state) · line 151

Quoted 4 lines

arch/x86/kernel/unwind_frame.c +4 -0

+	if (state->stack_info.type == prev_type) {
+		if (state->regs &&
+		    (void *)next_frame < (void *)state->regs +
+					 regs_size(state->regs))

No line breaks that make the code worse please.

arch/x86/kernel/unwind_frame.c bool unwind_next_frame(struct unwind_state *state) · line 155

Quoted 4 lines

arch/x86/kernel/unwind_frame.c +4 -0

+			goto bad_address;
+
+		if (state->bp &&
+		    (void *)next_frame < (void *)state->bp + FRAME_HEADER_SIZE)

Ditto.

Quoted 14 lines

arch/x86/kernel/unwind_frame.c +3 -4

+			goto bad_address;
+	}
+
 	/* move to the next frame */
 	if (regs) {
 		state->regs = regs;

@@ -146,10 +168,16 @@ bool unwind_next_frame(struct unwind_state *state)
 	return true;
 
 bad_address:
-	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_bp);

arch/x86/kernel/unwind_frame.c bool unwind_next_frame(struct unwind_state *state) · line 180

Quoted 10 lines

arch/x86/kernel/unwind_frame.c +10 -0

+	if (state->regs)
+		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);
+	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);

Multi-line statements should have curly braces.

Thanks,

Ingo

Josh Poimboeuf

author

2016-10-27 14:27 UTC #5 lore raw

PATCH v2 1/1 x86/unwind: ensure stack grows down

Add a sanity check to ensure the stack only grows down, and print a warning if the check fails.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

arch/x86/kernel/unwind_frame.c +30 -4

index 5639db6..fe4d387 100644
@@ -32,6 +32,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
+static size_t regs_size(struct pt_regs *regs)
+{
+	/* x86_32 regs from kernel mode are two words shorter */
+	if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
+		return sizeof(*regs) - 2*sizeof(long);
+
+	return sizeof(*regs);
+}
+
 static bool is_last_task_frame(struct unwind_state *state)
 {
 	unsigned long bp = (unsigned long)state->bp;

@@ -79,6 +88,7 @@ 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;
 
 	if (unwind_done(state))
 		return false;

@@ -142,6 +152,15 @@ 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;

@@ -154,10 +173,17 @@ bool unwind_next_frame(struct unwind_state *state)
 	return true;
 
 bad_address:
-	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_bp);
+	if (state->regs) {
+		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);
+	} 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);
+	}
 the_end:
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-10-28 06:50 UTC #6 lore raw

[tip:x86/asm] x86/unwind: Ensure stack grows down

x86/unwind: Ensure stack grows down

Commit-ID:  24d86f59093b0bcb3756cdf47f2db10ff4e90dbb
Gitweb:     http://git.kernel.org/tip/24d86f59093b0bcb3756cdf47f2db10ff4e90dbb
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Thu, 27 Oct 2016 08:10:58 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 28 Oct 2016 08:16:45 +0200

x86/unwind: Ensure stack grows down

Add a sanity check to ensure the stack only grows down, and print a warning if the check fails.

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/20161027131058.tpdffwlqipv7pcd6@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

arch/x86/kernel/unwind_frame.c +30 -4

index 5639db6..ea7b7f9 100644
@@ -32,6 +32,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);
 
+static size_t regs_size(struct pt_regs *regs)
+{
+	/* x86_32 regs from kernel mode are two words shorter: */
+	if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
+		return sizeof(*regs) - 2*sizeof(long);
+
+	return sizeof(*regs);
+}
+
 static bool is_last_task_frame(struct unwind_state *state)
 {
 	unsigned long bp = (unsigned long)state->bp;

@@ -79,6 +88,7 @@ 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;
 
 	if (unwind_done(state))
 		return false;

@@ -142,6 +152,15 @@ 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;

@@ -154,10 +173,17 @@ bool unwind_next_frame(struct unwind_state *state)
 	return true;
 
 bad_address:
-	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_bp);
+	if (state->regs) {
+		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);
+	} 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);
+	}
 the_end:
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;

Josh Poimboeuf

author

2016-10-26 15:43 UTC #7 lore raw

PATCH 3/4 x86/dumpstack: warn on stack recursion

In reply to Josh Poimboeuf

Print a warning if stack recursion is detected.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/dumpstack_32.c | 5 ++++-
 arch/x86/kernel/dumpstack_64.c | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

arch/x86/kernel/dumpstack_32.c +4 -1

index 90cf460..99ac8d2 100644
@@ -109,8 +109,11 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,
 	 * just break out and report an unknown stack type.
 	 */
 	if (visit_mask) {
-		if (*visit_mask & (1UL << info->type))
+		if (*visit_mask & (1UL << info->type)) {
+			printk_deferred_once(KERN_WARNING "WARNING: stack recursion on stack type %d\n",
+					     info->type);
 			goto unknown;
+		}
 		*visit_mask |= 1UL << info->type;
 	}

arch/x86/kernel/dumpstack_64.c +4 -1

index 310abf4..26e6e83 100644
@@ -128,8 +128,11 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,
 	 * just break out and report an unknown stack type.
 	 */
 	if (visit_mask) {
-		if (*visit_mask & (1UL << info->type))
+		if (*visit_mask & (1UL << info->type)) {
+			printk_deferred_once(KERN_WARNING "WARNING: stack recursion on stack type %d\n",
+					     info->type);
 			goto unknown;
+		}
 		*visit_mask |= 1UL << info->type;
 	}

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-10-27 07:40 UTC #8 lore raw

[tip:x86/asm] x86/dumpstack: Warn on stack recursion

x86/dumpstack: Warn on stack recursion

Commit-ID:  0d2b8579add41e08aa1110da864f1071d58e6048
Gitweb:     http://git.kernel.org/tip/0d2b8579add41e08aa1110da864f1071d58e6048
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 26 Oct 2016 10:41:50 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 27 Oct 2016 08:32:38 +0200

x86/dumpstack: Warn on stack recursion

Print a warning if stack recursion is detected.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/def18247aafaab480844484398e793f552b79bda.1477496147.git.jpoimboe@redhat.com open here

[ Unbroke the lines. ]

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/dumpstack_32.c | 4 +++-
 arch/x86/kernel/dumpstack_64.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

arch/x86/kernel/dumpstack_32.c +3 -1

index 90cf460..d7d20999 100644
@@ -109,8 +109,10 @@ recursion_check:
 	 * just break out and report an unknown stack type.
 	 */
 	if (visit_mask) {
-		if (*visit_mask & (1UL << info->type))
+		if (*visit_mask & (1UL << info->type)) {
+			printk_deferred_once(KERN_WARNING "WARNING: stack recursion on stack type %d\n", info->type);
 			goto unknown;
+		}
 		*visit_mask |= 1UL << info->type;
 	}

arch/x86/kernel/dumpstack_64.c +3 -1

index 310abf4..ab0f8b9 100644
@@ -128,8 +128,10 @@ recursion_check:
 	 * just break out and report an unknown stack type.
 	 */
 	if (visit_mask) {
-		if (*visit_mask & (1UL << info->type))
+		if (*visit_mask & (1UL << info->type)) {
+			printk_deferred_once(KERN_WARNING "WARNING: stack recursion on stack type %d\n", info->type);
 			goto unknown;
+		}
 		*visit_mask |= 1UL << info->type;
 	}

Josh Poimboeuf

author

2016-10-26 15:42 UTC #9 lore raw

PATCH 4/4 x86/unwind: detect bad stack return address

In reply to Josh Poimboeuf

If __kernel_text_address() doesn't recognize a return address on the stack, it probably means that it's some generated code which __kernel_text_address() doesn't know about yet.

Otherwise there's probably some stack corruption.

Either way, warn about it.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

arch/x86/kernel/unwind_frame.c +9 -1

index 0d8d2f2..b6fcf44 100644
@@ -20,7 +20,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 	addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, *addr_p,
 				     addr_p);
 
-	return __kernel_text_address(addr) ? addr : 0;
+	if (!__kernel_text_address(addr)) {
+		printk_deferred_once(KERN_WARNING
+			"WARNING: unrecognized kernel stack return address %p at %p in %s:%d\n",
+			(void *)addr, addr_p, state->task->comm,
+			state->task->pid);
+		return 0;
+	}
+
+	return addr;
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-10-27 07:39 UTC #10 lore raw

[tip:x86/asm] x86/unwind: Detect bad stack return address

x86/unwind: Detect bad stack return address

Commit-ID:  b6959a362177053c1c90db6dc1af25b6bddd8548
Gitweb:     http://git.kernel.org/tip/b6959a362177053c1c90db6dc1af25b6bddd8548
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 26 Oct 2016 10:41:51 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 27 Oct 2016 08:32:38 +0200

x86/unwind: Detect bad stack return address

If __kernel_text_address() doesn't recognize a return address on the stack, it probably means that it's some generated code which __kernel_text_address() doesn't know about yet.

Otherwise there's probably some stack corruption.

Either way, warn about it.

Use printk_deferred_once() because the unwinder can be called with the console lock by lockdep via save_stack_trace().

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/2d897898f324e275943b590d160b55e482bba65f.1477496147.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/unwind_frame.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

arch/x86/kernel/unwind_frame.c +9 -1

index 9be9a8f..5639db6 100644
@@ -20,7 +20,15 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
 	addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, *addr_p,
 				     addr_p);
 
-	return __kernel_text_address(addr) ? addr : 0;
+	if (!__kernel_text_address(addr)) {
+		printk_deferred_once(KERN_WARNING
+			"WARNING: unrecognized kernel stack return address %p at %p in %s:%d\n",
+			(void *)addr, addr_p, state->task->comm,
+			state->task->pid);
+		return 0;
+	}
+
+	return addr;
 }
 EXPORT_SYMBOL_GPL(unwind_get_return_address);