Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932358AbeAKGrD (ORCPT + 1 other); Thu, 11 Jan 2018 01:47:03 -0500 Received: from wtarreau.pck.nerim.net ([62.212.114.60]:39567 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754102AbeAKGq7 (ORCPT ); Thu, 11 Jan 2018 01:46:59 -0500 Date: Thu, 11 Jan 2018 07:46:31 +0100 From: Willy Tarreau To: Dave Hansen Cc: linux-kernel@vger.kernel.org, x86@kernel.org, Andy Lutomirski , Borislav Petkov , Brian Gerst , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Thomas Gleixner , Josh Poimboeuf , "H. Peter Anvin" , Kees Cook Subject: Re: [RFC PATCH v2 5/6] x86/entry/pti: avoid setting CR3 when it's already correct Message-ID: <20180111064631.GD14920@1wt.eu> References: <1515502580-12261-1-git-send-email-w@1wt.eu> <1515502580-12261-6-git-send-email-w@1wt.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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