linux-kernel - Re: [PATCH] x86/orc: Don't bail on stack overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Nov 2017 06:45:25 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>, X86 ML <x86@...nel.org>,
        Borislav Petkov <bpetkov@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86/orc: Don't bail on stack overflow

On Mon, Nov 27, 2017 at 10:38:42AM +0100, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> 
> > On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> > > Can you send me whatever config and exact commit hash generated this?
> > > I can try to figure out why it failed.
> > 
> > Sorry, I've been traveling.  I just got some time to take a look at
> > this.  I think there are at least two unwinder issues here:
> > 
> > - It doesn't deal gracefully with the case where the stack overflows and
> >   the stack pointer itself isn't on a valid stack but the
> >   to-be-dereferenced data *is*.
> > 
> > - The oops dump code doesn't know how to print partial pt_regs, for the
> >   case where if we get an interrupt/exception in *early* entry code
> >   before the full pt_regs have been saved.
> > 
> > (Andy, I'm not quite sure about your patch, and whether it's still
> > needed after these patches.  I'll need to look at it later when I have
> > more time.)
> > 
> > I attempted to fix both of the issues with the below patch.  Thomas or
> > Ingo, can you test to see if this gets rid of the question marks?
> > 
> > I can split it up into proper patches next week.  I'm assuming this
> > isn't holding up the KAISER merge?
> 
> It's not holding up the Kaiser merge, but good debuggability of weird crashes is a 
> really good thing, so I constructed a changelog and picked up this patch as a 
> single commit, and added your Signed-off-by, if that's OK with you.
> 
> Will only push it out if it passes testing.

The commit log looks good, though there's a CONFIG_FRAME_POINTER build
failure.  Can you fold in this fix?

diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index 7dd0d2a0d142..77835bc021c7 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -102,7 +102,7 @@ __save_stack_trace_reliable(struct stack_trace *trace,
 	for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
 	     unwind_next_frame(&state)) {
 
-		regs = unwind_get_entry_regs(&state, NULL);
+		regs = unwind_get_entry_regs(&state);
 		if (regs) {
 			/*
 			 * Kernel mode registers on the stack indicate an