linux-kernel - Re: [PATCH 03/14] x86/entry/64: Fix unwind hints in register clearing code

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAG48ez3xfARut-t38p=+Vx6mogESL4OFjJdjp=1F7Vukg4+ijg@mail.gmail.com>
Date:   Thu, 12 Mar 2020 22:24:07 +0100
From:   Jann Horn <jannh@...gle.com>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        kernel list <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Vince Weaver <vincent.weaver@...ne.edu>,
        Dave Jones <dsj@...com>, Miroslav Benes <mbenes@...e.cz>,
        Andy Lutomirski <luto@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 03/14] x86/entry/64: Fix unwind hints in register clearing code

On Thu, Mar 12, 2020 at 8:29 PM Andy Lutomirski <luto@...capital.net> wrote:
> > On Mar 12, 2020, at 10:31 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> > The PUSH_AND_CLEAR_REGS macro zeroes each register immediately after
> > pushing it.  If an NMI or exception hits after a register is cleared,
> > but before the UNWIND_HINT_REGS annotation, the ORC unwinder will
> > wrongly think the previous value of the register was zero.  This can
> > confuse the unwinding process and cause it to exit early.
> >
> > Because ORC is simpler than DWARF, there are a limited number of unwind
> > annotation states, so it's not possible to add an individual unwind hint
> > after each push/clear combination.  Instead, the register clearing
> > instructions need to be consolidated and moved to after the
> > UNWIND_HINT_REGS annotation.
>
> I don’t suppose you know how bad t he performance hit is on a non-PTI machine?
[...]
> > +    xorl    %edx,  %edx    /* nospec dx  */
> > +    xorl    %ecx,  %ecx    /* nospec cx  */
> > +    xorl    %r8d,  %r8d    /* nospec r8  */
> > +    xorl    %r9d,  %r9d    /* nospec r9  */
> > +    xorl    %r10d, %r10d    /* nospec r10 */
> > +    xorl    %r11d, %r11d    /* nospec r11 */
> > +    xorl    %ebx,  %ebx    /* nospec rbx */
> > +    xorl    %ebp,  %ebp    /* nospec rbp */
> > +    xorl    %r12d, %r12d    /* nospec r12 */
> > +    xorl    %r13d, %r13d    /* nospec r13 */
> > +    xorl    %r14d, %r14d    /* nospec r14 */
> > +    xorl    %r15d, %r15d    /* nospec r15 */

I'm curious what the reason for a performance impact would be. Are you
worried about the performance of the instruction decoder or the
renamer? As long as those aren't slowed down, this will just end up
giving the store uops to the backend earlier, right? (Since the XOR
instructions shouldn't reach the backend at all AKAIU.)