linux-kernel - Re: [PATCH 02/16] x86/dumpstack: Add get_stack

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]

Message-ID: <20171120204212.6rlbhh4btxqnjanm@treble>
Date:   Mon, 20 Nov 2017 14:42:12 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     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 02/16] x86/dumpstack: Add get_stack_info() support for
 the SYSENTER stack

On Mon, Nov 20, 2017 at 09:07:33AM -0800, Andy Lutomirski wrote:
> +bool in_SYSENTER_stack(unsigned long *stack, struct stack_info *info)

Can you make it lowercase for consistency with the other in_*_stack()
functions?  For example, in_irq_stack() is all lowercase even though
"IRQ" is normally written in uppercase.

But also, I'm wondering whether this get_stack_info() support is even
really needed.

As currently written, the trampoline code doesn't have any ORC data
associated with it.  So the unwinder would never have the need to
actually read the SYSENTER stack.

You _could_ add an UNWIND_HINT_IRET_REGS annotation after the simulated
iret frame is written, which would allow the unwinder to dump those regs
when unwinding from an NMI.

But there's only a tiny window where that would be possible: only a few
instructions.  I'm not sure that would be worth the effort, unless we
got to the point where we expect to have 100% unwinder coverage.  But
that's currently unrealistic anyway because of generated code and
runtime patching.

-- 
Josh