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 17:40:21 -0000 Received: from us-smtp-1.mimecast.com ([207.211.31.81] helo=us-smtp-delivery-1.mimecast.com) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1iX7UB-0008CC-UB for speck@linutronix.de; Tue, 19 Nov 2019 18:40:20 +0100 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AA51B800686 for ; Tue, 19 Nov 2019 17:40:10 +0000 (UTC) Received: from treble (ovpn-124-31.rdu2.redhat.com [10.10.124.31]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 50BE466091 for ; Tue, 19 Nov 2019 17:40:10 +0000 (UTC) Date: Tue, 19 Nov 2019 11:40:08 -0600 From: Josh Poimboeuf Subject: [MODERATED] LVI Message-ID: <20191119174008.7dbymix2eo4mrv57@treble> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: 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);