[MODERATED] LVI

[MODERATED] LVI

[Josh Poimboeuf]

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();		\
 })
 
@@ -446,9 +446,9 @@ __pu_label:							\
 	__inttype(*(ptr)) __gu_val;					\
 	__typeof__(ptr) __gu_ptr = (ptr);				\
 	__typeof__(size) __gu_size = (size);				\
-	__uaccess_begin_nospec();					\
+	__uaccess_begin();					\
 	__get_user_size(__gu_val, __gu_ptr, __gu_size, __gu_err, -EFAULT);	\
-	__uaccess_end();						\
+	__uaccess_end_nospec();						\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
 	__builtin_expect(__gu_err, 0);					\
 })
@@ -496,10 +496,10 @@ struct __large_struct { unsigned long buf[100]; };
 
 #define uaccess_try_nospec do {						\
 	current->thread.uaccess_err = 0;				\
-	__uaccess_begin_nospec();					\
+	__uaccess_begin();						\
 
 #define uaccess_catch(err)						\
-	__uaccess_end();						\
+	__uaccess_end_nospec();						\
 	(err) |= (current->thread.uaccess_err ? -EFAULT : 0);		\
 } while (0)
 
@@ -592,7 +592,7 @@ extern void __cmpxchg_wrong_size(void)
 	int __ret = 0;							\
 	__typeof__(*(ptr)) __old = (old);				\
 	__typeof__(*(ptr)) __new = (new);				\
-	__uaccess_begin_nospec();					\
+	__uaccess_begin();						\
 	switch (size) {							\
 	case 1:								\
 	{								\
@@ -664,7 +664,7 @@ extern void __cmpxchg_wrong_size(void)
 	default:							\
 		__cmpxchg_wrong_size();					\
 	}								\
-	__uaccess_end();						\
+	__uaccess_end_nospec();						\
 	*(uval) = __old;						\
 	__ret;								\
 })
diff --git a/arch/x86/include/asm/uaccess_32.h b/arch/x86/include/asm/uaccess_32.h
index ba2dc1930630..c23fdec72b26 100644
--- a/arch/x86/include/asm/uaccess_32.h
+++ b/arch/x86/include/asm/uaccess_32.h
@@ -29,24 +29,24 @@ raw_copy_from_user(void *to, const void __user *from, unsigned long n)
 		switch (n) {
 		case 1:
 			ret = 0;
-			__uaccess_begin_nospec();
+			__uaccess_begin();
 			__get_user_asm_nozero(*(u8 *)to, from, ret,
 					      "b", "b", "=q", 1);
-			__uaccess_end();
+			__uaccess_end_nospec();
 			return ret;
 		case 2:
 			ret = 0;
-			__uaccess_begin_nospec();
+			__uaccess_begin();
 			__get_user_asm_nozero(*(u16 *)to, from, ret,
 					      "w", "w", "=r", 2);
-			__uaccess_end();
+			__uaccess_end_nospec();
 			return ret;
 		case 4:
 			ret = 0;
-			__uaccess_begin_nospec();
+			__uaccess_begin();
 			__get_user_asm_nozero(*(u32 *)to, from, ret,
 					      "l", "k", "=r", 4);
-			__uaccess_end();
+			__uaccess_end_nospec();
 			return ret;
 		}
 	}
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index 5cd1caa8bc65..7013f9ffded7 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -71,48 +71,48 @@ raw_copy_from_user(void *dst, const void __user *src, unsigned long size)
 		return copy_user_generic(dst, (__force void *)src, size);
 	switch (size) {
 	case 1:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u8 *)dst, (u8 __user *)src,
 			      ret, "b", "b", "=q", 1);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	case 2:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u16 *)dst, (u16 __user *)src,
 			      ret, "w", "w", "=r", 2);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	case 4:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u32 *)dst, (u32 __user *)src,
 			      ret, "l", "k", "=r", 4);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	case 8:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src,
 			      ret, "q", "", "=r", 8);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	case 10:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src,
 			       ret, "q", "", "=r", 10);
 		if (likely(!ret))
 			__get_user_asm_nozero(*(u16 *)(8 + (char *)dst),
 				       (u16 __user *)(8 + (char __user *)src),
 				       ret, "w", "w", "=r", 2);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	case 16:
-		__uaccess_begin_nospec();
+		__uaccess_begin();
 		__get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src,
 			       ret, "q", "", "=r", 16);
 		if (likely(!ret))
 			__get_user_asm_nozero(*(u64 *)(8 + (char *)dst),
 				       (u64 __user *)(8 + (char __user *)src),
 				       ret, "q", "", "=r", 8);
-		__uaccess_end();
+		__uaccess_end_nospec();
 		return ret;
 	default:
 		return copy_user_generic(dst, (__force void *)src, size);
diff --git a/arch/x86/lib/usercopy_32.c b/arch/x86/lib/usercopy_32.c
index 7d290777246d..b3136b75d550 100644
--- a/arch/x86/lib/usercopy_32.c
+++ b/arch/x86/lib/usercopy_32.c
@@ -331,12 +331,12 @@ do {									\
 
 unsigned long __copy_user_ll(void *to, const void *from, unsigned long n)
 {
-	__uaccess_begin_nospec();
+	__uaccess_begin();
 	if (movsl_is_ok(to, from, n))
 		__copy_user(to, from, n);
 	else
 		n = __copy_user_intel(to, from, n);
-	__uaccess_end();
+	__uaccess_end_nospec();
 	return n;
 }
 EXPORT_SYMBOL(__copy_user_ll);
@@ -344,7 +344,7 @@ EXPORT_SYMBOL(__copy_user_ll);
 unsigned long __copy_from_user_ll_nocache_nozero(void *to, const void __user *from,
 					unsigned long n)
 {
-	__uaccess_begin_nospec();
+	__uaccess_begin();
 #ifdef CONFIG_X86_INTEL_USERCOPY
 	if (n > 64 && static_cpu_has(X86_FEATURE_XMM2))
 		n = __copy_user_intel_nocache(to, from, n);
@@ -353,7 +353,7 @@ unsigned long __copy_from_user_ll_nocache_nozero(void *to, const void __user *fr
 #else
 	__copy_user(to, from, n);
 #endif
-	__uaccess_end();
+	__uaccess_end_nospec();
 	return n;
 }
 EXPORT_SYMBOL(__copy_from_user_ll_nocache_nozero);

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

On 19/11/2019 17:40, speck for Josh Poimboeuf wrote:
> Hi,
>
> What kernel changes (if any) are needed for LVI?  I haven't seen any
> discussion here.

I have similar questions when it comes to virt.  For one, EPT A/D bits
undermine any action the guest kernel takes to protect itself.

Given various pieces of academic literature on gaming the paging-out
algorithm, I'm not inclined to take the bet that an attacker couldn't
control EPT A/D bits to their advantage.

~Andrew

[MODERATED] Re: LVI

[Greg KH]

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

[MODERATED] Re: LVI

[Josh Poimboeuf]

On Tue, Nov 19, 2019 at 07:12:57PM +0100, speck for Greg KH wrote:
> > --- 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?

Notice I renamed it to from __uaccess_begin_nospec() to
__uaccess_end_nospec() ;-)

The idea is to move the LFENCE from _before_ the copy to _after_ the
copy.  I didn't mess with stac/clac.

-- 
Josh

[MODERATED] Re: LVI

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

On 19/11/19 19:12, speck for Greg KH wrote:
>> -#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?

The diff is confusing, this is now an end-of-access macro
(__uaccess_begin_nospec got renamed to __uaccess_end_nospec) so you want
to re-enable SMAP here by clearing the AC flag.

Paolo

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

On 19/11/2019 17:40, 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.

I'd phrase this slightly differently, e.g. "Attacker puts something
deliberate in a CPU buffer or L1".

It doesn't have to be a secret, or an address of a secret.  In the case
of ret hijacking, it is just the address of a disclosure gadget to
speculate to.

The important point of LVI is that a victim load ends up speculatively
using an attacker-chosen value.

> 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?

The lfence before protects from speculating into copy_from_user() with a
bad pointer.  This protection is still necessary.

An lfence afterwards would be necessary to prevent (speculatively)
returning to calling context with bad data.

On current Intel parts, stac/clac have lfence semantics, but I don't
recall if this is intended to be relied upon in future parts.

~Andrew

[MODERATED] Re: LVI

[Josh Poimboeuf]

On Tue, Nov 19, 2019 at 06:22:09PM +0000, speck for Andrew Cooper wrote:
> On 19/11/2019 17:40, 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.
> 
> I'd phrase this slightly differently, e.g. "Attacker puts something
> deliberate in a CPU buffer or L1".
> 
> It doesn't have to be a secret, or an address of a secret.  In the case
> of ret hijacking, it is just the address of a disclosure gadget to
> speculate to.
> 
> The important point of LVI is that a victim load ends up speculatively
> using an attacker-chosen value.

Good point.

> > 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?
> 
> The lfence before protects from speculating into copy_from_user() with a
> bad pointer.  This protection is still necessary.

What's the harm of speculating the copy with a bad pointer if we can
confirm there are no gadgets between the copy and the LFENCE?

> An lfence afterwards would be necessary to prevent (speculatively)
> returning to calling context with bad data.
> 
> On current Intel parts, stac/clac have lfence semantics, but I don't
> recall if this is intended to be relied upon in future parts.

Ah, that would be interesting to know..

-- 
Josh

[MODERATED] Re: LVI

[Josh Poimboeuf]

On Tue, Nov 19, 2019 at 05:51:40PM +0000, speck for Andrew Cooper wrote:
> On 19/11/2019 17:40, speck for Josh Poimboeuf wrote:
> > Hi,
> >
> > What kernel changes (if any) are needed for LVI?  I haven't seen any
> > discussion here.
> 
> I have similar questions when it comes to virt.  For one, EPT A/D bits
> undermine any action the guest kernel takes to protect itself.
> 
> Given various pieces of academic literature on gaming the paging-out
> algorithm, I'm not inclined to take the bet that an attacker couldn't
> control EPT A/D bits to their advantage.

Hm, so IIUC, that would open up every load in the guest to a potential
attack, if it has gadgets after it?  That does sound bad...

If the A/D bit control is feasible then it sounds like we'd need an
L1TF-like flushing mitigation after vmexit?  That would protect the host
kernel too.

For L1TF-affected systems would it be feasible to move the vmenter
flushing to vmexit?  Or would we need both?

-- 
Josh

[MODERATED] Re: LVI

[Luck, Tony]

On Tue, Nov 19, 2019 at 12:27:09PM -0600, speck for Josh Poimboeuf wrote:
> > On current Intel parts, stac/clac have lfence semantics, but I don't
> > recall if this is intended to be relied upon in future parts.
> 
> Ah, that would be interesting to know..

Not quite lfence semantics, but they do break speculation.

Spoiler for tomorrow's industry meeting - I'm going to ask for
folks to send us all the "promises" that we've made about architecture
through random e-mails and conversations on Keybase. Goal is to
get the SDM updated with this information so we don't have to rely
on people's recollection of these informal disclosures.

-Tony

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On 19/11/2019 18:27, speck for Josh Poimboeuf wrote:
>>> 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?
>> The lfence before protects from speculating into copy_from_user() with a
>> bad pointer.  This protection is still necessary.
> What's the harm of speculating the copy with a bad pointer if we can
> confirm there are no gadgets between the copy and the LFENCE?

Because the other hyperthread can read the data directly out of the
cache using L1TF/MDS/other as applicable.

True - this is applicable to any speculative OoB read on the first
thread so isn't unique to copy_from_user(), but copy_from_user() is very
easy for an attacker to control.

~Andrew

[MODERATED] Re: LVI

[Greg KH]

On Tue, Nov 19, 2019 at 12:21:20PM -0600, speck for Josh Poimboeuf wrote:
> On Tue, Nov 19, 2019 at 07:12:57PM +0100, speck for Greg KH wrote:
> > > --- 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?
> 
> Notice I renamed it to from __uaccess_begin_nospec() to
> __uaccess_end_nospec() ;-)
> 
> The idea is to move the LFENCE from _before_ the copy to _after_ the
> copy.  I didn't mess with stac/clac.

Ah, ick, sorry, totally mis-read that thing.  Nevermind...

greg k-h

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]

On 19/11/2019 18:27, speck for Josh Poimboeuf wrote:
> On Tue, Nov 19, 2019 at 05:51:40PM +0000, speck for Andrew Cooper wrote:
>> On 19/11/2019 17:40, speck for Josh Poimboeuf wrote:
>>> Hi,
>>>
>>> What kernel changes (if any) are needed for LVI?  I haven't seen any
>>> discussion here.
>> I have similar questions when it comes to virt.  For one, EPT A/D bits
>> undermine any action the guest kernel takes to protect itself.
>>
>> Given various pieces of academic literature on gaming the paging-out
>> algorithm, I'm not inclined to take the bet that an attacker couldn't
>> control EPT A/D bits to their advantage.
> Hm, so IIUC, that would open up every load in the guest to a potential
> attack, if it has gadgets after it?  That does sound bad...

Yes.  In the face of a malicious hypervisor playing with A/D bits, it
degrades to the SGX case of requiring an lfence between every memory
operand which may alias modulo 4k.

Of course, this case isn't interesting as the hypervisor can just read
the data it wants, and there are no Intel parts with encrypted VM
technology yet, but it is worth keeping sight of the worst case when
reasoning about how controllable A/D bits are.

In practice, it will depend largely on whether guest userspace can
influence migration and/or host paging enough to do anything useful.

> If the A/D bit control is feasible then it sounds like we'd need an
> L1TF-like flushing mitigation after vmexit?  That would protect the host
> kernel too.

EPT A/D is an optional feature, available in Broadwell and later, and
can be turned off.

The practical impact of this would be forcing these parts to use legacy
logdirty mechanisms, rather than the hardware Page Modification Logging
feature, whose implementation depends on EPT A/D being active.

That said, there may be issues speculating past an EPT_VIOLATION, which
may also play a part here (and I could probably do with a refresher on
which exits have which serialising properties, etc).

~Andrew

[MODERATED] Re: LVI

[Josh Poimboeuf]

On Tue, Nov 19, 2019 at 06:39:34PM +0000, speck for Andrew Cooper wrote:
> On 19/11/2019 18:27, speck for Josh Poimboeuf wrote:
> >>> 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?
> >> The lfence before protects from speculating into copy_from_user() with a
> >> bad pointer.  This protection is still necessary.
> > What's the harm of speculating the copy with a bad pointer if we can
> > confirm there are no gadgets between the copy and the LFENCE?
> 
> Because the other hyperthread can read the data directly out of the
> cache using L1TF/MDS/other as applicable.
> 
> True - this is applicable to any speculative OoB read on the first
> thread so isn't unique to copy_from_user(), but copy_from_user() is very
> easy for an attacker to control.

Ok, I guess had just assumed we didn't care about SMT at this point, we
should at least try to do a best effort there.

Does this match your thinking?

if (SMT && (BUG_L1TF || BUG_MDS || BUG_TAA))
	lfence();
	
... do the copy ...

if (BUG_LVI)
	lfence();

-- 
Josh

[MODERATED] Re: LVI

[Josh Poimboeuf]

On Tue, Nov 19, 2019 at 03:00:07PM -0600, speck for Josh Poimboeuf wrote:
> On Tue, Nov 19, 2019 at 06:39:34PM +0000, speck for Andrew Cooper wrote:
> > On 19/11/2019 18:27, speck for Josh Poimboeuf wrote:
> > >>> 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?
> > >> The lfence before protects from speculating into copy_from_user() with a
> > >> bad pointer.  This protection is still necessary.
> > > What's the harm of speculating the copy with a bad pointer if we can
> > > confirm there are no gadgets between the copy and the LFENCE?
> > 
> > Because the other hyperthread can read the data directly out of the
> > cache using L1TF/MDS/other as applicable.
> > 
> > True - this is applicable to any speculative OoB read on the first
> > thread so isn't unique to copy_from_user(), but copy_from_user() is very
> > easy for an attacker to control.
> 
> Ok, I guess had just assumed we didn't care about SMT at this point, we
> should at least try to do a best effort there.
> 
> Does this match your thinking?
> 
> if (SMT && (BUG_L1TF || BUG_MDS || BUG_TAA))
> 	lfence();
> 	
> ... do the copy ...
> 
> if (BUG_LVI)
> 	lfence();

Or, taking SMAP and Spectre v1 into account...

if (!SMAP && SMT && (BUG_L1TF || BUG_MDS || BUG_TAA))
 	lfence();
 	
... do the copy ...
 
if (!SMAP && (BUG_SPECTRE_V1 || BUG_LVI))
	lfence();


-- 
Josh

[MODERATED] Re: LVI

[Peter Zijlstra]

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.

How about you start by explaining what LVI is, because some of us don't
have a clue. I'm guessing it doesn't stand for Low Voltage Interface.

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

On 20/11/2019 08:04, speck for Peter Zijlstra wrote:
> 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.
> How about you start by explaining what LVI is, because some of us don't
> have a clue. I'm guessing it doesn't stand for Low Voltage Interface.

Load Value Injection.

(This is all from memory, because the DRM service appears to be offline,
and stopping me reading the appropriate PPDF on the matter.)

LVI is MDS, but with a different take who does what.

For MDS, you (the attacker) sample data by arranging for one of your own
loads to take a fault/assist, and leak the speculatively forwarded data.

For LVI, you (the attacker) arrange for a victim load to take a
fault/assist, and forward a piece of attacker chosen data from one of
the leaky microarchitectural buffers, into the otherwise correct
speculation within the victim context.

From the victims point of view, any load may end up speculating with a
maliciously chosen value.

SGX is, once again, the main target for the researchers on this one,
owing to the fact that the kernel (attacker) has complete control of the
pagetables and can arrange for an A/D bit to be set during a specific
victim load.

The current SGX recommendation for keeping your secrets secret is an
lfence between every instruction with a memory operand, including ones
which implicitly load from the stack (i.e. ret).  (Yes - you did read
this right.)


From a kernel perspective, things look rather better because the
attacker isn't in the position of having direct pagetable control. 
However, at the moment, it is still a very open ended question of "which
loads may end up taking a fault/assist to an attackers advantage".

~Andrew

[MODERATED] Re: LVI

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

On 19/11/19 19:27, speck for Josh Poimboeuf wrote:
>> For one, EPT A/D bits
>> undermine any action the guest kernel takes to protect itself.
>> 
>> Given various pieces of academic literature on gaming the paging-out
>> algorithm, I'm not inclined to take the bet that an attacker couldn't
>> control EPT A/D bits to their advantage.

It seems feasible to use a guest framebuffer (which is refreshed, and D
bits are cleared, quite a few times per second) to cause PML(*) vmexits.

> Hm, so IIUC, that would open up every load in the guest to a potential
> attack, if it has gadgets after it?  That does sound bad...
> 
> If the A/D bit control is feasible then it sounds like we'd need an
> L1TF-like flushing mitigation after vmexit?  That would protect the host
> kernel too.
> 
> For L1TF-affected systems would it be feasible to move the vmenter
> flushing to vmexit?  Or would we need both?

It would need both, the vmenter flushing is needed to hide contents from
the host kernel to the guest.  Adding a flush on vmexit would be
horrible not just for performance but also latency.

Paolo

(*) page modification log - trigger a vmexit every 512 dirty pages

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

On 19/11/2019 21:03, speck for Josh Poimboeuf wrote:
> On Tue, Nov 19, 2019 at 03:00:07PM -0600, speck for Josh Poimboeuf wrote:
>> On Tue, Nov 19, 2019 at 06:39:34PM +0000, speck for Andrew Cooper wrote:
>>> On 19/11/2019 18:27, speck for Josh Poimboeuf wrote:
>>>>>> 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?
>>>>> The lfence before protects from speculating into copy_from_user() with a
>>>>> bad pointer.  This protection is still necessary.
>>>> What's the harm of speculating the copy with a bad pointer if we can
>>>> confirm there are no gadgets between the copy and the LFENCE?
>>> Because the other hyperthread can read the data directly out of the
>>> cache using L1TF/MDS/other as applicable.
>>>
>>> True - this is applicable to any speculative OoB read on the first
>>> thread so isn't unique to copy_from_user(), but copy_from_user() is very
>>> easy for an attacker to control.
>> Ok, I guess had just assumed we didn't care about SMT at this point, we
>> should at least try to do a best effort there.
>>
>> Does this match your thinking?
>>
>> if (SMT && (BUG_L1TF || BUG_MDS || BUG_TAA))
>> 	lfence();
>> 	
>> ... do the copy ...
>>
>> if (BUG_LVI)
>> 	lfence();
> Or, taking SMAP and Spectre v1 into account...
>
> if (!SMAP && SMT && (BUG_L1TF || BUG_MDS || BUG_TAA))
>  	lfence();
>  	
> ... do the copy ...
>  
> if (!SMAP && (BUG_SPECTRE_V1 || BUG_LVI))
> 	lfence();

I'd be hesitant to depend on SMAP like this, until we get some concrete
statements on the behaviour we may rely upon from both Intel and AMD.

Their implementations are not necessarily equivalent when it comes to
hidden side effects on speculation.

~Andrew

[MODERATED] Re: LVI

[Greg KH]

On Tue, Nov 19, 2019 at 10:36:46AM -0800, speck for Luck, Tony wrote:
> Spoiler for tomorrow's industry meeting - I'm going to ask for
> folks to send us all the "promises" that we've made about architecture
> through random e-mails and conversations on Keybase. Goal is to
> get the SDM updated with this information so we don't have to rely
> on people's recollection of these informal disclosures.

I don't want to sound snarky, but wtf?

Don't you all have this all documented/written down somewhere already
that you used to tell us this in the first place?

Shouldn't you all be documenting what does, and does not, happen with
regards to flushing buffers and the like for different situations so
that you all know how to test and verify that we actually are fixing all
of this with the different OS releases?

thanks,

greg k-h

[MODERATED] Re: LVI

[Josh Poimboeuf]

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.

Luckily this has been moved to March 10, 2020.

When can we expect proper patches from Intel?

-- 
Josh

[MODERATED] Re: LVI

[Greg KH]

On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
> 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.
> 
> Luckily this has been moved to March 10, 2020.

It has?  Where is this all being tracked?

Last I saw was a strange presentation with odd dates, can't we have a
"simple" text file in keybase at the very least for this type of thing?

thanks,

greg k-h

[MODERATED] Re: LVI

[Tyler Hicks]

On 2019-11-20 18:25:29, speck for Greg KH wrote:
> On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
> > 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.
> > 
> > Luckily this has been moved to March 10, 2020.
> 
> It has?  Where is this all being tracked?
> 
> Last I saw was a strange presentation with odd dates, can't we have a
> "simple" text file in keybase at the very least for this type of thing?

It was discussed in today's call and now I see that Christian Ludloff
has updated the authorized_issues.tsv file in the top-level keybase git
tree for the team.

Tyler

> 
> thanks,
> 
> greg k-h

[MODERATED] Re: LVI

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

On 20/11/2019 17:25, speck for Greg KH wrote:
> On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
>> 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.
>> Luckily this has been moved to March 10, 2020.
> It has?

Yes, on the call which ended half an hour ago.

>   Where is this all being tracked?
>
> Last I saw was a strange presentation with odd dates, can't we have a
> "simple" text file in keybase at the very least for this type of thing?

There is.  Look at the keybase git repo.  If things are set up suitably,

git clone keybase://team/vulnresearch.2h_2019/vulnresearch.2h_2019

authorized_issues.tsv is already up to date.

~Andrew

[MODERATED] Re: LVI

[Greg KH]

On Wed, Nov 20, 2019 at 05:30:02PM +0000, speck for Andrew Cooper wrote:
> On 20/11/2019 17:25, speck for Greg KH wrote:
> > On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
> >> 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.
> >> Luckily this has been moved to March 10, 2020.
> > It has?
> 
> Yes, on the call which ended half an hour ago.
> 
> >   Where is this all being tracked?
> >
> > Last I saw was a strange presentation with odd dates, can't we have a
> > "simple" text file in keybase at the very least for this type of thing?
> 
> There is.  Look at the keybase git repo.  If things are set up suitably,
> 
> git clone keybase://team/vulnresearch.2h_2019/vulnresearch.2h_2019
> 
> authorized_issues.tsv is already up to date.

Ok, nice, _now_ it is, nothing like documenting after-the-fact :)

Anyway, why did it move?  And what is the status of patches from Intel
for this?

thanks,

greg k-h

[MODERATED] Re: LVI

[Peter Zijlstra]

On Wed, Nov 20, 2019 at 06:25:29PM +0100, speck for Greg KH wrote:
> On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
> > 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.
> > 
> > Luckily this has been moved to March 10, 2020.
> 
> It has?  Where is this all being tracked?
> 
> Last I saw was a strange presentation with odd dates, can't we have a
> "simple" text file in keybase at the very least for this type of thing?

Please, no keybase, kill and exterminate that crap.

[MODERATED] Re: LVI

[Greg KH]

On Wed, Nov 20, 2019 at 08:09:43PM +0100, speck for Peter Zijlstra wrote:
> On Wed, Nov 20, 2019 at 06:25:29PM +0100, speck for Greg KH wrote:
> > On Wed, Nov 20, 2019 at 11:13:53AM -0600, speck for Josh Poimboeuf wrote:
> > > 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.
> > > 
> > > Luckily this has been moved to March 10, 2020.
> > 
> > It has?  Where is this all being tracked?
> > 
> > Last I saw was a strange presentation with odd dates, can't we have a
> > "simple" text file in keybase at the very least for this type of thing?
> 
> Please, no keybase, kill and exterminate that crap.

For sending pictures of people drinking beer, it's fine.  For trying to
talk about technical things, it's horrid.

But, there is a git plugin and there is a repo that uses it, see the
other email for details on how to clone it.  I can live with that for
basic things like "list of issues / disclosure dates" as long as people
keep it up to date.

thanks,

greg k-h

Re: LVI

[Thomas Gleixner]

On Wed, 20 Nov 2019, speck for Greg KH wrote:
> On Wed, Nov 20, 2019 at 08:09:43PM +0100, speck for Peter Zijlstra wrote:
> > On Wed, Nov 20, 2019 at 06:25:29PM +0100, speck for Greg KH wrote:
> > > > > 
> > > > > 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.
> > > > 
> > > > Luckily this has been moved to March 10, 2020.
> > > 
> > > It has?  Where is this all being tracked?
> > > 
> > > Last I saw was a strange presentation with odd dates, can't we have a
> > > "simple" text file in keybase at the very least for this type of thing?
> > 
> > Please, no keybase, kill and exterminate that crap.
> 
> For sending pictures of people drinking beer, it's fine.  For trying to
> talk about technical things, it's horrid.
> 
> But, there is a git plugin and there is a repo that uses it, see the
> other email for details on how to clone it.  I can live with that for
> basic things like "list of issues / disclosure dates" as long as people
> keep it up to date.

This requires to have a keybase account in the first place. Which I don't
have and Peter neither. I don't have one because back then when this
keybase thingy started participation was requiring some form of social
media ID (what a retarded security concept) which I don't have.

Just for the record. We are fixing Intel's crap and it's Intel's
repsonsibility to make information accessible to us in ways which do not
force us to use randomly chosen chit-chat infrastructure which happens to
have a git repository associated. Do I have to repeat that forever?

If there is a way to submit a ssh key to get access to that git repository
without telling the farcebooks and gurgles of this world about me, then I
might be in.

Alternatively I'm happy to give write access to a speck repo branch or a
seperate repo, which is accessible to this group by old fashioned, but
halfways trustable technology, to someone who volunteers to sync that
stuff.

Thanks,

	tglx

[MODERATED] Re: LVI

[Greg KH]

On Thu, Nov 21, 2019 at 01:50:05AM +0100, speck for Thomas Gleixner wrote:
> On Wed, 20 Nov 2019, speck for Greg KH wrote:
> > On Wed, Nov 20, 2019 at 08:09:43PM +0100, speck for Peter Zijlstra wrote:
> > > On Wed, Nov 20, 2019 at 06:25:29PM +0100, speck for Greg KH wrote:
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > Luckily this has been moved to March 10, 2020.
> > > > 
> > > > It has?  Where is this all being tracked?
> > > > 
> > > > Last I saw was a strange presentation with odd dates, can't we have a
> > > > "simple" text file in keybase at the very least for this type of thing?
> > > 
> > > Please, no keybase, kill and exterminate that crap.
> > 
> > For sending pictures of people drinking beer, it's fine.  For trying to
> > talk about technical things, it's horrid.
> > 
> > But, there is a git plugin and there is a repo that uses it, see the
> > other email for details on how to clone it.  I can live with that for
> > basic things like "list of issues / disclosure dates" as long as people
> > keep it up to date.
> 
> This requires to have a keybase account in the first place. Which I don't
> have and Peter neither. I don't have one because back then when this
> keybase thingy started participation was requiring some form of social
> media ID (what a retarded security concept) which I don't have.

Fair enough, I only remember to log into it when someone mentiones
something might have changed.

> Just for the record. We are fixing Intel's crap and it's Intel's
> repsonsibility to make information accessible to us in ways which do not
> force us to use randomly chosen chit-chat infrastructure which happens to
> have a git repository associated. Do I have to repeat that forever?

I think we do, as it really does not seem to sink in :(

> If there is a way to submit a ssh key to get access to that git repository
> without telling the farcebooks and gurgles of this world about me, then I
> might be in.
> 
> Alternatively I'm happy to give write access to a speck repo branch or a
> seperate repo, which is accessible to this group by old fashioned, but
> halfways trustable technology, to someone who volunteers to sync that
> stuff.

I think an Intel person should do this if they wish to use keybase to
try to hold things we are supposed to be paying attention to.

greg k-h

[MODERATED] Re: LVI

[Andi Kleen]

Hi Folks,

We (well Tony, but he's currently on vacation) did a lot of analysis on LVI and we
concluded the kernel does not need any new changes. That's why you didn't see any
patches from Intel on this.

Longer story: 

Assists are somewhat messy and can happen in many circumstances. However most
are rare and hard to trigger, so if you get them they're typically not usable
for a high loop count practical side channel. The main exception is the page A/D
assist which can be triggered in the kernel by *_user()

*_user is protected by STAC/CLAC already and those have strong enough semantics
to stop an LVI attack outside the uaccess region. But of course there are CPUs
(pre BDW) which don't have STAC/CLAC.

But to do anything with LVI you need a Spectre v1 style read gadget. Without 
a gadget the attack is not feasible. And those gadgets are usually Spectre v1
problems, so they would need to be fixed anyways.

We already spent a lot of time looking for those in the past and fixing the few
found. Tony did an additional full tree audit, and the only additional case
found was in Infiniband. The patch for this is already upstream for some time
("61f259821dd3306e49: IB/core: Add mitigation for Spectre V1")

So in summary, on modern CPUs (BDW+) STAC/CLAC mitigates LVIs, and on older CPUs the
Spectre V1 mitigation.

The only real active (and messy) mitigation for LVI needed is when you're creating
SGX enclaves, but I assume noone here is interested in that.

-Andi

[MODERATED] Re: LVI

[Greg KH]

On Mon, Nov 25, 2019 at 04:54:17PM -0800, speck for Andi Kleen wrote:
> 
> Hi Folks,
> 
> We (well Tony, but he's currently on vacation) did a lot of analysis on LVI and we
> concluded the kernel does not need any new changes. That's why you didn't see any
> patches from Intel on this.
> 
> Longer story: 
> 
> Assists are somewhat messy and can happen in many circumstances. However most
> are rare and hard to trigger, so if you get them they're typically not usable
> for a high loop count practical side channel. The main exception is the page A/D
> assist which can be triggered in the kernel by *_user()
> 
> *_user is protected by STAC/CLAC already and those have strong enough semantics
> to stop an LVI attack outside the uaccess region. But of course there are CPUs
> (pre BDW) which don't have STAC/CLAC.
> 
> But to do anything with LVI you need a Spectre v1 style read gadget. Without 
> a gadget the attack is not feasible. And those gadgets are usually Spectre v1
> problems, so they would need to be fixed anyways.
> 
> We already spent a lot of time looking for those in the past and fixing the few
> found. Tony did an additional full tree audit, and the only additional case
> found was in Infiniband. The patch for this is already upstream for some time
> ("61f259821dd3306e49: IB/core: Add mitigation for Spectre V1")

What's to keep these types of things coming back into our tree?  Do we
have anything that can scan for them yet?

> So in summary, on modern CPUs (BDW+) STAC/CLAC mitigates LVIs, and on older CPUs the
> Spectre V1 mitigation.

So, all is good and the researchers can release their paper now and get
on with their lives?  Or is there something that we still need to do
here?

Do you have the PoC to share with us so that we can verify all of this?

> The only real active (and messy) mitigation for LVI needed is when you're creating
> SGX enclaves, but I assume noone here is interested in that.

We don't care, no, but do our users?  I think some of them used to,
before it was found to not be useful at all :)

thanks,

greg k-h

[MODERATED] Re: LVI

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On 26/11/19 01:54, speck for Andi Kleen wrote:
> 
> Hi Folks,
> 
> We (well Tony, but he's currently on vacation) did a lot of analysis on LVI and we
> concluded the kernel does not need any new changes. That's why you didn't see any
> patches from Intel on this.
> 
> Longer story: 
> 
> Assists are somewhat messy and can happen in many circumstances. However most
> are rare and hard to trigger, so if you get them they're typically not usable
> for a high loop count practical side channel. The main exception is the page A/D
> assist which can be triggered in the kernel by *_user()
> 
> *_user is protected by STAC/CLAC already and those have strong enough semantics
> to stop an LVI attack outside the uaccess region. But of course there are CPUs
> (pre BDW) which don't have STAC/CLAC.
> 
> But to do anything with LVI you need a Spectre v1 style read gadget. Without 
> a gadget the attack is not feasible. And those gadgets are usually Spectre v1
> problems, so they would need to be fixed anyways.

Don't you need only half of a Spectrev1 gadget (see the Xen advisory at
https://xenbits.xen.org/xsa/advisory-289.html and the KVM patch at
https://marc.info/?l=kvm&m=157444806904659&w=2)?  Also I assume you
didn't take into account using vmexits as an assist.

Paolo

[MODERATED] Re: LVI

[Andi Kleen]

On Tue, Nov 26, 2019 at 11:37:22AM +0100, speck for Greg KH wrote:
> > We already spent a lot of time looking for those in the past and fixing the few
> > found. Tony did an additional full tree audit, and the only additional case
> > found was in Infiniband. The patch for this is already upstream for some time
> > ("61f259821dd3306e49: IB/core: Add mitigation for Spectre V1")
> 
> What's to keep these types of things coming back into our tree?  Do we
> have anything that can scan for them yet?

AFAIK both coverity and smatch have some checks, but I'm not sure
how good they are (well and how high the false positive rate is).

So far we're mostly relying on code review.

But yes this is an area that could probably be improved.

Might be good to have a discussion on that on the public mailing list for
Spectre v1.

> 
> > So in summary, on modern CPUs (BDW+) STAC/CLAC mitigates LVIs, and on older CPUs the
> > Spectre V1 mitigation.
> 
> So, all is good and the researchers can release their paper now and get
> on with their lives?  Or is there something that we still need to do
> here?

I believe so from the kernel perspective.

> 
> Do you have the PoC to share with us so that we can verify all of this?

I don't.
> 
> > The only real active (and messy) mitigation for LVI needed is when you're creating
> > SGX enclaves, but I assume noone here is interested in that.
> 
> We don't care, no, but do our users?  I think some of them used to,
> before it was found to not be useful at all :)

Some users will care, we have SGX users using the out of tree drivers.

But this will be handled through separate avenues. The users generally
use the Intel SGX SDK to build their enclaves, and there will be a new
release of that. The SGX kernel drivers don't need any changes.

-Andi

[MODERATED] Re: LVI

[Andi Kleen]

> Don't you need only half of a Spectrev1 gadget (see the Xen advisory at
> https://xenbits.xen.org/xsa/advisory-289.html and the KVM patch at
> https://marc.info/?l=kvm&m=157444806904659&w=2)?  

You need the universal read gadget at least, which is the primary thing
a Spectre v1 audit is looking for. My understanding is for the kernel
cases it was all the same locations.

> Also I assume you didn't take into account using vmexits as an assist.

Yes that was missing from the write up, thanks. 

For vmexit assists there are two cases:

- The CPU is not MDS_NO (e.g. pre CLX) and is vulnerable to L1TF:
In this case the L1TF mitigations (L1 cache clear and/or core scheduling)
are sufficient to avoid an attacker in a guest ever seeing any side effects

- The CPU is MDS_NO (e.g. CLX): LVI can only inject 0 values, which
are not considered a security risk for normal operation.

So no extra changes needed for vmexits.

-Andi

[MODERATED] Re: LVI

[Greg KH]

On Tue, Nov 26, 2019 at 10:23:12AM -0800, speck for Andi Kleen wrote:
> On Tue, Nov 26, 2019 at 11:37:22AM +0100, speck for Greg KH wrote:
> > > We already spent a lot of time looking for those in the past and fixing the few
> > > found. Tony did an additional full tree audit, and the only additional case
> > > found was in Infiniband. The patch for this is already upstream for some time
> > > ("61f259821dd3306e49: IB/core: Add mitigation for Spectre V1")
> > 
> > What's to keep these types of things coming back into our tree?  Do we
> > have anything that can scan for them yet?
> 
> AFAIK both coverity and smatch have some checks, but I'm not sure
> how good they are (well and how high the false positive rate is).
> 
> So far we're mostly relying on code review.

So we are screwed :(

> But yes this is an area that could probably be improved.

"could"?

> Might be good to have a discussion on that on the public mailing list for
> Spectre v1.

Yes please.

> > > So in summary, on modern CPUs (BDW+) STAC/CLAC mitigates LVIs, and on older CPUs the
> > > Spectre V1 mitigation.
> > 
> > So, all is good and the researchers can release their paper now and get
> > on with their lives?  Or is there something that we still need to do
> > here?
> 
> I believe so from the kernel perspective.

Great, has Intel told the researchers this yet?  I know they were upset
about the additional date slippage, right?

> > Do you have the PoC to share with us so that we can verify all of this?
> 
> I don't.

Then how does anyone know if this really is fixed?  The researchers gave
Intel PoC for these problems, why are you not using them, and giving
them to us to test as well?

> > > The only real active (and messy) mitigation for LVI needed is when you're creating
> > > SGX enclaves, but I assume noone here is interested in that.
> > 
> > We don't care, no, but do our users?  I think some of them used to,
> > before it was found to not be useful at all :)
> 
> Some users will care, we have SGX users using the out of tree drivers.

I feel sad for those users :)

thanks,

greg k-h