linux-kernel - Re: [GIT PULL] x86/asm changes for v5.6

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:   Wed, 29 Jan 2020 09:07:25 -0800
From:   "Luck, Tony" <tony.luck@...el.com>
To:     Borislav Petkov <bp@...e.de>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] x86/asm changes for v5.6

On Wed, Jan 29, 2020 at 02:26:18PM +0100, Borislav Petkov wrote:
> On Tue, Jan 28, 2020 at 12:06:53PM -0800, Linus Torvalds wrote:
> > On Tue, Jan 28, 2020 at 11:51 AM Linus Torvalds
> > <torvalds@...ux-foundation.org> wrote:
> > >
> > >         ALTERNATIVE_2 \
> > >                 "cmp  $680, %rdx ; jb 3f ; cmpb %dil, %sil; je 4f", \
> > >                 "movq %rdx, %rcx ; rep movsb; retq", X86_FEATURE_FSRM, \
> > >                 "cmp $0x20, %rdx; jb 1f; movq %rdx, %rcx; rep movsb; retq", X86_FEATURE_ERMS
> > 
> > Note the UNTESTED part.
> > 
> > In particular, I didn't check what the priority for the alternatives
> > is. Since FSRM being set always implies ERMS being set too, it may be
> > that the ERMS case is always picked with the above code.

So I wrote a tiny function to test (rather than wrestle with trying
to disassemble the post-alternative patched binary of the running system):

        .globl  feature
        .type   feature, @function
feature:
        .cfi_startproc
       ALTERNATIVE_2 \
               "movl    $1, %eax", \
               "movl    $2, %eax", X86_FEATURE_FSRM, \
               "movl    $3, %eax", X86_FEATURE_ERMS

        ret

This returns "3" ... not what we want. But swapping the ERMS/FSRM order
gets the correct version.

> And yes, your idea makes sense to use ALTERNATIVE_2 but as it is, it
> triple-faults my guest. I'll debug it more later to find out why, when I
> get a chance.

Triple fault is a surprise.  As long as you have ERMS, it shouldn't
hurt to take the FSRM code path.

Does the code that performs the patch use memmove() to copy the alternate
version into place? That could get ugly!

I'm not in the same city as my test machine, so I'm going to defer testing
Linus' patch (with FSRM/ERMS swapped) until I'm near enough to poke it
if it breaks.

-Tony