2017-12-04 16:50:55

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 02/60] x86/unwinder/orc: Dont bail on stack overflow

From: Andy Lutomirski <[email protected]>

If the stack overflows into a guard page and the ORC unwinder should work
well: by construction, there can't be any meaningful data in the guard page
because no writes to the guard page will have succeeded.

But there is a bug that prevents unwinding from working correctly: if the
starting register state has RSP pointing into a stack guard page, the ORC
unwinder bails out immediately.

Instead of bailing out immediately check whether the next page up is a
valid check page and if so analyze that. As a result the ORC unwinder will
start the unwind.

Tested by intentionally overflowing the task stack. The result is an
accurate call trace instead of a trace consisting purely of '?' entries.

There are a few other bugs that are triggered if the unwinder encounters a
stack overflow after the first step, but they are outside the scope of this
fix.

Signed-off-by: Andy Lutomirski <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/927042950d7f1a7007dd0f58538966a593508f8b.1511715954.git.luto@kernel.org

---
arch/x86/kernel/unwind_orc.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -553,8 +553,18 @@ void __unwind_start(struct unwind_state
}

if (get_stack_info((unsigned long *)state->sp, state->task,
- &state->stack_info, &state->stack_mask))
- return;
+ &state->stack_info, &state->stack_mask)) {
+ /*
+ * We weren't on a valid stack. It's possible that
+ * we overflowed a valid stack into a guard page.
+ * See if the next page up is valid so that we can
+ * generate some kind of backtrace if this happens.
+ */
+ void *next_page = (void *)PAGE_ALIGN((unsigned long)regs->sp);
+ if (get_stack_info(next_page, state->task, &state->stack_info,
+ &state->stack_mask))
+ return;
+ }

/*
* The caller can provide the address of the first frame directly



2017-12-04 20:31:23

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 02/60] x86/unwinder/orc: Dont bail on stack overflow

Fixlet lost here

> On Dec 4, 2017, at 6:07 AM, Thomas Gleixner <[email protected]> wrote:
>
> regs->sp

Should be state->sp

2017-12-04 21:32:14

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 02/60] x86/unwinder/orc: Dont bail on stack overflow

On Mon, 4 Dec 2017, Andy Lutomirski wrote:
> Fixlet lost here
>
> > On Dec 4, 2017, at 6:07 AM, Thomas Gleixner <[email protected]> wrote:
> >
> > regs->sp
>
> Should be state->sp

Fixed.

Thanks,

tglx