From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (193.142.43.55:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 19 Nov 2019 18:13:09 -0000 Received: from mail.kernel.org ([198.145.29.99]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1iX7zv-0000JT-DP for speck@linutronix.de; Tue, 19 Nov 2019 19:13:08 +0100 Date: Tue, 19 Nov 2019 19:12:57 +0100 From: Greg KH Subject: [MODERATED] Re: LVI Message-ID: <20191119181257.GA2283647@kroah.com> References: <20191119174008.7dbymix2eo4mrv57@treble> MIME-Version: 1.0 In-Reply-To: <20191119174008.7dbymix2eo4mrv57@treble> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Tue, Nov 19, 2019 at 11:40:08AM -0600, speck for Josh Poimboeuf wrote: > Hi, > > What kernel changes (if any) are needed for LVI? I haven't seen any > discussion here. > > The last I heard, the official CRD was Dec 10, but was likely to move to > March. > > For the uninitiated, LVI is a reverse MDS/L1TF: > > 1) Victim puts secret data in CPU buffer or L1. Alternatively, > attacker puts address of secret data in CPU buffer or L1. > > 2) Attacker gets victim to fault or assist on a load. (Note that an > assist gives a much bigger speculation window - it can be triggered > if a page Accessed bit needs updating) > > 3) While waiting for the fault/assist to complete, victim speculatively > reads CPU buffer or L1 to get data (or address) from step 1. > > 4) Victim gadgets expose the data via the usual L1 side channel. > > > To protect the kernel, we'd presumably need to look for places where > users can trigger a faulting/assisting load. For example, > copy_from_user(). > > copy_from_user() has an LFENCE between the access_ok() check and the > actual copy to protect against Spectre v1. What if we move that LFENCE > to *after* the copy? I think that would protect against both Spectre v1 > and LVI. > > Thoughts? > > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > index 61d93f062a36..457207aece71 100644 > --- a/arch/x86/include/asm/uaccess.h > +++ b/arch/x86/include/asm/uaccess.h > @@ -119,9 +119,9 @@ extern int __get_user_bad(void); > > #define __uaccess_begin() stac() > #define __uaccess_end() clac() > -#define __uaccess_begin_nospec() \ > +#define __uaccess_end_nospec() \ > ({ \ > - stac(); \ > + clac(); \ > barrier_nospec(); \ > }) Um, don't we _really_ want stac() here? That's the only thing that is saving our butts in a number of places. Why get rid of it? Otherwise it should be easy to cause a speculatave load by userspace for any address they feel like. Or am I mistaken about what you are trying to solve here? thanks, greg k-h