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]

Date:   Wed, 10 Jan 2018 12:29:17 -0800
From:   Dave Hansen <dave.hansen@...ux.intel.com>
To:     Willy Tarreau <w@....eu>, linux-kernel@...r.kernel.org,
        x86@...nel.org
Cc:     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 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.