Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752204AbeAJU3U (ORCPT + 1 other); Wed, 10 Jan 2018 15:29:20 -0500 Received: from mga01.intel.com ([192.55.52.88]:39945 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751405AbeAJU3S (ORCPT ); Wed, 10 Jan 2018 15:29:18 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,341,1511856000"; d="scan'208";a="19078654" Subject: Re: [RFC PATCH v2 5/6] x86/entry/pti: avoid setting CR3 when it's already correct To: Willy Tarreau , linux-kernel@vger.kernel.org, x86@kernel.org References: <1515502580-12261-1-git-send-email-w@1wt.eu> <1515502580-12261-6-git-send-email-w@1wt.eu> Cc: Andy Lutomirski , Borislav Petkov , Brian Gerst , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Thomas Gleixner , Josh Poimboeuf , "H. Peter Anvin" , Kees Cook From: Dave Hansen Message-ID: Date: Wed, 10 Jan 2018 12:29:17 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1515502580-12261-6-git-send-email-w@1wt.eu> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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.