linux-kernel - Re: [RFC PATCH v2 5/6] x86/entry/pti: avoid setting CR3 when it's already correct

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: <20180111064631.GD14920@1wt.eu>
Date:   Thu, 11 Jan 2018 07:46:31 +0100
From:   Willy Tarreau <w@....eu>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Brian Gerst <brgerst@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Kees Cook <keescook@...omium.org>
Subject: Re: [RFC PATCH v2 5/6] x86/entry/pti: avoid setting CR3 when it's
 already correct

On Wed, Jan 10, 2018 at 12:29:17PM -0800, Dave Hansen wrote:
> On 01/09/2018 04:56 AM, Willy Tarreau wrote:
> > --- a/arch/x86/entry/calling.h
> > +++ b/arch/x86/entry/calling.h
> > @@ -214,6 +214,11 @@
> >  .macro SWITCH_TO_KERNEL_CR3 scratch_reg:req
> >  	ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI
> >  	mov	%cr3, \scratch_reg
> > +
> > +	/* if we're already on the kernel PGD, we don't switch */
> > +	testq $(PTI_SWITCH_PGTABLES_MASK), \scratch_reg
> > +	jz .Lend_\@
> > +
> >  	ADJUST_KERNEL_CR3 \scratch_reg
> 
> Willy, this is not specific to your patch, but it is one of the first
> *changes* to this code since Spectre, thus I'm bringing it up in this
> thread.
> 
> The code already has some, but new conditional branches give me the
> willies.  None of them take the form that can be used to exploit
> Spectre/Variant1 that I can see, but I do think we need to start talking
> about why this is not vulnerable and probably documenting it in the
> entry code.
> 
> Spectre/V1 (conditional branches)
>  * Data reads in entry/exit when you have the user CR3 must be in
>    cpu_entry_area and readable to a Meltdown exploit, anyway.  That
>    implies that there is no data to be leaked in the address space
>    at all at this point.
>  * Conditional branches in the entry/exit code with a kernel CR3 value
>    are the only concern.  It is safe, however, if the data being checked
>    is not user-controllable.
> 
> Spectre/V2 (indirect branches)
>  * Indirect Branches in the entry code are forbidden because of
>    Spectre/V2.  Retpolines or other mitigations (IBRS) must be used
>    instead.
> 
> Anybody disagree?
> 
> In this case, the data being checked (CR3) is not user-controllable and
> there are no indirect branches.  So this code is OK.

Just let me know what comment you'd like me to add there. I suppose
something like "This code has code has been checked against Spectre/v1/v2
attacks ; it's safe as CR3 is not user-controllable and there's no
indirect branches" ?

Thanks,
Willy