lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  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:   Sat, 13 Jan 2018 07:33:20 +0100
From:   Willy Tarreau <w@....eu>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Borislav Petkov <bp@...en8.de>,
        Laura Abbott <labbott@...hat.com>, X86 ML <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: Yet another KPTI regression with 4.14.x series in a VM

On Fri, Jan 12, 2018 at 10:08:20PM -0800, Andy Lutomirski wrote:
> In fact, it looks like this code is totally bogus and has never been
> correct at all.  Even in:
> 
> commit 4b1d5ae3b103eda43f9d0f85c355bb6995b03a30
> Author: Peter Zijlstra <peterz@...radead.org>
> Date:   Mon Dec 4 15:07:59 2017 +0100
> 
>     x86/mm: Use/Fix PCID to optimize user/kernel switches
> 
> We have:
> 
> .macro SWITCH_TO_USER_CR3_NOSTACK scratch_reg:req scratch_reg2:req
>         ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI
>         mov     %cr3, \scratch_reg
> 
>         ALTERNATIVE "jmp .Lwrcr3_\@", "", X86_FEATURE_PCID
> 
> ...
> 
> .Lwrcr3_\@:
>         /* Flip the PGD and ASID to the user version */
>         orq     $(PTI_SWITCH_MASK), \scratch_reg
>         mov     \scratch_reg, %cr3
> .Lend_\@:
> 
> That's bogus.  PTI_SWITCH_MASK is 0x1800, which has PCID = 0x800.
> 
> This should probably use an alternative to select between 0x1000 and
> 0x800 depending on X86_FEATURE_PCID or just use an entirely different
> label for the !PCID case.
> 
> FWIW, this bit in SAVE_AND_SWITCH_TO_KERNEL_CR3
> 
>         testq   $(PTI_SWITCH_MASK), \scratch_reg
>         jz      .Ldone_\@
> 
> is a bit silly, too.  It's *correct* (I think), but shouldn't that
> just be bt $(PTI_SWITCH_PGTABLES_BIT), \scratch_reg, with the obvious
> caveat that the headers don't actually define PTI_SWITCH_PGTABLES_BIT?

I wondered the same initially when reading this but thought there was
surely a good reason that I could not understand due to my lack of
knowledge and stopped wondering. BTW your PTI_SWITCH_PGTABLES_BIT would
in fact be PAGE_SHIFT :-)

> Was this code *ever* tested with nopcid?

At least it booted fine in qemu on my machine where pcid was initially
disabled by default, and on an Atom D510 which doesn't have PCID. I've
worked on the initial per-task PTI code with this. That doesn't mean
it's valid, just that there are situations where it works fine :-)

Willy

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux - Powered by OpenVZ