[patch V2 00/10] MDS basics+ 0

[patch V2 00/10] MDS basics+ 0

[Thomas Gleixner]

Hi!

This is an update to yesterdays series with the following changes:

   - Addressed review comments (on/off list)

   - Changed the approach with static keys slightly

   - Added "cc" clobber to the VERW asm magic (spotted by Peterz)

   - Added x86 specific documentation which explains the mitigation methods
     and details on why particular code pathes are excluded.

   - Added an internal 'HOPE' mitigation mode to address the VMWare wish.

   - Added the basic infrastructure for conditional mode

Dropped the documentation patch for now as I'm not done with updating it
and I have to run now and attend my grandson's birthday party.

Thanks,

	tglx

[patch V2 01/10] MDS basics+ 1

[Thomas Gleixner]

Subject: [patch V2 01/10] x86/speculation/mds: Add basic bug infrastructure for MDS
From: Andi Kleen <ak@linux.intel.com>

Microarchitectural Data Sampling (MDS), is a class of side channel attacks
on internal buffers in Intel CPUs. The variants are:

 - Microarchitectural Store Buffer Data Sampling (MSBDS) (CVE-2018-12126)
 - Microarchitectural Fill Buffer Data Sampling (MFBDS) (CVE-2018-12130)
 - Microarchitectural Load Port Data (MLPDS) (CVE-2018-12127)

MSBDS leaks Store Buffer Entries which can be speculatively forwarded to a
dependent load (store-load forwarding) as an optimization. The forward can
also happen to a faulting or assisting load operation for a different
memory address, which can be exploited under certain conditions. Store
buffers are partitionened between Hyper-Threads so cross thread forwarding
is not possible. But if a thread enters or exits a sleep state the store
buffer is repartioned which can expose data from one thread to the other.

MFBDS leaks Fill Buffer Entries. Fill buffers are used internally to manage
L1 miss situations and to hold data which is returned or sent in response
to a memory or I/O operation. Fill buffers can forward data to a load
operation and also write data to the cache. When the fill buffer is
deallocated it can retain the stale data of the preceeding operations which
can then be forwarded to a faulting or assisting load operation, which can
be exploited under certain conditions. Fill buffers are shared between
Hyper-Threads so cross thread leakage is possible.

MLDPS leaks Load Port Data. Load ports are used to perform load operations
from memory or I/O. The received data is then forwarded to the register
file or a subsequent operation. In some implementations the Load Port can
contain stale data from a previous operation which can be forwarded to
faulting or assisting loads under certain conditions, which again can be
exploited eventually. Load poorts are shared between Hyper-Threads so cross
thread leakage is possible.

All variants have the same mitigation for single CPU thread case (SMT off),
so the kernel can treat them as one MDS issue.

Add the basic infrastructure to detect if the current CPU is affected by
MDS.

[ tglx: Rewrote changelog ]

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/include/asm/cpufeatures.h |    2 ++
 arch/x86/include/asm/msr-index.h   |    5 +++++
 arch/x86/kernel/cpu/common.c       |   13 +++++++++++++
 3 files changed, 20 insertions(+)

--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -344,6 +344,7 @@
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
 #define X86_FEATURE_AVX512_4VNNIW	(18*32+ 2) /* AVX-512 Neural Network Instructions */
 #define X86_FEATURE_AVX512_4FMAPS	(18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
+#define X86_FEATURE_MD_CLEAR		(18*32+10) /* VERW flushs CPU state */
 #define X86_FEATURE_PCONFIG		(18*32+18) /* Intel PCONFIG */
 #define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + IBPB) */
 #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect Branch Predictors */
@@ -381,5 +382,6 @@
 #define X86_BUG_SPECTRE_V2		X86_BUG(16) /* CPU is affected by Spectre variant 2 attack with indirect branches */
 #define X86_BUG_SPEC_STORE_BYPASS	X86_BUG(17) /* CPU is affected by speculative store bypass attack */
 #define X86_BUG_L1TF			X86_BUG(18) /* CPU is affected by L1 Terminal Fault */
+#define X86_BUG_MDS			X86_BUG(19) /* CPU is affected by Microarchitectural data sampling */
 
 #endif /* _ASM_X86_CPUFEATURES_H */
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -77,6 +77,11 @@
 						    * attack, so no Speculative Store Bypass
 						    * control required.
 						    */
+#define ARCH_CAP_MDS_NO			(1 << 5)   /*
+						    * Not susceptible to
+						    * Microarchitectural Data
+						    * Sampling (MDS) vulnerabilities.
+						    */
 
 #define MSR_IA32_FLUSH_CMD		0x0000010b
 #define L1D_FLUSH			(1 << 0)   /*
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -998,6 +998,14 @@ static const __initconst struct x86_cpu_
 	{}
 };
 
+static const __initconst struct x86_cpu_id cpu_no_mds[] = {
+	/* in addition to cpu_no_speculation */
+	{ X86_VENDOR_INTEL,	6,	INTEL_FAM6_ATOM_GOLDMONT	},
+	{ X86_VENDOR_INTEL,	6,	INTEL_FAM6_ATOM_GOLDMONT_X	},
+	{ X86_VENDOR_INTEL,	6,	INTEL_FAM6_ATOM_GOLDMONT_PLUS	},
+	{}
+};
+
 static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 {
 	u64 ia32_cap = 0;
@@ -1019,6 +1027,11 @@ static void __init cpu_set_bug_bits(stru
 	if (ia32_cap & ARCH_CAP_IBRS_ALL)
 		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
 
+	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+	    !x86_match_cpu(cpu_no_mds)) &&
+	    !(ia32_cap & ARCH_CAP_MDS_NO))
+		setup_force_cpu_bug(X86_BUG_MDS);
+
 	if (x86_match_cpu(cpu_no_meltdown))
 		return;
 

[MODERATED] Re: [patch V2 01/10] MDS basics+ 1

[Borislav Petkov]

This is fine-tooth comb review, round 2.

On Wed, Feb 20, 2019 at 04:07:54PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 01/10] x86/speculation/mds: Add basic bug infrastructure for MDS
> From: Andi Kleen <ak@linux.intel.com>
> 
> Microarchitectural Data Sampling (MDS), is a class of side channel attacks

s/,//

> on internal buffers in Intel CPUs. The variants are:
> 
>  - Microarchitectural Store Buffer Data Sampling (MSBDS) (CVE-2018-12126)
>  - Microarchitectural Fill Buffer Data Sampling (MFBDS) (CVE-2018-12130)
>  - Microarchitectural Load Port Data (MLPDS) (CVE-2018-12127)
				      ^
				      Sampling

> MSBDS leaks Store Buffer Entries which can be speculatively forwarded to a
> dependent load (store-load forwarding) as an optimization. The forward can

AFAIK, that's called "store-to-load" forwarding, abbreviated as STLF in
CPU speak:

https://en.wikipedia.org/wiki/Memory_disambiguation#Store_to_load_forwarding

> also happen to a faulting or assisting load operation for a different
> memory address, which can be exploited under certain conditions. Store
> buffers are partitionened between Hyper-Threads so cross thread forwarding

partitioned

> is not possible. But if a thread enters or exits a sleep state the store
> buffer is repartioned which can expose data from one thread to the other.

"... repartitioned, "

> 
> MFBDS leaks Fill Buffer Entries. Fill buffers are used internally to manage
> L1 miss situations and to hold data which is returned or sent in response
> to a memory or I/O operation. Fill buffers can forward data to a load
> operation and also write data to the cache. When the fill buffer is
> deallocated it can retain the stale data of the preceeding operations which

WARNING: 'preceeding' may be misspelled - perhaps 'preceding'?

> can then be forwarded to a faulting or assisting load operation, which can
> be exploited under certain conditions. Fill buffers are shared between
> Hyper-Threads so cross thread leakage is possible.
> 
> MLDPS leaks Load Port Data. Load ports are used to perform load operations
> from memory or I/O. The received data is then forwarded to the register
> file or a subsequent operation. In some implementations the Load Port can
> contain stale data from a previous operation which can be forwarded to
> faulting or assisting loads under certain conditions, which again can be
> exploited eventually. Load poorts are shared between Hyper-Threads so cross

ports

> thread leakage is possible.
> 
> All variants have the same mitigation for single CPU thread case (SMT off),
> so the kernel can treat them as one MDS issue.
> 
> Add the basic infrastructure to detect if the current CPU is affected by
> MDS.
> 
> [ tglx: Rewrote changelog ]
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> ---
>  arch/x86/include/asm/cpufeatures.h |    2 ++
>  arch/x86/include/asm/msr-index.h   |    5 +++++
>  arch/x86/kernel/cpu/common.c       |   13 +++++++++++++
>  3 files changed, 20 insertions(+)
> 
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -344,6 +344,7 @@
>  /* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
>  #define X86_FEATURE_AVX512_4VNNIW	(18*32+ 2) /* AVX-512 Neural Network Instructions */
>  #define X86_FEATURE_AVX512_4FMAPS	(18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
> +#define X86_FEATURE_MD_CLEAR		(18*32+10) /* VERW flushs CPU state */
							   ^
							   flushes


>  #define X86_FEATURE_PCONFIG		(18*32+18) /* Intel PCONFIG */
>  #define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + IBPB) */
>  #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect Branch Predictors */
> @@ -381,5 +382,6 @@
>  #define X86_BUG_SPECTRE_V2		X86_BUG(16) /* CPU is affected by Spectre variant 2 attack with indirect branches */
>  #define X86_BUG_SPEC_STORE_BYPASS	X86_BUG(17) /* CPU is affected by speculative store bypass attack */
>  #define X86_BUG_L1TF			X86_BUG(18) /* CPU is affected by L1 Terminal Fault */
> +#define X86_BUG_MDS			X86_BUG(19) /* CPU is affected by Microarchitectural data sampling */
>  
>  #endif /* _ASM_X86_CPUFEATURES_H */

With that addressed:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V2 01/10] MDS basics+ 1

[Greg KH]

On Wed, Feb 20, 2019 at 04:07:54PM +0100, speck for Thomas Gleixner wrote:
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -77,6 +77,11 @@
>  						    * attack, so no Speculative Store Bypass
>  						    * control required.
>  						    */
> +#define ARCH_CAP_MDS_NO			(1 << 5)   /*
> +						    * Not susceptible to
> +						    * Microarchitectural Data
> +						    * Sampling (MDS) vulnerabilities.
> +						    */

Nit, maybe BIT(5)?  I know the others in this list are not using BIT,
but other parts of this file are.

Anyway, not a big deal:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[patch V2 02/10] MDS basics+ 2

[Thomas Gleixner]

From: Andi Kleen <ak@linux.intel.com>
Subject: [patch V2 02/10] x86/kvm: Expose X86_FEATURE_MD_CLEAR to guests

X86_FEATURE_MD_CLEAR is a new CPUID bit which is set when microcode
provides the mechanism to invoke a flush of various exploitable CPU buffers
by invoking the VERW instruction.

Hand it through to guests so they can adjust their mitigations.

This also requires corresponding qemu changes, which are available
seperately.

[ tglx: Massaged changelog ]

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kvm/cpuid.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -409,7 +409,8 @@ static inline int __do_cpuid_ent(struct
 	/* cpuid 7.0.edx*/
 	const u32 kvm_cpuid_7_0_edx_x86_features =
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
-		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP);
+		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
+		F(MD_CLEAR);
 
 	/* all calls to cpuid_count() should be made on the same cpu */
 	get_cpu();

[MODERATED] Re: [patch V2 02/10] MDS basics+ 2

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:07:55PM +0100, speck for Thomas Gleixner wrote:
> From: Andi Kleen <ak@linux.intel.com>
> Subject: [patch V2 02/10] x86/kvm: Expose X86_FEATURE_MD_CLEAR to guests
> 
> X86_FEATURE_MD_CLEAR is a new CPUID bit which is set when microcode
> provides the mechanism to invoke a flush of various exploitable CPU buffers
> by invoking the VERW instruction.
> 
> Hand it through to guests so they can adjust their mitigations.
> 
> This also requires corresponding qemu changes, which are available
> seperately.

WARNING: 'seperately' may be misspelled - perhaps 'separately'?

With that:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V2 02/10] MDS basics+ 2

[Greg KH]

On Wed, Feb 20, 2019 at 04:07:55PM +0100, speck for Thomas Gleixner wrote:
> From: Andi Kleen <ak@linux.intel.com>
> Subject: [patch V2 02/10] x86/kvm: Expose X86_FEATURE_MD_CLEAR to guests
> 
> X86_FEATURE_MD_CLEAR is a new CPUID bit which is set when microcode
> provides the mechanism to invoke a flush of various exploitable CPU buffers
> by invoking the VERW instruction.
> 
> Hand it through to guests so they can adjust their mitigations.
> 
> This also requires corresponding qemu changes, which are available
> seperately.

"separately"  :)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[patch V2 03/10] MDS basics+ 3

[Thomas Gleixner]

Subject: [patch V2 03/10] x86/speculation/mds: Add mds_clear_cpu_buffer()
From: Thomas Gleixner <tglx@linutronix.de>

The Microarchitectural Data Sampling (MDS) vulernabilities are mitigated by
clearing the affected CPU buffers. The mechanism for clearing the buffers
uses the unused and obsolete VERW instruction in combination with a
microcode update which triggers a CPU buffer clear when VERW is executed.

Provide a inline function with the assembly magic. The argument of the VERW
instruction must be a memory operand.

Add x86 specific documentation about MDS and the internal workings of the
mitigation.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V1 --> V2: Add "cc" clobber and documentation
---
 Documentation/index.rst              |    1 
 Documentation/x86/conf.py            |   10 ++++
 Documentation/x86/index.rst          |    8 +++
 Documentation/x86/mds.rst            |   72 +++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/nospec-branch.h |   20 +++++++++
 5 files changed, 111 insertions(+)

--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -101,6 +101,7 @@ implementation.
    :maxdepth: 2
 
    sh/index
+   x86/index
 
 Filesystem Documentation
 ------------------------
--- /dev/null
+++ b/Documentation/x86/conf.py
@@ -0,0 +1,10 @@
+# -*- coding: utf-8; mode: python -*-
+
+project = "X86 architecture specific documentation"
+
+tags.add("subproject")
+
+latex_documents = [
+    ('index', 'x86.tex', project,
+     'The kernel development community', 'manual'),
+]
--- /dev/null
+++ b/Documentation/x86/index.rst
@@ -0,0 +1,8 @@
+==========================
+x86 architecture specifics
+==========================
+
+.. toctree::
+   :maxdepth: 1
+
+   mds
--- /dev/null
+++ b/Documentation/x86/mds.rst
@@ -0,0 +1,72 @@
+Microarchitecural Data Sampling (MDS) mitigation
+================================================
+
+Microarchitectural Data Sampling (MDS), is a class of side channel attacks
+on internal buffers in Intel CPUs. The variants are:
+
+ - Microarchitectural Store Buffer Data Sampling (MSBDS) (CVE-2018-12126)
+ - Microarchitectural Fill Buffer Data Sampling (MFBDS) (CVE-2018-12130)
+ - Microarchitectural Load Port Data (MLPDS) (CVE-2018-12127)
+
+MSBDS leaks Store Buffer Entries which can be speculatively forwarded to a
+dependent load (store-load forwarding) as an optimization. The forward can
+also happen to a faulting or assisting load operation for a different
+memory address, which can be exploited under certain conditions. Store
+buffers are partitionened between Hyper-Threads so cross thread forwarding
+is not possible. But if a thread enters or exits a sleep state the store
+buffer is repartioned which can expose data from one thread to the other.
+
+MFBDS leaks Fill Buffer Entries. Fill buffers are used internally to manage
+L1 miss situations and to hold data which is returned or sent in response
+to a memory or I/O operation. Fill buffers can forward data to a load
+operation and also write data to the cache. When the fill buffer is
+deallocated it can retain the stale data of the preceeding operations which
+can then be forwarded to a faulting or assisting load operation, which can
+be exploited under certain conditions. Fill buffers are shared between
+Hyper-Threads so cross thread leakage is possible.
+
+MLDPS leaks Load Port Data. Load ports are used to perform load operations
+from memory or I/O. The received data is then forwarded to the register
+file or a subsequent operation. In some implementations the Load Port can
+contain stale data from a previous operation which can be forwarded to
+faulting or assisting loads under certain conditions, which again can be
+exploited eventually. Load ports are shared between Hyper-Threads so cross
+thread leakage is possible.
+
+Mitigation strategy
+-------------------
+
+All variants have the same mitigation strategy at least for the single CPU
+thread case (SMT off): Force the CPU to clear the affected buffers.
+
+This is achieved by using the otherwise unused and obsolete VERW
+instruction in combination with a microcode update. The microcode clears
+the affected CPU buffers when the VERW instruction is executed.
+
+For virtualization there are two ways to achieve CPU buffer
+clearing. Either the modified VERW instruction or via the L1D Flush
+command. The latter is issued when L1TF mitigation is enabled so the extra
+VERW can be spared. If the CPU is not affected by L1TF then VERW needs to
+be issued.
+
+If the VERW instruction with the supplied segment selector argument is
+executed on a CPU without the microcode update there is no side effect
+other than a small number of pointlessly wasted CPU cycles.
+
+This does not protect against cross Hyper-Thread attacks except for MSBDS
+which is only exploitable cross Hyper-thread when one of the Hyper-Threads
+enters a C-state.
+
+The kernel provides a function to invoke the buffer clearing:
+
+    mds_clear_cpu_buffers()
+
+The mitigation is invoked on kernel/userspace, hypervisor/guest and C-state
+(idle) transitions. Depending on the mitigation mode and the system state
+the invocation can be enforced or conditional.
+
+According to current knowledge additional mitigations inside the kernel
+itself are not required because the necessary gadgets to expose the leaked
+data cannot be controlled in a way which allows exploitation from malicious
+user space or VM guests.
+
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -318,6 +318,26 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
 DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
 DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
 
+#include <asm/segment.h>
+
+/**
+ * mds_clear_cpu_buffers - Mitigation for MDS vulnerability
+ *
+ * This uses the otherwise unused and obsolete VERW instruction in
+ * combination with microcode which triggers a CPU buffer flush when the
+ * instruction is executed.
+ */
+static inline void mds_clear_cpu_buffers(void)
+{
+	static const u16 ds = __KERNEL_DS;
+
+	/*
+	 * Has to be memory form, don't modify to use a register. VERW
+	 * modifies ZF.
+	 */
+	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
+}
+
 #endif /* __ASSEMBLY__ */
 
 /*

[MODERATED] Re: [patch V2 03/10] MDS basics+ 3

[mark gross]

On Wed, Feb 20, 2019 at 04:07:56PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 03/10] x86/speculation/mds: Add mds_clear_cpu_buffer()
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> The Microarchitectural Data Sampling (MDS) vulernabilities are mitigated by
> clearing the affected CPU buffers. The mechanism for clearing the buffers
> uses the unused and obsolete VERW instruction in combination with a
> microcode update which triggers a CPU buffer clear when VERW is executed.
> 
> Provide a inline function with the assembly magic. The argument of the VERW
> instruction must be a memory operand.
> 
> Add x86 specific documentation about MDS and the internal workings of the
> mitigation.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> V1 --> V2: Add "cc" clobber and documentation
> ---
>  Documentation/index.rst              |    1 
>  Documentation/x86/conf.py            |   10 ++++
>  Documentation/x86/index.rst          |    8 +++
>  Documentation/x86/mds.rst            |   72 +++++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/nospec-branch.h |   20 +++++++++
>  5 files changed, 111 insertions(+)
> 
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -101,6 +101,7 @@ implementation.
>     :maxdepth: 2
>  
>     sh/index
> +   x86/index
>  
>  Filesystem Documentation
>  ------------------------
> --- /dev/null
> +++ b/Documentation/x86/conf.py
> @@ -0,0 +1,10 @@
> +# -*- coding: utf-8; mode: python -*-
> +
> +project = "X86 architecture specific documentation"
> +
> +tags.add("subproject")
> +
> +latex_documents = [
> +    ('index', 'x86.tex', project,
> +     'The kernel development community', 'manual'),
> +]
> --- /dev/null
> +++ b/Documentation/x86/index.rst
> @@ -0,0 +1,8 @@
> +==========================
> +x86 architecture specifics
> +==========================
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   mds
> --- /dev/null
> +++ b/Documentation/x86/mds.rst
> @@ -0,0 +1,72 @@
> +Microarchitecural Data Sampling (MDS) mitigation
> +================================================
> +
> +Microarchitectural Data Sampling (MDS), is a class of side channel attacks
> +on internal buffers in Intel CPUs. The variants are:
> +
> + - Microarchitectural Store Buffer Data Sampling (MSBDS) (CVE-2018-12126)
> + - Microarchitectural Fill Buffer Data Sampling (MFBDS) (CVE-2018-12130)
> + - Microarchitectural Load Port Data (MLPDS) (CVE-2018-12127)
> +
> +MSBDS leaks Store Buffer Entries which can be speculatively forwarded to a
> +dependent load (store-load forwarding) as an optimization. The forward can
> +also happen to a faulting or assisting load operation for a different
> +memory address, which can be exploited under certain conditions. Store
> +buffers are partitionened between Hyper-Threads so cross thread forwarding
> +is not possible. But if a thread enters or exits a sleep state the store
> +buffer is repartioned which can expose data from one thread to the other.
> +
> +MFBDS leaks Fill Buffer Entries. Fill buffers are used internally to manage
> +L1 miss situations and to hold data which is returned or sent in response
> +to a memory or I/O operation. Fill buffers can forward data to a load
> +operation and also write data to the cache. When the fill buffer is
> +deallocated it can retain the stale data of the preceeding operations which
> +can then be forwarded to a faulting or assisting load operation, which can
> +be exploited under certain conditions. Fill buffers are shared between
> +Hyper-Threads so cross thread leakage is possible.
> +
> +MLDPS leaks Load Port Data. Load ports are used to perform load operations
> +from memory or I/O. The received data is then forwarded to the register
> +file or a subsequent operation. In some implementations the Load Port can
> +contain stale data from a previous operation which can be forwarded to
> +faulting or assisting loads under certain conditions, which again can be
> +exploited eventually. Load ports are shared between Hyper-Threads so cross
> +thread leakage is possible.
> +
> +Mitigation strategy
> +-------------------
> +
> +All variants have the same mitigation strategy at least for the single CPU
> +thread case (SMT off): Force the CPU to clear the affected buffers.
> +
> +This is achieved by using the otherwise unused and obsolete VERW
> +instruction in combination with a microcode update. The microcode clears
> +the affected CPU buffers when the VERW instruction is executed.
> +
> +For virtualization there are two ways to achieve CPU buffer
> +clearing. Either the modified VERW instruction or via the L1D Flush
> +command. The latter is issued when L1TF mitigation is enabled so the extra
> +VERW can be spared. If the CPU is not affected by L1TF then VERW needs to
> +be issued.
> +
> +If the VERW instruction with the supplied segment selector argument is
> +executed on a CPU without the microcode update there is no side effect
> +other than a small number of pointlessly wasted CPU cycles.
> +
> +This does not protect against cross Hyper-Thread attacks except for MSBDS
> +which is only exploitable cross Hyper-thread when one of the Hyper-Threads
> +enters a C-state.
> +
> +The kernel provides a function to invoke the buffer clearing:
> +
> +    mds_clear_cpu_buffers()
> +
> +The mitigation is invoked on kernel/userspace, hypervisor/guest and C-state
> +(idle) transitions. Depending on the mitigation mode and the system state
> +the invocation can be enforced or conditional.
> +
> +According to current knowledge additional mitigations inside the kernel
> +itself are not required because the necessary gadgets to expose the leaked
> +data cannot be controlled in a way which allows exploitation from malicious
> +user space or VM guests.
> +
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -318,6 +318,26 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
> +#include <asm/segment.h>
> +
> +/**
> + * mds_clear_cpu_buffers - Mitigation for MDS vulnerability
> + *
> + * This uses the otherwise unused and obsolete VERW instruction in
> + * combination with microcode which triggers a CPU buffer flush when the
> + * instruction is executed.
> + */
> +static inline void mds_clear_cpu_buffers(void)
> +{
> +	static const u16 ds = __KERNEL_DS;
> +
> +	/*
> +	 * Has to be memory form, don't modify to use a register. VERW
> +	 * modifies ZF.
> +	 */
> +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
> +}
> +
>  #endif /* __ASSEMBLY__ */
>  
>  /*
>
Perhaps a dumb question but, is there any point to including that fall back ASM
altrinitive for platfroms without the uCode update enabling verw to clear
buffers?

--mark

Re: [patch V2 03/10] MDS basics+ 3

[Thomas Gleixner]

On Wed, 20 Feb 2019, speck for mark gross wrote:
> On Wed, Feb 20, 2019 at 04:07:56PM +0100, speck for Thomas Gleixner wrote:
> > +static inline void mds_clear_cpu_buffers(void)
> > +{
> > +	static const u16 ds = __KERNEL_DS;
> > +
> > +	/*
> > +	 * Has to be memory form, don't modify to use a register. VERW
> > +	 * modifies ZF.
> > +	 */
> > +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
> > +}
> > +
> >  #endif /* __ASSEMBLY__ */
> >  
> >  /*
> >
> Perhaps a dumb question but, is there any point to including that fall back ASM
> altrinitive for platfroms without the uCode update enabling verw to clear
> buffers?

See 8/10 ....

[MODERATED] Re: [patch V2 03/10] MDS basics+ 3

[mark gross]

On Wed, Feb 20, 2019 at 05:57:04PM +0100, speck for Thomas Gleixner wrote:
> On Wed, 20 Feb 2019, speck for mark gross wrote:
> > On Wed, Feb 20, 2019 at 04:07:56PM +0100, speck for Thomas Gleixner wrote:
> > > +static inline void mds_clear_cpu_buffers(void)
> > > +{
> > > +	static const u16 ds = __KERNEL_DS;
> > > +
> > > +	/*
> > > +	 * Has to be memory form, don't modify to use a register. VERW
> > > +	 * modifies ZF.
> > > +	 */
> > > +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
> > > +}
> > > +
> > >  #endif /* __ASSEMBLY__ */
> > >  
> > >  /*
> > >
> > Perhaps a dumb question but, is there any point to including that fall back ASM
> > altrinitive for platfroms without the uCode update enabling verw to clear
> > buffers?
> 
> See 8/10 ....
I was just wrapping my head around that one.  I don't like the name for that
mode but I can't think of a better one other than MDS_GUEST_CHICKEN_BIT.  Which
is worse.  

Anyway, Andi pointed out off list to me that Linus rejected the ASM sequences
that could be used if verw was not available before I was on this list.  So,
please ingore my initial comment as it was related to those.

--mark

Re: [patch V2 03/10] MDS basics+ 3

[Thomas Gleixner]

On Wed, 20 Feb 2019, speck for mark gross wrote:
> On Wed, Feb 20, 2019 at 05:57:04PM +0100, speck for Thomas Gleixner wrote:
> > On Wed, 20 Feb 2019, speck for mark gross wrote:
> > > On Wed, Feb 20, 2019 at 04:07:56PM +0100, speck for Thomas Gleixner wrote:
> > > > +static inline void mds_clear_cpu_buffers(void)
> > > > +{
> > > > +	static const u16 ds = __KERNEL_DS;
> > > > +
> > > > +	/*
> > > > +	 * Has to be memory form, don't modify to use a register. VERW
> > > > +	 * modifies ZF.
> > > > +	 */
> > > > +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
> > > > +}
> > > > +
> > > >  #endif /* __ASSEMBLY__ */
> > > >  
> > > >  /*
> > > >
> > > Perhaps a dumb question but, is there any point to including that fall back ASM
> > > altrinitive for platfroms without the uCode update enabling verw to clear
> > > buffers?
> > 
> > See 8/10 ....
> I was just wrapping my head around that one.  I don't like the name for that
> mode but I can't think of a better one other than MDS_GUEST_CHICKEN_BIT.  Which
> is worse.  

What about MDS_MITIGATION_VMWERV ?

Thanks,

	tglx

[MODERATED] Re: [patch V2 03/10] MDS basics+ 3

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:07:56PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 03/10] x86/speculation/mds: Add mds_clear_cpu_buffer()
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> The Microarchitectural Data Sampling (MDS) vulernabilities are mitigated by

vulnerabilities

> clearing the affected CPU buffers. The mechanism for clearing the buffers
> uses the unused and obsolete VERW instruction in combination with a
> microcode update which triggers a CPU buffer clear when VERW is executed.
> 
> Provide a inline function with the assembly magic. The argument of the VERW
	  an

> instruction must be a memory operand.

Do we know why it has to be a memop?

> Add x86 specific documentation about MDS and the internal workings of the
> mitigation.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> V1 --> V2: Add "cc" clobber and documentation
> ---
>  Documentation/index.rst              |    1 
>  Documentation/x86/conf.py            |   10 ++++
>  Documentation/x86/index.rst          |    8 +++
>  Documentation/x86/mds.rst            |   72 +++++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/nospec-branch.h |   20 +++++++++
>  5 files changed, 111 insertions(+)
> 
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -101,6 +101,7 @@ implementation.
>     :maxdepth: 2
>  
>     sh/index
> +   x86/index
>  
>  Filesystem Documentation
>  ------------------------
> --- /dev/null
> +++ b/Documentation/x86/conf.py
> @@ -0,0 +1,10 @@
> +# -*- coding: utf-8; mode: python -*-
> +
> +project = "X86 architecture specific documentation"
> +
> +tags.add("subproject")
> +
> +latex_documents = [
> +    ('index', 'x86.tex', project,
> +     'The kernel development community', 'manual'),
> +]
> --- /dev/null
> +++ b/Documentation/x86/index.rst
> @@ -0,0 +1,8 @@
> +==========================
> +x86 architecture specifics
> +==========================
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   mds
> --- /dev/null
> +++ b/Documentation/x86/mds.rst
> @@ -0,0 +1,72 @@
> +Microarchitecural Data Sampling (MDS) mitigation
> +================================================
> +
> +Microarchitectural Data Sampling (MDS), is a class of side channel attacks

s/,//

basically the same nitpicks as to the commit message of patch 1 :).

> +on internal buffers in Intel CPUs. The variants are:
> +
> + - Microarchitectural Store Buffer Data Sampling (MSBDS) (CVE-2018-12126)
> + - Microarchitectural Fill Buffer Data Sampling (MFBDS) (CVE-2018-12130)
> + - Microarchitectural Load Port Data (MLPDS) (CVE-2018-12127)
> +
> +MSBDS leaks Store Buffer Entries which can be speculatively forwarded to a
> +dependent load (store-load forwarding) as an optimization. The forward can
> +also happen to a faulting or assisting load operation for a different
> +memory address, which can be exploited under certain conditions. Store
> +buffers are partitionened between Hyper-Threads so cross thread forwarding
> +is not possible. But if a thread enters or exits a sleep state the store
> +buffer is repartioned which can expose data from one thread to the other.
> +
> +MFBDS leaks Fill Buffer Entries. Fill buffers are used internally to manage
> +L1 miss situations and to hold data which is returned or sent in response
> +to a memory or I/O operation. Fill buffers can forward data to a load
> +operation and also write data to the cache. When the fill buffer is
> +deallocated it can retain the stale data of the preceeding operations which

WARNING: 'preceeding' may be misspelled - perhaps 'preceding'?

> +can then be forwarded to a faulting or assisting load operation, which can
> +be exploited under certain conditions. Fill buffers are shared between
> +Hyper-Threads so cross thread leakage is possible.
> +
> +MLDPS leaks Load Port Data. Load ports are used to perform load operations
> +from memory or I/O. The received data is then forwarded to the register
> +file or a subsequent operation. In some implementations the Load Port can
> +contain stale data from a previous operation which can be forwarded to
> +faulting or assisting loads under certain conditions, which again can be
> +exploited eventually. Load ports are shared between Hyper-Threads so cross
> +thread leakage is possible.
> +
> +Mitigation strategy
> +-------------------
> +
> +All variants have the same mitigation strategy at least for the single CPU
> +thread case (SMT off): Force the CPU to clear the affected buffers.
> +
> +This is achieved by using the otherwise unused and obsolete VERW
> +instruction in combination with a microcode update. The microcode clears
> +the affected CPU buffers when the VERW instruction is executed.
> +
> +For virtualization there are two ways to achieve CPU buffer
> +clearing. Either the modified VERW instruction or via the L1D Flush
> +command. The latter is issued when L1TF mitigation is enabled so the extra
> +VERW can be spared. If the CPU is not affected by L1TF then VERW needs to

maybe s/spared/avoided/

> +be issued.
> +
> +If the VERW instruction with the supplied segment selector argument is
> +executed on a CPU without the microcode update there is no side effect
> +other than a small number of pointlessly wasted CPU cycles.
> +
> +This does not protect against cross Hyper-Thread attacks except for MSBDS
> +which is only exploitable cross Hyper-thread when one of the Hyper-Threads
> +enters a C-state.
> +
> +The kernel provides a function to invoke the buffer clearing:
> +
> +    mds_clear_cpu_buffers()
> +
> +The mitigation is invoked on kernel/userspace, hypervisor/guest and C-state
> +(idle) transitions. Depending on the mitigation mode and the system state
> +the invocation can be enforced or conditional.
> +
> +According to current knowledge additional mitigations inside the kernel
> +itself are not required because the necessary gadgets to expose the leaked
> +data cannot be controlled in a way which allows exploitation from malicious
> +user space or VM guests.
> +
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -318,6 +318,26 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
> +#include <asm/segment.h>
> +
> +/**
> + * mds_clear_cpu_buffers - Mitigation for MDS vulnerability
> + *
> + * This uses the otherwise unused and obsolete VERW instruction in
> + * combination with microcode which triggers a CPU buffer flush when the
> + * instruction is executed.
> + */
> +static inline void mds_clear_cpu_buffers(void)
> +{
> +	static const u16 ds = __KERNEL_DS;
> +
> +	/*
> +	 * Has to be memory form, don't modify to use a register. VERW
> +	 * modifies ZF.

Yeah, so did it say somewhere why it has to be memory operation? Would
be useful to have it here in the comment.

With that:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [patch V2 03/10] MDS basics+ 3

[Thomas Gleixner]

On Wed, 20 Feb 2019, speck for Borislav Petkov wrote:
> > Provide a inline function with the assembly magic. The argument of the VERW
> 	  an
> 
> > instruction must be a memory operand.
> 
> Do we know why it has to be a memop?

From the magic PDF:

  MD_CLEAR enumerates that the memory-operand variant of VERW (for example,
  VERW m16) has been extended to also overwrite buffers affected by MDS.

  This buffer overwriting functionality is not guaranteed for the register
  operand variant of VERW.

Will add some blurb.

Thanks,

	tglx

[MODERATED] Re: [patch V2 03/10] MDS basics+ 3

[Andrew Cooper]

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

On 20/02/2019 15:07, speck for Thomas Gleixner wrote:
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -318,6 +318,26 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
> +#include <asm/segment.h>
> +
> +/**
> + * mds_clear_cpu_buffers - Mitigation for MDS vulnerability
> + *
> + * This uses the otherwise unused and obsolete VERW instruction in
> + * combination with microcode which triggers a CPU buffer flush when the
> + * instruction is executed.
> + */
> +static inline void mds_clear_cpu_buffers(void)
> +{
> +	static const u16 ds = __KERNEL_DS;

In Xen, I've added a note justifying the choice of selector, in the
expectation that people probably won't remember exactly why in 6 months
time.

For least latency (allegedly to avoid a static prediction stall in
microcode), it should be a writeable data segment which is hot in the
cache, and being adjacent to __KERNEL_CS is a pretty good bet.

> +
> +	/*
> +	 * Has to be memory form, don't modify to use a register. VERW
> +	 * modifies ZF.

I don't understand why everyone is so concerned about VERW modifying
ZF.  Its not as if this fact is relevant anywhere that the mitigation is
liable to be used.

> +	 */
> +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");

The "i" (0) isn't referenced in the assembly, and can be dropped.

On a tangent, have GCC or Clang made any indication that they're going
to stop assuming that all asm() statements clobber flags, and start
making the "cc" clobber necessary on x86 targets?

~Andrew

[MODERATED] Re: [patch V2 03/10] MDS basics+ 3

[Peter Zijlstra]

On Thu, Feb 21, 2019 at 02:12:19AM +0000, speck for Andrew Cooper wrote:
> On 20/02/2019 15:07, speck for Thomas Gleixner wrote:

> > +
> > +	/*
> > +	 * Has to be memory form, don't modify to use a register. VERW
> > +	 * modifies ZF.
> 
> I don't understand why everyone is so concerned about VERW modifying
> ZF.  Its not as if this fact is relevant anywhere that the mitigation is
> liable to be used.
> 
> > +	 */
> > +	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
> 
> The "i" (0) isn't referenced in the assembly, and can be dropped.
> 
> On a tangent, have GCC or Clang made any indication that they're going
> to stop assuming that all asm() statements clobber flags, and start
> making the "cc" clobber necessary on x86 targets?

Not as far as I know, but that doesn't mean we shouldn't use it
properly.

[MODERATED] Re: Re: [patch V2 03/10] MDS basics+ 3

[Borislav Petkov]

On Thu, Feb 21, 2019 at 02:12:19AM +0000, speck for Andrew Cooper wrote:
> For least latency (allegedly to avoid a static prediction stall in
> microcode), it should be a writeable data segment which is hot in the
> cache, and being adjacent to __KERNEL_CS is a pretty good bet.

That should be in a comment somewhere.

> I don't understand why everyone is so concerned about VERW modifying
> ZF.  Its not as if this fact is relevant anywhere that the mitigation is
> liable to be used.

Err, so that gcc can know that it is clobbered? What do you mean here?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [patch V2 03/10] MDS basics+ 3

[Thomas Gleixner]

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

On Thu, 21 Feb 2019, speck for Andrew Cooper wrote:
> On 20/02/2019 15:07, speck for Thomas Gleixner wrote:
> > +static inline void mds_clear_cpu_buffers(void)
> > +{
> > +	static const u16 ds = __KERNEL_DS;
> 
> In Xen, I've added a note justifying the choice of selector, in the
> expectation that people probably won't remember exactly why in 6 months
> time.
> 
> For least latency (allegedly to avoid a static prediction stall in
> microcode), it should be a writeable data segment which is hot in the
> cache, and being adjacent to __KERNEL_CS is a pretty good bet.

I already added info to the comment. Boris asked so as well.

> > +	/*
> > +	 * Has to be memory form, don't modify to use a register. VERW
> > +	 * modifies ZF.
> 
> I don't understand why everyone is so concerned about VERW modifying
> ZF.  Its not as if this fact is relevant anywhere that the mitigation is
> liable to be used.

No, it's not relevant to the mitigation, but for correctness sake I care
about adding the "cc" clobber even if it's not officially required
today. For that matter I rather have documented why "cc" is there.

Thanks,

	tglx

[MODERATED] Re: Re: [patch V2 03/10] MDS basics+ 3

[Borislav Petkov]

On Thu, Feb 21, 2019 at 11:04:56AM +0100, speck for Thomas Gleixner wrote:
> No, it's not relevant to the mitigation, but for correctness sake I care
> about adding the "cc" clobber even if it's not officially required
> today. For that matter I rather have documented why "cc" is there.

Yeah and I read section "6.46.2.6 Clobbers and Scratch Registers"

here https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

like we should specify "cc" and it doesn't say explicitly that gcc
considers rFLAGS being clobbered in an inline asm. Or?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[patch V2 04/10] MDS basics+ 4

[Thomas Gleixner]

Subject: [patch V2 04/10] x86/speculation/mds: Clear CPU buffers on exit to user
From: Thomas Gleixner <tglx@linutronix.de>

Add a static key which controls the invocation of the CPU buffer clear
mechanism on exit to user space and add the call into
prepare_exit_to_usermode() right before actually returning.

Add documentation which kernel to user space transition this covers and
explain in detail why those which are not mitigated do not need it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/x86/mds.rst            |   79 +++++++++++++++++++++++++++++++++++
 arch/x86/entry/common.c              |    9 +++
 arch/x86/include/asm/nospec-branch.h |    2 
 arch/x86/kernel/cpu/bugs.c           |    4 +
 4 files changed, 93 insertions(+), 1 deletion(-)

--- a/Documentation/x86/mds.rst
+++ b/Documentation/x86/mds.rst
@@ -64,3 +64,82 @@ itself are not required because the nece
 data cannot be controlled in a way which allows exploitation from malicious
 user space or VM guests.
 
+Mitigation points
+-----------------
+
+1. Return to user space
+^^^^^^^^^^^^^^^^^^^^^^^
+   When transition from kernel to user space the CPU buffers are flushed
+   on affected CPUs:
+
+   - always when the mitigation mode is full. In this case the invocation
+     depends on the static key mds_user_clear_always.
+
+   - depending on executed functions between entering kernel space and
+     returning to user space. This is not yet implemented.
+
+   This covers transitions from kernel to user space through a return to
+   user space from a syscall and from an interrupt or a regular exception.
+
+   There are other kernel to user space transitions which are not covered
+   by this: NMIs and all non maskable exceptions which go through the
+   paranoid exit, which means that they are not going to the regular
+   prepare_exit_to_usermode() exit path which handles the CPU buffer
+   clearing.
+
+   The occasional non maskable exceptions which go through paranoid exit
+   are not controllable by user space in any way and most of these
+   exceptions cannot expose any valuable information either.
+
+   Neither can NMIs be reliably controlled by a non priviledged attacker
+   and their exposure to sensitive data is very limited. NMIs originate
+   from:
+
+      - Performance monitoring.
+
+	Performance monitoring is restricted by various mechanisms, i.e. a
+	regular user on a properly secured system can- if at all - only
+	monitor it's own user space processes. The performance monitoring
+	NMI surely executes priviledged kernel code and accesses kernel
+	internal data structures, which might be exploitable to break the
+	kernel's address space layout randomization, which is a non-issue
+	on affected CPUs as there are simpler ways to achieve that.
+
+      - Watchdog
+
+	The kernel uses - if enabled - a performance monitoring event to
+	trigger NMIs periodically which allow detection of hard lockups in
+	kernel space due to deadlocks or other issues.
+
+	The watchdog period is a multiple of seconds and the code path
+	executed cannot expose any secret information other than kernel
+	address space layout. Due to the low frequency and a limited
+	control of a potential attacker to align on the watchdog period the
+	attack surface is close to zero.
+
+      - Legacy oprofile NMI handler
+
+	Similar to performance monitoring, albeit potentially less
+	restricted, but has been widely replaced by the performance
+	monitoring interface perf. State of the art systems will not expose
+	the oprofile interface and even if exposed the potentially
+	exploitable information is accessible by other and simpler means.
+
+      - KGBD
+
+        If the kernel debugger is accessible by an unpriviledged attacker,
+        then the NMI handler is the least of the problems.
+
+      - ACPI/GHES
+
+        A firmware based error reporting mechanism which uses NMIs for
+        notification. Similar to Machine Check Exceptions there is no known
+        way for an attacker to reliably control and trigger errors which
+        would cause NMIs. Even if that would be the case the potentially
+        exploitable data, e.g. kernel address space layout, would be
+        accessible by simpler means.
+
+      - IPMI, vendor specific NMIs, forced shutdown NMI
+
+	None of those are controllable by unpriviledged attackers to form a
+	reliable exploit surface.
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -31,6 +31,7 @@
 #include <asm/vdso.h>
 #include <linux/uaccess.h>
 #include <asm/cpufeature.h>
+#include <asm/nospec-branch.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
@@ -180,6 +181,12 @@ static void exit_to_usermode_loop(struct
 	}
 }
 
+static inline void mds_user_clear_cpu_buffers(void)
+{
+	if (static_branch_likely(&mds_user_clear_always))
+		mds_clear_cpu_buffers();
+}
+
 /* Called with IRQs disabled. */
 __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 {
@@ -212,6 +219,8 @@ static void exit_to_usermode_loop(struct
 #endif
 
 	user_enter_irqoff();
+
+	mds_user_clear_cpu_buffers();
 }
 
 #define SYSCALL_EXIT_WORK_FLAGS				\
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -318,6 +318,8 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
 DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
 DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
 
+DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
+
 #include <asm/segment.h>
 
 /**
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -63,10 +63,12 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_i
 /* Control unconditional IBPB in switch_mm() */
 DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
 
+/* Control MDS CPU buffer clear before returning to user space */
+DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
+
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
-
 	/*
 	 * identify_boot_cpu() initialized SMT support information, let the
 	 * core code know.

[MODERATED] Re: [patch V2 04/10] MDS basics+ 4

[Greg KH]

On Wed, Feb 20, 2019 at 04:07:57PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 04/10] x86/speculation/mds: Clear CPU buffers on exit to user
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add a static key which controls the invocation of the CPU buffer clear
> mechanism on exit to user space and add the call into
> prepare_exit_to_usermode() right before actually returning.
> 
> Add documentation which kernel to user space transition this covers and
> explain in detail why those which are not mitigated do not need it.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[MODERATED] Re: [patch V2 04/10] MDS basics+ 4

[mark gross]

On Wed, Feb 20, 2019 at 04:07:57PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 04/10] x86/speculation/mds: Clear CPU buffers on exit to user
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add a static key which controls the invocation of the CPU buffer clear
> mechanism on exit to user space and add the call into
> prepare_exit_to_usermode() right before actually returning.
> 
> Add documentation which kernel to user space transition this covers and
> explain in detail why those which are not mitigated do not need it.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  Documentation/x86/mds.rst            |   79 +++++++++++++++++++++++++++++++++++
>  arch/x86/entry/common.c              |    9 +++
>  arch/x86/include/asm/nospec-branch.h |    2 
>  arch/x86/kernel/cpu/bugs.c           |    4 +
>  4 files changed, 93 insertions(+), 1 deletion(-)
> 
> --- a/Documentation/x86/mds.rst
> +++ b/Documentation/x86/mds.rst
> @@ -64,3 +64,82 @@ itself are not required because the nece
>  data cannot be controlled in a way which allows exploitation from malicious
>  user space or VM guests.
>  
> +Mitigation points
> +-----------------
> +
> +1. Return to user space
> +^^^^^^^^^^^^^^^^^^^^^^^
> +   When transition from kernel to user space the CPU buffers are flushed
> +   on affected CPUs:
> +
> +   - always when the mitigation mode is full. In this case the invocation
> +     depends on the static key mds_user_clear_always.
> +
> +   - depending on executed functions between entering kernel space and
> +     returning to user space. This is not yet implemented.
> +
> +   This covers transitions from kernel to user space through a return to
> +   user space from a syscall and from an interrupt or a regular exception.
> +
> +   There are other kernel to user space transitions which are not covered
> +   by this: NMIs and all non maskable exceptions which go through the
> +   paranoid exit, which means that they are not going to the regular
> +   prepare_exit_to_usermode() exit path which handles the CPU buffer
> +   clearing.
> +
> +   The occasional non maskable exceptions which go through paranoid exit
> +   are not controllable by user space in any way and most of these
> +   exceptions cannot expose any valuable information either.
> +
> +   Neither can NMIs be reliably controlled by a non priviledged attacker
> +   and their exposure to sensitive data is very limited. NMIs originate
> +   from:
> +
> +      - Performance monitoring.
> +
> +	Performance monitoring is restricted by various mechanisms, i.e. a
> +	regular user on a properly secured system can- if at all - only
> +	monitor it's own user space processes. The performance monitoring
> +	NMI surely executes priviledged kernel code and accesses kernel
> +	internal data structures, which might be exploitable to break the
> +	kernel's address space layout randomization, which is a non-issue
> +	on affected CPUs as there are simpler ways to achieve that.
> +
> +      - Watchdog
> +
> +	The kernel uses - if enabled - a performance monitoring event to
> +	trigger NMIs periodically which allow detection of hard lockups in
> +	kernel space due to deadlocks or other issues.
> +
> +	The watchdog period is a multiple of seconds and the code path
> +	executed cannot expose any secret information other than kernel
> +	address space layout. Due to the low frequency and a limited
> +	control of a potential attacker to align on the watchdog period the
> +	attack surface is close to zero.
> +
> +      - Legacy oprofile NMI handler
> +
> +	Similar to performance monitoring, albeit potentially less
> +	restricted, but has been widely replaced by the performance
> +	monitoring interface perf. State of the art systems will not expose
> +	the oprofile interface and even if exposed the potentially
> +	exploitable information is accessible by other and simpler means.
> +
> +      - KGBD
> +
> +        If the kernel debugger is accessible by an unpriviledged attacker,
> +        then the NMI handler is the least of the problems.
> +
> +      - ACPI/GHES
> +
> +        A firmware based error reporting mechanism which uses NMIs for
> +        notification. Similar to Machine Check Exceptions there is no known
> +        way for an attacker to reliably control and trigger errors which
> +        would cause NMIs. Even if that would be the case the potentially
> +        exploitable data, e.g. kernel address space layout, would be
> +        accessible by simpler means.
> +
> +      - IPMI, vendor specific NMIs, forced shutdown NMI
> +
> +	None of those are controllable by unpriviledged attackers to form a
> +	reliable exploit surface.
I agree we need some balance between paranoia and reality.  

However; if I'm being pedantic, the attacker not having controlability aspect
of your argument can apply to most aspects of the MDS vulnerability.  I think
that's why its name uses "data sampling".  Also, I need to ask the chip heads
about if this list of NMI's is complete and can be expected to stay that way
across processor and platfrom generations.

--mark

> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -31,6 +31,7 @@
>  #include <asm/vdso.h>
>  #include <linux/uaccess.h>
>  #include <asm/cpufeature.h>
> +#include <asm/nospec-branch.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/syscalls.h>
> @@ -180,6 +181,12 @@ static void exit_to_usermode_loop(struct
>  	}
>  }
>  
> +static inline void mds_user_clear_cpu_buffers(void)
> +{
> +	if (static_branch_likely(&mds_user_clear_always))
> +		mds_clear_cpu_buffers();
> +}
> +
>  /* Called with IRQs disabled. */
>  __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
>  {
> @@ -212,6 +219,8 @@ static void exit_to_usermode_loop(struct
>  #endif
>  
>  	user_enter_irqoff();
> +
> +	mds_user_clear_cpu_buffers();
>  }
>  
>  #define SYSCALL_EXIT_WORK_FLAGS				\
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -318,6 +318,8 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
> +DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
> +
>  #include <asm/segment.h>
>  
>  /**
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -63,10 +63,12 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_i
>  /* Control unconditional IBPB in switch_mm() */
>  DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
> +/* Control MDS CPU buffer clear before returning to user space */
> +DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
> +
>  void __init check_bugs(void)
>  {
>  	identify_boot_cpu();
> -
>  	/*
>  	 * identify_boot_cpu() initialized SMT support information, let the
>  	 * core code know.
> 

[MODERATED] Encrypted Message

[Tim Chen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/rfc822-headers; protected-headers="v1", Size: 135 bytes --]

From: Tim Chen <tim.c.chen@linux.intel.com>
To: speck for mark gross <speck@linutronix.de>
Subject: Re: [patch V2 04/10] MDS basics+ 4

[-- Attachment #2: Type: text/plain, Size: 829 bytes --]

On 2/20/19 9:10 AM, speck for mark gross wrote:

>> +
>> +      - KGBD

s/KGBD/KGDB

>> +
>> +        If the kernel debugger is accessible by an unpriviledged attacker,
>> +        then the NMI handler is the least of the problems.
>> +

...

> 
> However; if I'm being pedantic, the attacker not having controlability aspect
> of your argument can apply to most aspects of the MDS vulnerability.  I think
> that's why its name uses "data sampling".  Also, I need to ask the chip heads
> about if this list of NMI's is complete and can be expected to stay that way
> across processor and platfrom generations.
> 
> --mark
> 


I don't think any of the code paths listed touches any user data.  So even
if an attacker have some means to control NMI, he won't get any useful data.

Thanks.

Tim 

Re: Encrypted Message

[Thomas Gleixner]

Tim,

On Thu, 21 Feb 2019, speck for Tim Chen wrote:

> On 2/20/19 9:10 AM, speck for mark gross wrote:

> > However; if I'm being pedantic, the attacker not having controlability aspect
> > of your argument can apply to most aspects of the MDS vulnerability.  I think
> > that's why its name uses "data sampling".  Also, I need to ask the chip heads
> > about if this list of NMI's is complete and can be expected to stay that way
> > across processor and platfrom generations.

> I don't think any of the code paths listed touches any user data.  So
> even if an attacker have some means to control NMI, he won't get any
> useful data.

Can you please stop to tout this 'user data' mantra?

It's a completely unspecified term. Several people have asked what it
means, the answer until today is deafening silence.

Same applies for 'useful data'. What's useful data? That largely depends on
what the attacker is after.

Engineering 101:

   Correctness first

   Definititons first

not

   Performance first

   Unspecified assumptions and buzzwords first

The whole mess we are forced to deal with originates from the latter two.

Thanks,

	tglx

[MODERATED] Re: Encrypted Message

[Jiri Kosina]

On Thu, 21 Feb 2019, speck for Tim Chen wrote:

> I don't think any of the code paths listed touches any user data.  So 
> even if an attacker have some means to control NMI, he won't get any 
> useful data.

Tim,

so how about the kernel stack contents I mentioned yesterday?

Thanks,

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: [patch V2 04/10] MDS basics+ 4

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:07:57PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 04/10] x86/speculation/mds: Clear CPU buffers on exit to user
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add a static key which controls the invocation of the CPU buffer clear
> mechanism on exit to user space and add the call into
> prepare_exit_to_usermode() right before actually returning.
> 
> Add documentation which kernel to user space transition this covers and
> explain in detail why those which are not mitigated do not need it.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  Documentation/x86/mds.rst            |   79 +++++++++++++++++++++++++++++++++++
>  arch/x86/entry/common.c              |    9 +++
>  arch/x86/include/asm/nospec-branch.h |    2 
>  arch/x86/kernel/cpu/bugs.c           |    4 +
>  4 files changed, 93 insertions(+), 1 deletion(-)
> 
> --- a/Documentation/x86/mds.rst
> +++ b/Documentation/x86/mds.rst
> @@ -64,3 +64,82 @@ itself are not required because the nece
>  data cannot be controlled in a way which allows exploitation from malicious
>  user space or VM guests.
>  
> +Mitigation points
> +-----------------
> +
> +1. Return to user space
> +^^^^^^^^^^^^^^^^^^^^^^^
> +   When transition from kernel to user space the CPU buffers are flushed

transitioning

With that:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V2 04/10] MDS basics+ 4

[Jiri Kosina]

On Wed, 20 Feb 2019, speck for Thomas Gleixner wrote:

> +   Neither can NMIs be reliably controlled by a non priviledged attacker
> +   and their exposure to sensitive data is very limited. NMIs originate
> +   from:
[ ... snip ... ]
> +	None of those are controllable by unpriviledged attackers to form a
> +	reliable exploit surface.

One thing where I am not completely sure about this at the moment, is NMI 
that's used to trigger backtrace on all CPUs.

In a hypothetical case where we have a situation where unprivileged user 
can (due to some other issue) controllably trigger either of:

- hung task situation
- rcu stall
- hardlockup

then MDS turns this into revealing contents of kernel stacks (as the 
stacktrace unwinder will walk the whole thing between the stack top and 
%rsp) of all CPUs.

-- 
Jiri Kosina
SUSE Labs

Re: [patch V2 04/10] MDS basics+ 4

[Thomas Gleixner]

Jiri,

On Wed, 20 Feb 2019, speck for Jiri Kosina wrote:

> On Wed, 20 Feb 2019, speck for Thomas Gleixner wrote:
> 
> > +   Neither can NMIs be reliably controlled by a non priviledged attacker
> > +   and their exposure to sensitive data is very limited. NMIs originate
> > +   from:
> [ ... snip ... ]
> > +	None of those are controllable by unpriviledged attackers to form a
> > +	reliable exploit surface.
> 
> One thing where I am not completely sure about this at the moment, is NMI 
> that's used to trigger backtrace on all CPUs.
> 
> In a hypothetical case where we have a situation where unprivileged user 
> can (due to some other issue) controllably trigger either of:
> 
> - hung task situation
> - rcu stall
> - hardlockup
> 
> then MDS turns this into revealing contents of kernel stacks (as the 
> stacktrace unwinder will walk the whole thing between the stack top and 
> %rsp) of all CPUs.

As I said to Andi, if the consensus is that full == paranoid, lets just add
it and be done with it. One flush more or less is probably not buying us
anything.

Thanks,

	tglx

[patch V2 05/10] MDS basics+ 5

[Thomas Gleixner]

Subject: [patch V2 05/10] x86/speculation/mds: Conditionaly clear CPU buffers on idle entry
From: Thomas Gleixner <tglx@linutronix.de>

Add a static key which controls the invocation of the CPU buffer clear
mechanism on idle entry. This is independent of other MDS mitigations
because the idle entry invocation to mitigate the potential leakage due to
store buffer repartitioning is only necessary on SMT systems.

Add the actual invocations to the different halt/mwait variants which
covers all usage sites. mwaitx is not patched as it's not available on
Intel CPUs.

The buffer clear is only invoked before entering the C-State to prevent
that stale data from the idling CPU can be spilled to the Hyper-Thread
sibling after the Store buffer got repartitioned and all entries are
available to the non idle sibling.

When coming out of idle the store buffer is partitioned again so each
sibling has half of it available. Now the back from idle CPU could be
speculatively exposed to contents of the sibling, but the buffers are
flushed either on exit to user space or on VMENTER.

When later on conditional buffer clearing is implemented on top of this,
then there is no action required either because before returning to user
space the context switch will set the condition flag which causes a flush
on the return to user path.

This intentionaly does not handle the case in the acpi/processor_idle
driver which uses the legacy IO port interface for C-State transitions for
two reasons:

 - The acpi/processor_idle driver was replaced by the intel_idle driver
   almost a decade ago. Anything Nehalem upwards supports it and defaults
   to that new driver.

 - The legacy IO port interface is likely to be used on older and therefore
   unaffected CPUs or on systems which do not receive microcode updates
   anymore, so there is no point in adding that.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/x86/mds.rst            |   33 +++++++++++++++++++++++++++++++++
 arch/x86/include/asm/irqflags.h      |    4 ++++
 arch/x86/include/asm/mwait.h         |    7 +++++++
 arch/x86/include/asm/nospec-branch.h |   12 ++++++++++++
 arch/x86/kernel/cpu/bugs.c           |    2 ++
 5 files changed, 58 insertions(+)

--- a/Documentation/x86/mds.rst
+++ b/Documentation/x86/mds.rst
@@ -143,3 +143,36 @@ Mitigation points
 
 	None of those are controllable by unpriviledged attackers to form a
 	reliable exploit surface.
+
+2. C-State transition
+^^^^^^^^^^^^^^^^^^^^^
+
+   When a CPU goes idle and enters a C-State the CPU buffers need to be
+   cleared on affected CPUs when SMT is active. This addresses the
+   repartitioning of the Store buffer when one of the Hyper-Thread enters a
+   C-State.
+
+   When SMT is inactive, i.e. either the CPU does not support it or all
+   sibling threads are offline CPU buffer clearing is not required.
+
+   The invocation is controlled by the static key mds_idle_clear which is
+   switched depending on the chosen mitigation mode and the SMT state of
+   the system.
+
+   The buffer clear is only invoked before entering the C-State to prevent
+   that stale data from the idling CPU can be spilled to the Hyper-Thread
+   sibling after the Store buffer got repartitioned and all entries are
+   available to the non idle sibling.
+
+   When coming out of idle the store buffer is partitioned again so each
+   sibling has half of it available. Now the back from idle CPU could be
+   speculatively exposed to contents of the sibling, but the buffers are
+   flushed either on exit to user space or on VMENTER.
+
+   The mitigation is hooked into all variants of halt()/mwait(), but does
+   not cover the legacy ACPI IO-Port mechanism because the ACPI idle driver
+   has been superseeded by the intel_idle driver around 2010 and is
+   preferred on all affected CPUs which still receive microcode updates
+   (Nehalem onwards). Aside of that the IO-Port mechanism is a legacy
+   interface which is only used on older systems which are either not
+   affected or do not receive microcode updates anymore.
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -6,6 +6,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/nospec-branch.h>
+
 /* Provide __cpuidle; we can't safely include <linux/cpu.h> */
 #define __cpuidle __attribute__((__section__(".cpuidle.text")))
 
@@ -54,11 +56,13 @@ static inline void native_irq_enable(voi
 
 static inline __cpuidle void native_safe_halt(void)
 {
+	mds_idle_clear_cpu_buffers();
 	asm volatile("sti; hlt": : :"memory");
 }
 
 static inline __cpuidle void native_halt(void)
 {
+	mds_idle_clear_cpu_buffers();
 	asm volatile("hlt": : :"memory");
 }
 
--- a/arch/x86/include/asm/mwait.h
+++ b/arch/x86/include/asm/mwait.h
@@ -6,6 +6,7 @@
 #include <linux/sched/idle.h>
 
 #include <asm/cpufeature.h>
+#include <asm/nospec-branch.h>
 
 #define MWAIT_SUBSTATE_MASK		0xf
 #define MWAIT_CSTATE_MASK		0xf
@@ -40,6 +41,8 @@ static inline void __monitorx(const void
 
 static inline void __mwait(unsigned long eax, unsigned long ecx)
 {
+	mds_idle_clear_cpu_buffers();
+
 	/* "mwait %eax, %ecx;" */
 	asm volatile(".byte 0x0f, 0x01, 0xc9;"
 		     :: "a" (eax), "c" (ecx));
@@ -74,6 +77,8 @@ static inline void __mwait(unsigned long
 static inline void __mwaitx(unsigned long eax, unsigned long ebx,
 			    unsigned long ecx)
 {
+	/* No MDS buffer clear as this is AMD/HYGON only */
+
 	/* "mwaitx %eax, %ebx, %ecx;" */
 	asm volatile(".byte 0x0f, 0x01, 0xfb;"
 		     :: "a" (eax), "b" (ebx), "c" (ecx));
@@ -81,6 +86,8 @@ static inline void __mwaitx(unsigned lon
 
 static inline void __sti_mwait(unsigned long eax, unsigned long ecx)
 {
+	mds_idle_clear_cpu_buffers();
+
 	trace_hardirqs_on();
 	/* "mwait %eax, %ecx;" */
 	asm volatile("sti; .byte 0x0f, 0x01, 0xc9;"
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -319,6 +319,7 @@ DECLARE_STATIC_KEY_FALSE(switch_mm_cond_
 DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
 
 DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
+DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
 
 #include <asm/segment.h>
 
@@ -340,6 +341,17 @@ static inline void mds_clear_cpu_buffers
 	asm volatile("verw %[ds]" : : "i" (0), [ds] "m" (ds) : "cc");
 }
 
+/**
+ * mds_idle_clear_cpu_buffers - Mitigation for MDS vulnerability
+ *
+ * Clear CPU buffers if the corresponding static key is enabled
+ */
+static inline void mds_idle_clear_cpu_buffers(void)
+{
+	if (static_branch_likely(&mds_idle_clear))
+		mds_clear_cpu_buffers();
+}
+
 #endif /* __ASSEMBLY__ */
 
 /*
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -65,6 +65,8 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_always
 
 /* Control MDS CPU buffer clear before returning to user space */
 DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
+/* Control MDS CPU buffer clear before idling (halt, mwait) */
+DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
 
 void __init check_bugs(void)
 {

[MODERATED] Re: [patch V2 05/10] MDS basics+ 5

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:07:58PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 05/10] x86/speculation/mds: Conditionaly clear CPU buffers on idle entry

WARNING: 'Conditionaly' may be misspelled - perhaps 'Conditionally'?

> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add a static key which controls the invocation of the CPU buffer clear
> mechanism on idle entry. This is independent of other MDS mitigations
> because the idle entry invocation to mitigate the potential leakage due to
> store buffer repartitioning is only necessary on SMT systems.
> 
> Add the actual invocations to the different halt/mwait variants which
> covers all usage sites. mwaitx is not patched as it's not available on
> Intel CPUs.
> 
> The buffer clear is only invoked before entering the C-State to prevent
> that stale data from the idling CPU can be spilled to the Hyper-Thread

s/can //

> sibling after the Store buffer got repartitioned and all entries are
> available to the non idle sibling.
> 
> When coming out of idle the store buffer is partitioned again so each
> sibling has half of it available. Now the back from idle CPU could be

"Now the CPU which returned from idle... "

> speculatively exposed to contents of the sibling, but the buffers are
> flushed either on exit to user space or on VMENTER.
> 
> When later on conditional buffer clearing is implemented on top of this,
> then there is no action required either because before returning to user
> space the context switch will set the condition flag which causes a flush
> on the return to user path.
> 
> This intentionaly does not handle the case in the acpi/processor_idle
> driver which uses the legacy IO port interface for C-State transitions for
> two reasons:
> 
>  - The acpi/processor_idle driver was replaced by the intel_idle driver
>    almost a decade ago. Anything Nehalem upwards supports it and defaults
>    to that new driver.
> 
>  - The legacy IO port interface is likely to be used on older and therefore
>    unaffected CPUs or on systems which do not receive microcode updates
>    anymore, so there is no point in adding that.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  Documentation/x86/mds.rst            |   33 +++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/irqflags.h      |    4 ++++
>  arch/x86/include/asm/mwait.h         |    7 +++++++
>  arch/x86/include/asm/nospec-branch.h |   12 ++++++++++++
>  arch/x86/kernel/cpu/bugs.c           |    2 ++
>  5 files changed, 58 insertions(+)
> 
> --- a/Documentation/x86/mds.rst
> +++ b/Documentation/x86/mds.rst
> @@ -143,3 +143,36 @@ Mitigation points
>  
>  	None of those are controllable by unpriviledged attackers to form a
>  	reliable exploit surface.
> +
> +2. C-State transition
> +^^^^^^^^^^^^^^^^^^^^^
> +
> +   When a CPU goes idle and enters a C-State the CPU buffers need to be
> +   cleared on affected CPUs when SMT is active. This addresses the
> +   repartitioning of the Store buffer when one of the Hyper-Thread enters a

"... one of the Hyper-Threads... "

> +   C-State.
> +
> +   When SMT is inactive, i.e. either the CPU does not support it or all
> +   sibling threads are offline CPU buffer clearing is not required.
> +
> +   The invocation is controlled by the static key mds_idle_clear which is
> +   switched depending on the chosen mitigation mode and the SMT state of
> +   the system.
> +
> +   The buffer clear is only invoked before entering the C-State to prevent
> +   that stale data from the idling CPU can be spilled to the Hyper-Thread
> +   sibling after the Store buffer got repartitioned and all entries are
> +   available to the non idle sibling.
> +
> +   When coming out of idle the store buffer is partitioned again so each
> +   sibling has half of it available. Now the back from idle CPU could be
> +   speculatively exposed to contents of the sibling, but the buffers are
> +   flushed either on exit to user space or on VMENTER.
> +
> +   The mitigation is hooked into all variants of halt()/mwait(), but does
> +   not cover the legacy ACPI IO-Port mechanism because the ACPI idle driver
> +   has been superseeded by the intel_idle driver around 2010 and is

WARNING: 'superseeded' may be misspelled - perhaps 'superseded'?

> +   preferred on all affected CPUs which still receive microcode updates
> +   (Nehalem onwards). Aside of that the IO-Port mechanism is a legacy
> +   interface which is only used on older systems which are either not
> +   affected or do not receive microcode updates anymore.

With that addressed:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V2 05/10] MDS basics+ 5

[Andrew Cooper]

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

On 20/02/2019 15:07, speck for Thomas Gleixner wrote:
> --- a/Documentation/x86/mds.rst
> +++ b/Documentation/x86/mds.rst
> @@ -143,3 +143,36 @@ Mitigation points
>  
>  	None of those are controllable by unpriviledged attackers to form a
>  	reliable exploit surface.
> +
> +2. C-State transition
> +^^^^^^^^^^^^^^^^^^^^^
> +
> +   When a CPU goes idle and enters a C-State the CPU buffers need to be
> +   cleared on affected CPUs when SMT is active. This addresses the
> +   repartitioning of the Store buffer when one of the Hyper-Thread enters a
> +   C-State.
> +
> +   When SMT is inactive, i.e. either the CPU does not support it or all
> +   sibling threads are offline CPU buffer clearing is not required.
> +
> +   The invocation is controlled by the static key mds_idle_clear which is
> +   switched depending on the chosen mitigation mode and the SMT state of
> +   the system.
> +
> +   The buffer clear is only invoked before entering the C-State to prevent
> +   that stale data from the idling CPU can be spilled to the Hyper-Thread
> +   sibling after the Store buffer got repartitioned and all entries are
> +   available to the non idle sibling.
> +
> +   When coming out of idle the store buffer is partitioned again so each
> +   sibling has half of it available. Now the back from idle CPU could be
> +   speculatively exposed to contents of the sibling, but the buffers are
> +   flushed either on exit to user space or on VMENTER.
> +
> +   The mitigation is hooked into all variants of halt()/mwait(), but does
> +   not cover the legacy ACPI IO-Port mechanism because the ACPI idle driver
> +   has been superseeded by the intel_idle driver around 2010 and is
> +   preferred on all affected CPUs which still receive microcode updates
> +   (Nehalem onwards).

I'd perhaps phrase this as "and is preferred on all CPUs which are
expected to gain the MD_CLEAR functionality in microcode".

ISTR there were some pre-Nehalem ucode updates for one of the
speculative issues (although I'm at a loss to locate the schedule
document I think the answer is in).

~Andrew

Re: [patch V2 05/10] MDS basics+ 5

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Andrew Cooper wrote:
> > +   The mitigation is hooked into all variants of halt()/mwait(), but does
> > +   not cover the legacy ACPI IO-Port mechanism because the ACPI idle driver
> > +   has been superseeded by the intel_idle driver around 2010 and is
> > +   preferred on all affected CPUs which still receive microcode updates
> > +   (Nehalem onwards).
> 
> I'd perhaps phrase this as "and is preferred on all CPUs which are
> expected to gain the MD_CLEAR functionality in microcode".
> 
> ISTR there were some pre-Nehalem ucode updates for one of the
> speculative issues (although I'm at a loss to locate the schedule
> document I think the answer is in).

One of those "supersecure" documents which I can't work with again....

But that aside the real question is whether it actually matters. Though
that's a tough one to answer because at least I have no clue what the
actual attack surface is.

Maybe it's described in some other !$!%@$%!@ document or this has been
discussed between cat pictures, lame jokes and beer appointments on that
security consecrated whatsapp mimicry.

While I think to know how to exploit that in C/ASM, I have no idea whether
that stuff is exploitable by other means.

  1) What's the state of javascript?

     If that's not an issue, then we really can stop worrying about pre
     Nehalem for reality sake. If it's an issue we need at have some
     information for Pavel at least. :)

  2) What about BPF?

     If untrusted BPF can be abused to exploit this, then we have to think
     hard about howto handle that.

     Assume the following:

     User A -> BPF -> preemption -> kernel thread -> back to BPF

     Because there is no kernel to user transition no buffer clear will
     happen - neither in full nor in conditional mode.

     So the BPF sampler can happily collect the leftovers of the kernel
     thread.

     Might be theoretical, but it needs to be investigated at least.

Thanks,

	tglx

Re: [patch V2 05/10] MDS basics+ 5

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Thomas Gleixner wrote:
>   2) What about BPF?
> 
>      If untrusted BPF can be abused to exploit this, then we have to think
>      hard about howto handle that.
> 
>      Assume the following:
> 
>      User A -> BPF -> preemption -> kernel thread -> back to BPF
> 
>      Because there is no kernel to user transition no buffer clear will
>      happen - neither in full nor in conditional mode.
> 
>      So the BPF sampler can happily collect the leftovers of the kernel
>      thread.
> 
>      Might be theoretical, but it needs to be investigated at least.

Just learned that BPF runs with preemption disabled. Oh well, yet another
thing which can't be used with Realtime systems. Well thought out.

Though the

       BPF -> interrupt -> softinterrupt -> BPF

scenario remains. It's not covered by full because it's not a user exit....

Thanks,

	tglx

[MODERATED] Attack Surface [Was [patch V2 05/10] MDS basics+ 5]

[Andrew Cooper]

On 21/02/2019 10:36, speck for Thomas Gleixner wrote:
> On Thu, 21 Feb 2019, speck for Andrew Cooper wrote:
>>> +   The mitigation is hooked into all variants of halt()/mwait(), but does
>>> +   not cover the legacy ACPI IO-Port mechanism because the ACPI idle driver
>>> +   has been superseeded by the intel_idle driver around 2010 and is
>>> +   preferred on all affected CPUs which still receive microcode updates
>>> +   (Nehalem onwards).
>> I'd perhaps phrase this as "and is preferred on all CPUs which are
>> expected to gain the MD_CLEAR functionality in microcode".
>>
>> ISTR there were some pre-Nehalem ucode updates for one of the
>> speculative issues (although I'm at a loss to locate the schedule
>> document I think the answer is in).
> One of those "supersecure" documents which I can't work with again....

Actually just a plain PDF - the problem is the hundreds of
almost-identical ones I've got from around the same period.

> But that aside the real question is whether it actually matters. Though
> that's a tough one to answer because at least I have no clue what the
> actual attack surface is.

I'm afraid that is a little complicated.  This is to the best of my
understanding...

1) The load and fill buffers issues let an attacker observe stale data
from memory reads and writes which have just occurred in the core.

The single threaded case isn't very interesting - the exit to guest path
most likely clobbers anything interesting, and this is probably also
true of a javascript engine switching between logical contexts.  I
personally think you need an overly-contrived scenario to leak
interesting data in this case.

The hyperthreading case is very interesting however.  An attacker can
read all memory operands from the sibling thread as it is executing. 
Combined with analysis techniques such as those presented in the TLBleed
paper, this is a potentially very powerful attack.

There is an existing PoC from one of the researchers which can leak
arbitrary kernel memory using setrlimit() on one thread and observing
the fill buffers on the other thread.

2) The store buffer issue is much more complicated to reason about. 
Ignoring the idle rebalancing corner case, it is restricted to a single
threaded case.

An attacker can read stale data from the store buffer.  The store buffer
is as wide as the SIMD pipeline in the core, but memory writes don't
zero-extend and overwrite the full width of the buffer.

Therefore, while the bottom 64 bits get modified very frequently, the
upper bits are only modified by a full-width SIMD write, or a
long-enough string operation when FAST_STRINGS is enabled, or from
XSAVE/etc.

The prevailing concern is that an attacker can read most of the vector
register state of the previous task to be scheduled on the thread, which
could be very valuable information if the previous task was using AES-NI.

> Maybe it's described in some other !$!%@$%!@ document or this has been
> discussed between cat pictures, lame jokes and beer appointments on that
> security consecrated whatsapp mimicry.
>
> While I think to know how to exploit that in C/ASM, I have no idea whether
> that stuff is exploitable by other means.
>
>   1) What's the state of javascript?
>
>      If that's not an issue, then we really can stop worrying about pre
>      Nehalem for reality sake. If it's an issue we need at have some
>      information for Pavel at least. :)

I'd like to hope that nothing in a webpage can pull off the store-buffer
attack.  An attacker needs to be able to control its own faults to
create assist conditions, and #PF is the only plausible option, but
shouldn't be under attacker control.

However, I expect a webpage (webassembly in particular) could pull off
the load/fill buffer attack in the HT case.  This is certainly the
biggest risk from my point of view.

>   2) What about BPF?
>
>      If untrusted BPF can be abused to exploit this, then we have to think
>      hard about howto handle that.
>
>      Assume the following:
>
>      User A -> BPF -> preemption -> kernel thread -> back to BPF
>
>      Because there is no kernel to user transition no buffer clear will
>      happen - neither in full nor in conditional mode.
>
>      So the BPF sampler can happily collect the leftovers of the kernel
>      thread.
>
>      Might be theoretical, but it needs to be investigated at least.

TBH, I my gut feeling is that userspace might have an easier time
pulling off these attack that some BPF code, but this certainly doesn't
rule out the possibility.

I hope this is helpful and not too full of errors.  I'd certainly
appreciate some second opinions.

Thanks,

~Andrew

Re: Attack Surface [Was [patch V2 05/10] MDS basics+ 5]

[Thomas Gleixner]

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

On Thu, 21 Feb 2019, speck for Andrew Cooper wrote:
> On 21/02/2019 10:36, speck for Thomas Gleixner wrote:

....

> The prevailing concern is that an attacker can read most of the vector
> register state of the previous task to be scheduled on the thread, which
> could be very valuable information if the previous task was using AES-NI.

That part I think I had halfways groked. I'm more concerned about what kind
of context the attack can come from.

> >   1) What's the state of javascript?
> >
> >      If that's not an issue, then we really can stop worrying about pre
> >      Nehalem for reality sake. If it's an issue we need at have some
> >      information for Pavel at least. :)
> 
> I'd like to hope that nothing in a webpage can pull off the store-buffer
> attack.  An attacker needs to be able to control its own faults to
> create assist conditions, and #PF is the only plausible option, but
> shouldn't be under attacker control.
> 
> However, I expect a webpage (webassembly in particular) could pull off
> the load/fill buffer attack in the HT case.  This is certainly the
> biggest risk from my point of view.

Ok. 

> >   2) What about BPF?
> >
> >      If untrusted BPF can be abused to exploit this, then we have to think
> >      hard about howto handle that.
> >
> >      Assume the following:
> >
> >      User A -> BPF -> preemption -> kernel thread -> back to BPF
> >
> >      Because there is no kernel to user transition no buffer clear will
> >      happen - neither in full nor in conditional mode.
> >
> >      So the BPF sampler can happily collect the leftovers of the kernel
> >      thread.
> >
> >      Might be theoretical, but it needs to be investigated at least.
> 
> TBH, I my gut feeling is that userspace might have an easier time
> pulling off these attack that some BPF code, but this certainly doesn't
> rule out the possibility.

My concern here is: HT off, syscalls clear buffers, so BPF might be a good
way to do sampling by carefully constructing a gadget. No idea whether it
works at all.

Thanks,

	tglx

[patch V2 06/10] MDS basics+ 6

[Thomas Gleixner]

Subject: [patch V2 06/10] x86/speculation/mds: Add mitigation control for MDS
From: Thomas Gleixner <tglx@linutronix.de>

Now that the mitigations are in place, add a command line parameter to
control the mitigation, a mitigation selector function and a SMT update
mechanism.

This is the minimal straight forward initial implementation which just
provides an always on/off mode. The command line parameter is:

  mds=[full|off|auto]

This is consistent with the existing mitigations for other speculative
hardware vulnerabilities.

The idle invocation is dynamically updated according to the SMT state of
the system similar to the dynamic update of the STIBP mitigation.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/admin-guide/kernel-parameters.txt |   27 ++++++++
 arch/x86/include/asm/processor.h                |    6 +
 arch/x86/kernel/cpu/bugs.c                      |   76 ++++++++++++++++++++++++
 3 files changed, 109 insertions(+)

--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2362,6 +2362,33 @@
 			Format: <first>,<last>
 			Specifies range of consoles to be captured by the MDA.
 
+	mds=		[X86,INTEL]
+			Control mitigation for the Micro-architectural Data
+			Sampling (MDS) vulnerability.
+
+			Certain CPUs are vulnerable to an exploit against CPU
+			internal buffers which can forward information to a
+			disclosure gadget under certain conditions.
+
+			In vulnerable processors, the speculatively
+			forwarded data can be used in a cache side channel
+			attack, to access data to which the attacker does
+			not have direct access.
+
+			This parameter controls the MDS mitigation. The the
+			options are:
+
+			full    - Unconditionally enable MDS mitigation
+			off     - Unconditionally disable MDS mitigation
+			auto    - Kernel detects whether the CPU model is
+				  vulnerable to MDS and picks the most
+				  appropriate mitigation. If the CPU is not
+				  vulnerable, "off" is selected. If the CPU
+				  is vulnerable "full" is selected.
+
+			Not specifying this option is equivalent to
+			mds=auto.
+
 	mem=nn[KMG]	[KNL,BOOT] Force usage of a specific amount of memory
 			Amount of memory to be used when the kernel is not able
 			to see the whole system memory or for test.
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -991,4 +991,10 @@ enum l1tf_mitigations {
 
 extern enum l1tf_mitigations l1tf_mitigation;
 
+enum mds_mitigations {
+	MDS_MITIGATION_OFF,
+	MDS_MITIGATION_AUTO,
+	MDS_MITIGATION_FULL,
+};
+
 #endif /* _ASM_X86_PROCESSOR_H */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -37,6 +37,7 @@
 static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
 static void __init l1tf_select_mitigation(void);
+static void __init mds_select_mitigation(void);
 
 /* The base value of the SPEC_CTRL MSR that always has to be preserved. */
 u64 x86_spec_ctrl_base;
@@ -105,6 +106,8 @@ void __init check_bugs(void)
 
 	l1tf_select_mitigation();
 
+	mds_select_mitigation();
+
 #ifdef CONFIG_X86_32
 	/*
 	 * Check whether we are able to run this kernel safely on SMP.
@@ -211,6 +214,59 @@ static void x86_amd_ssb_disable(void)
 }
 
 #undef pr_fmt
+#define pr_fmt(fmt)	"MDS: " fmt
+
+/* Default mitigation for L1TF-affected CPUs */
+static enum mds_mitigations mds_mitigation __ro_after_init = MDS_MITIGATION_AUTO;
+
+static const char * const mds_strings[] = {
+	[MDS_MITIGATION_OFF]	= "Vulnerable",
+	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers"
+};
+
+static void mds_select_mitigation(void)
+{
+	if (!boot_cpu_has_bug(X86_BUG_MDS)) {
+		mds_mitigation = MDS_MITIGATION_OFF;
+		return;
+	}
+
+	switch (mds_mitigation) {
+	case MDS_MITIGATION_OFF:
+		break;
+	case MDS_MITIGATION_AUTO:
+	case MDS_MITIGATION_FULL:
+		if (boot_cpu_has(X86_FEATURE_MD_CLEAR)) {
+			mds_mitigation = MDS_MITIGATION_FULL;
+			static_branch_enable(&mds_user_clear_always);
+		} else {
+			mds_mitigation = MDS_MITIGATION_OFF;
+		}
+		break;
+	}
+	pr_info("%s\n", mds_strings[mds_mitigation]);
+}
+
+static int __init mds_cmdline(char *str)
+{
+	if (!boot_cpu_has_bug(X86_BUG_MDS))
+		return 0;
+
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "off"))
+		mds_mitigation = MDS_MITIGATION_OFF;
+	else if (!strcmp(str, "auto"))
+		mds_mitigation = MDS_MITIGATION_AUTO;
+	else if (!strcmp(str, "full"))
+		mds_mitigation = MDS_MITIGATION_FULL;
+
+	return 0;
+}
+early_param("mds", mds_cmdline);
+
+#undef pr_fmt
 #define pr_fmt(fmt)     "Spectre V2 : " fmt
 
 static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
@@ -614,6 +670,15 @@ static void update_indir_branch_cond(voi
 		static_branch_disable(&switch_to_cond_stibp);
 }
 
+/* Update the static key controlling the MDS CPU buffer clear in idle */
+static void update_mds_branch_idle(void)
+{
+	if (sched_smt_active())
+		static_branch_enable(&mds_idle_clear);
+	else
+		static_branch_disable(&mds_idle_clear);
+}
+
 void arch_smt_update(void)
 {
 	/* Enhanced IBRS implies STIBP. No update required. */
@@ -635,6 +700,17 @@ void arch_smt_update(void)
 		break;
 	}
 
+	switch (mds_mitigation) {
+	case MDS_MITIGATION_OFF:
+		break;
+	case MDS_MITIGATION_FULL:
+		update_mds_branch_idle();
+		break;
+	/* Keep GCC happy */
+	case MDS_MITIGATION_AUTO:
+		break;
+	}
+
 	mutex_unlock(&spec_ctrl_mutex);
 }
 

[MODERATED] Re: [patch V2 06/10] MDS basics+ 6

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:07:59PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 06/10] x86/speculation/mds: Add mitigation control for MDS
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Now that the mitigations are in place, add a command line parameter to
> control the mitigation, a mitigation selector function and a SMT update
> mechanism.
> 
> This is the minimal straight forward initial implementation which just
> provides an always on/off mode. The command line parameter is:
> 
>   mds=[full|off|auto]
> 
> This is consistent with the existing mitigations for other speculative
> hardware vulnerabilities.
> 
> The idle invocation is dynamically updated according to the SMT state of
> the system similar to the dynamic update of the STIBP mitigation.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   27 ++++++++
>  arch/x86/include/asm/processor.h                |    6 +
>  arch/x86/kernel/cpu/bugs.c                      |   76 ++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[patch V2 07/10] MDS basics+ 7

[Thomas Gleixner]

Subject: [patch V2 07/10] x86/speculation/mds: Add sysfs reporting for MDS
From: Thomas Gleixner <tglx@linutronix.de>

Add the sysfs reporting file for MDS. It exposes the vulnerability and
mitigation state similar to the existing files for the other speculative
hardware vulnerabilities.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/ABI/testing/sysfs-devices-system-cpu |    1 +
 arch/x86/kernel/cpu/bugs.c                         |   20 ++++++++++++++++++++
 drivers/base/cpu.c                                 |    6 ++++--
 include/linux/cpu.h                                |    2 ++
 4 files changed, 27 insertions(+), 2 deletions(-)

--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -484,6 +484,7 @@ What:		/sys/devices/system/cpu/vulnerabi
 		/sys/devices/system/cpu/vulnerabilities/spectre_v2
 		/sys/devices/system/cpu/vulnerabilities/spec_store_bypass
 		/sys/devices/system/cpu/vulnerabilities/l1tf
+		/sys/devices/system/cpu/vulnerabilities/mds
 Date:		January 2018
 Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
 Description:	Information about CPU vulnerabilities
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1187,6 +1187,17 @@ static ssize_t l1tf_show_state(char *buf
 }
 #endif
 
+static ssize_t mds_show_state(char *buf)
+{
+	if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
+		return sprintf(buf, "%s; SMT Host state unknown\n",
+			       mds_strings[mds_mitigation]);
+	}
+
+	return sprintf(buf, "%s; SMT %s\n", mds_strings[mds_mitigation],
+		       sched_smt_active() ? "vulnerable" : "disabled");
+}
+
 static char *stibp_state(void)
 {
 	if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED)
@@ -1253,6 +1264,10 @@ static ssize_t cpu_show_common(struct de
 		if (boot_cpu_has(X86_FEATURE_L1TF_PTEINV))
 			return l1tf_show_state(buf);
 		break;
+
+	case X86_BUG_MDS:
+		return mds_show_state(buf);
+
 	default:
 		break;
 	}
@@ -1284,4 +1299,9 @@ ssize_t cpu_show_l1tf(struct device *dev
 {
 	return cpu_show_common(dev, attr, buf, X86_BUG_L1TF);
 }
+
+ssize_t cpu_show_mds(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cpu_show_common(dev, attr, buf, X86_BUG_MDS);
+}
 #endif
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -540,8 +540,8 @@ ssize_t __weak cpu_show_spec_store_bypas
 	return sprintf(buf, "Not affected\n");
 }
 
-ssize_t __weak cpu_show_l1tf(struct device *dev,
-			     struct device_attribute *attr, char *buf)
+ssize_t __weak cpu_show_mds(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	return sprintf(buf, "Not affected\n");
 }
@@ -551,6 +551,7 @@ static DEVICE_ATTR(spectre_v1, 0444, cpu
 static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
 static DEVICE_ATTR(spec_store_bypass, 0444, cpu_show_spec_store_bypass, NULL);
 static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL);
+static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL);
 
 static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	&dev_attr_meltdown.attr,
@@ -558,6 +559,7 @@ static struct attribute *cpu_root_vulner
 	&dev_attr_spectre_v2.attr,
 	&dev_attr_spec_store_bypass.attr,
 	&dev_attr_l1tf.attr,
+	&dev_attr_mds.attr,
 	NULL
 };
 
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -57,6 +57,8 @@ extern ssize_t cpu_show_spec_store_bypas
 					  struct device_attribute *attr, char *buf);
 extern ssize_t cpu_show_l1tf(struct device *dev,
 			     struct device_attribute *attr, char *buf);
+extern ssize_t cpu_show_mds(struct device *dev,
+			    struct device_attribute *attr, char *buf);
 
 extern __printf(4, 5)
 struct device *cpu_device_create(struct device *parent, void *drvdata,

[MODERATED] Re: [patch V2 07/10] MDS basics+ 7

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:08:00PM +0100, speck for Thomas Gleixner wrote:
> Subject: [patch V2 07/10] x86/speculation/mds: Add sysfs reporting for MDS
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add the sysfs reporting file for MDS. It exposes the vulnerability and
> mitigation state similar to the existing files for the other speculative
> hardware vulnerabilities.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  Documentation/ABI/testing/sysfs-devices-system-cpu |    1 +
>  arch/x86/kernel/cpu/bugs.c                         |   20 ++++++++++++++++++++
>  drivers/base/cpu.c                                 |    6 ++++--
>  include/linux/cpu.h                                |    2 ++
>  4 files changed, 27 insertions(+), 2 deletions(-)

...

> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -540,8 +540,8 @@ ssize_t __weak cpu_show_spec_store_bypas
>  	return sprintf(buf, "Not affected\n");
>  }
>  
> -ssize_t __weak cpu_show_l1tf(struct device *dev,
> -			     struct device_attribute *attr, char *buf)
> +ssize_t __weak cpu_show_mds(struct device *dev,
> +			    struct device_attribute *attr, char *buf)

Didn't you mean to copy cpu_show_l1tf() here and rename it instead of
only renaming it?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [patch V2 07/10] MDS basics+ 7

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Borislav Petkov wrote:

> On Wed, Feb 20, 2019 at 04:08:00PM +0100, speck for Thomas Gleixner wrote:
> > Subject: [patch V2 07/10] x86/speculation/mds: Add sysfs reporting for MDS
> > From: Thomas Gleixner <tglx@linutronix.de>
> > 
> > Add the sysfs reporting file for MDS. It exposes the vulnerability and
> > mitigation state similar to the existing files for the other speculative
> > hardware vulnerabilities.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  Documentation/ABI/testing/sysfs-devices-system-cpu |    1 +
> >  arch/x86/kernel/cpu/bugs.c                         |   20 ++++++++++++++++++++
> >  drivers/base/cpu.c                                 |    6 ++++--
> >  include/linux/cpu.h                                |    2 ++
> >  4 files changed, 27 insertions(+), 2 deletions(-)
> 
> ...
> 
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -540,8 +540,8 @@ ssize_t __weak cpu_show_spec_store_bypas
> >  	return sprintf(buf, "Not affected\n");
> >  }
> >  
> > -ssize_t __weak cpu_show_l1tf(struct device *dev,
> > -			     struct device_attribute *attr, char *buf)
> > +ssize_t __weak cpu_show_mds(struct device *dev,
> > +			    struct device_attribute *attr, char *buf)
> 
> Didn't you mean to copy cpu_show_l1tf() here and rename it instead of
> only renaming it?

Sigh. Yes. Copy and paste is hard

[patch V2 08/10] MDS basics+ 8

[Thomas Gleixner]

In virtualized environments it can happen that the host has the microcode
update which utilizes the VERW instruction to clear CPU buffers, but the
hypervisor is not yet updated to expose the X86_FEATURE_MD_CLEAR CPUID bit
to guests.

Introduce an internal mitigation mode 'HOPE' which enables the invocation
of the CPU buffer clearing even if X86_FEATURE_MD_CLEAR is not set. If the
system has no updated microcode this results in a pointless execution of
the VERW instruction wasting a few CPU cycles. If the microcode is updated,
but not exposed to a guest then the CPU buffers will be cleared.

Hope dies last....

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/x86/mds.rst        |    5 +++++
 arch/x86/include/asm/processor.h |    1 +
 arch/x86/kernel/cpu/bugs.c       |   14 ++++++++------
 3 files changed, 14 insertions(+), 6 deletions(-)

--- a/Documentation/x86/mds.rst
+++ b/Documentation/x86/mds.rst
@@ -65,6 +65,11 @@ The mitigation is invoked on kernel/user
 (idle) transitions. Depending on the mitigation mode and the system state
 the invocation can be enforced or conditional.
 
+As a special quirk to address virtualization scenarios where the host has
+the microcode updated, but the hypervisor does not (yet) expose the
+MD_CLEAR CPUID bit to guests, the kernel issues the VERW instruction in the
+hope that it might work. The state is reflected accordingly.
+
 According to current knowledge additional mitigations inside the kernel
 itself are not required because the necessary gadgets to expose the leaked
 data cannot be controlled in a way which allows exploitation from malicious
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -995,6 +995,7 @@ enum mds_mitigations {
 	MDS_MITIGATION_OFF,
 	MDS_MITIGATION_AUTO,
 	MDS_MITIGATION_FULL,
+	MDS_MITIGATION_HOPE,
 };
 
 #endif /* _ASM_X86_PROCESSOR_H */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -221,7 +221,8 @@ static enum mds_mitigations mds_mitigati
 
 static const char * const mds_strings[] = {
 	[MDS_MITIGATION_OFF]	= "Vulnerable",
-	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers"
+	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers",
+	[MDS_MITIGATION_HOPE]	= "Vulnerable: Clear CPU buffers attempted, no microcode",
 };
 
 static void mds_select_mitigation(void)
@@ -236,12 +237,12 @@ static void mds_select_mitigation(void)
 		break;
 	case MDS_MITIGATION_AUTO:
 	case MDS_MITIGATION_FULL:
-		if (boot_cpu_has(X86_FEATURE_MD_CLEAR)) {
+	case MDS_MITIGATION_HOPE:
+		if (boot_cpu_has(X86_FEATURE_MD_CLEAR))
 			mds_mitigation = MDS_MITIGATION_FULL;
-			static_branch_enable(&mds_user_clear_always);
-		} else {
-			mds_mitigation = MDS_MITIGATION_OFF;
-		}
+		else
+			mds_mitigation = MDS_MITIGATION_HOPE;
+		static_branch_enable(&mds_user_clear_always);
 		break;
 	}
 	pr_info("%s\n", mds_strings[mds_mitigation]);
@@ -704,6 +705,7 @@ void arch_smt_update(void)
 	case MDS_MITIGATION_OFF:
 		break;
 	case MDS_MITIGATION_FULL:
+	case MDS_MITIGATION_HOPE:
 		update_mds_branch_idle();
 		break;
 	/* Keep GCC happy */

[MODERATED] Re: [patch V2 08/10] MDS basics+ 8

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:08:01PM +0100, speck for Thomas Gleixner wrote:
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -221,7 +221,8 @@ static enum mds_mitigations mds_mitigati
>  
>  static const char * const mds_strings[] = {
>  	[MDS_MITIGATION_OFF]	= "Vulnerable",
> -	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers"
> +	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers",
> +	[MDS_MITIGATION_HOPE]	= "Vulnerable: Clear CPU buffers attempted, no microcode",
>  };
>  
>  static void mds_select_mitigation(void)
> @@ -236,12 +237,12 @@ static void mds_select_mitigation(void)
>  		break;
>  	case MDS_MITIGATION_AUTO:
>  	case MDS_MITIGATION_FULL:
> -		if (boot_cpu_has(X86_FEATURE_MD_CLEAR)) {
> +	case MDS_MITIGATION_HOPE:

Now we have:

        switch (mds_mitigation) {
        case MDS_MITIGATION_OFF:
                break;
        case MDS_MITIGATION_AUTO:
        case MDS_MITIGATION_FULL:
        case MDS_MITIGATION_HOPE:

I guess that switch-case statement becomes not really useful and we can
do an early exit with a simple if:

	if (mds_mitigation == MDS_MITIGATION_OFF)
		goto print;

	/* do rest here */

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [patch V2 08/10] MDS basics+ 8

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Borislav Petkov wrote:
> On Wed, Feb 20, 2019 at 04:08:01PM +0100, speck for Thomas Gleixner wrote:
> Now we have:
> 
>         switch (mds_mitigation) {
>         case MDS_MITIGATION_OFF:
>                 break;
>         case MDS_MITIGATION_AUTO:
>         case MDS_MITIGATION_FULL:
>         case MDS_MITIGATION_HOPE:
> 
> I guess that switch-case statement becomes not really useful and we can
> do an early exit with a simple if:
> 
> 	if (mds_mitigation == MDS_MITIGATION_OFF)
> 		goto print;
> 
> 	/* do rest here */

Well, no. We are getting more of those and I like the switch() because it
complains if you add a new enum value and don't update the case.

Thanks,

	tglx

[patch V2 09/10] MDS basics+ 9

[Thomas Gleixner]

To avoid the expensive CPU buffer flushing on every transition from kernel
to user space it is desired to provide a conditional mitigation mode.

Provide the infrastructure which is required to implement this:

 - A static key to enable conditional mode CPU buffer flushing.

 - A per CPU variable which indicates that CPU buffers need to
   be flushed on return to user space. The variable is defined
   next to __preempt_count to it ends up in a cacheline which
   is required on return to user space anyway.

 - The conditinal flush mechanics on return to user space.

 - A helper function to set the flush request. Is in processor.h for now to
   avoid include hell, but might move to a separate header.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/entry/common.c              |    6 ++++++
 arch/x86/include/asm/nospec-branch.h |    3 +++
 arch/x86/include/asm/processor.h     |   13 +++++++++++++
 arch/x86/kernel/cpu/bugs.c           |    1 +
 arch/x86/kernel/cpu/common.c         |    7 +++++++
 5 files changed, 30 insertions(+)

--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -183,6 +183,12 @@ static void exit_to_usermode_loop(struct
 
 static inline void mds_user_clear_cpu_buffers(void)
 {
+	if (static_branch_likely(&mds_user_clear_cond)) {
+		if (__this_cpu_read(mds_cond_clear)) {
+			__this_cpu_write(mds_cond_clear, 0);
+			mds_clear_cpu_buffers();
+		}
+	}
 	if (static_branch_likely(&mds_user_clear_always))
 		mds_clear_cpu_buffers();
 }
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -9,6 +9,7 @@
 #include <asm/alternative-asm.h>
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
+#include <asm/percpu.h>
 
 /*
  * Fill the CPU return stack buffer.
@@ -319,7 +320,9 @@ DECLARE_STATIC_KEY_FALSE(switch_mm_cond_
 DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
 
 DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
+DECLARE_STATIC_KEY_FALSE(mds_user_clear_cond);
 DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
+DECLARE_PER_CPU(unsigned int, mds_cond_clear);
 
 #include <asm/segment.h>
 
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -24,6 +24,7 @@ struct vm86;
 #include <asm/special_insns.h>
 #include <asm/fpu/types.h>
 #include <asm/unwind_hints.h>
+#include <asm/nospec-branch.h>
 
 #include <linux/personality.h>
 #include <linux/cache.h>
@@ -998,4 +999,16 @@ enum mds_mitigations {
 	MDS_MITIGATION_HOPE,
 };
 
+/**
+ * mds_request_buffer_clear - Set the request to clear CPU buffers
+ *
+ * This is invoked from contexts which identify a necessarity to clear CPU
+ * buffers on the next return to user space.
+ */
+static inline void mds_request_buffer_clear(void)
+{
+	if (static_branch_likely(&mds_user_clear_cond))
+		this_cpu_write(mds_cond_clear, 1);
+}
+
 #endif /* _ASM_X86_PROCESSOR_H */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -66,6 +66,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_always
 
 /* Control MDS CPU buffer clear before returning to user space */
 DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
+DEFINE_STATIC_KEY_FALSE(mds_user_clear_cond);
 /* Control MDS CPU buffer clear before idling (halt, mwait) */
 DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
 
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -8,6 +8,7 @@
 #include <linux/export.h>
 #include <linux/percpu.h>
 #include <linux/string.h>
+#include <linux/nospec.h>
 #include <linux/ctype.h>
 #include <linux/delay.h>
 #include <linux/sched/mm.h>
@@ -1544,6 +1545,9 @@ DEFINE_PER_CPU(unsigned int, irq_count)
 DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
 EXPORT_PER_CPU_SYMBOL(__preempt_count);
 
+/* Indicator for return to user space or VMENTER to clear CPU buffers */
+DEFINE_PER_CPU(unsigned int, mds_cond_clear);
+
 /* May not be marked __init: used by software suspend */
 void syscall_init(void)
 {
@@ -1617,6 +1621,9 @@ EXPORT_PER_CPU_SYMBOL(current_task);
 DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
 EXPORT_PER_CPU_SYMBOL(__preempt_count);
 
+/* Indicator for return to user space or VMENTER to clear CPU buffers */
+DEFINE_PER_CPU(unsigned int, mds_cond_clear);
+
 /*
  * On x86_32, vm86 modifies tss.sp0, so sp0 isn't a reliable way to find
  * the top of the kernel stack.  Use an extra percpu variable to track the

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Peter Zijlstra]

On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
>  static inline void mds_user_clear_cpu_buffers(void)
>  {
> +	if (static_branch_likely(&mds_user_clear_cond)) {

Are we sure we want _likely() ? That puts the body in-line.

> +		if (__this_cpu_read(mds_cond_clear)) {
> +			__this_cpu_write(mds_cond_clear, 0);
> +			mds_clear_cpu_buffers();
> +		}
		return;

_might_ generate better code; by making it explitit we'll never have
both branches enabled at the same time.

> +	}
>  	if (static_branch_likely(&mds_user_clear_always))
>  		mds_clear_cpu_buffers();
>  }

And in general; yuck! Should I once again look at tri-state jump_labels
or static_switch? Last time I tried I got stuck on the C part, writing
different targets is trivial.

Ideally we'd end up with something like:

1:	jmp 3f
	test $1, %gs:mds_cond_clear
	jz 3f
	mov $0, %gs:mds_cond_clear
2:	verw ds
3:

And change the jmp at 1 between: JMP 3f, JMP 2f and NOP for
mds={off,full,cond} resp.

Re: [patch V2 09/10] MDS basics+ 9

[Thomas Gleixner]

On Wed, 20 Feb 2019, speck for Peter Zijlstra wrote:
> On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
> >  static inline void mds_user_clear_cpu_buffers(void)
> >  {
> > +	if (static_branch_likely(&mds_user_clear_cond)) {
> 
> Are we sure we want _likely() ? That puts the body in-line.

Yes, no, dunno. 

> > +		if (__this_cpu_read(mds_cond_clear)) {
> > +			__this_cpu_write(mds_cond_clear, 0);
> > +			mds_clear_cpu_buffers();
> > +		}
> 		return;
> 
> _might_ generate better code; by making it explitit we'll never have
> both branches enabled at the same time.
> 
> > +	}
> >  	if (static_branch_likely(&mds_user_clear_always))
> >  		mds_clear_cpu_buffers();
> >  }
> 
> And in general; yuck! Should I once again look at tri-state jump_labels
> or static_switch? Last time I tried I got stuck on the C part, writing
> different targets is trivial.
> 
> Ideally we'd end up with something like:
> 
> 1:	jmp 3f
> 	test $1, %gs:mds_cond_clear
> 	jz 3f
> 	mov $0, %gs:mds_cond_clear
> 2:	verw ds
> 3:
> 
> And change the jmp at 1 between: JMP 3f, JMP 2f and NOP for
> mds={off,full,cond} resp.

Right. That'd be cute, but for backporting sake we should stick with the
tools we have now.

Thanks,

	tglx

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Jiri Kosina]

On Wed, 20 Feb 2019, speck for Thomas Gleixner wrote:

> > And change the jmp at 1 between: JMP 3f, JMP 2f and NOP for
> > mds={off,full,cond} resp.
> 
> Right. That'd be cute, but for backporting sake we should stick with the
> tools we have now.

Thomas, thanks a lot for taking this into consideration. It'd indeed be an 
annoyingly big issue for us.

-- 
Jiri Kosina
SUSE Labs

Re: [patch V2 09/10] MDS basics+ 9

[Thomas Gleixner]

Jiri,

On Wed, 20 Feb 2019, speck for Jiri Kosina wrote:

> On Wed, 20 Feb 2019, speck for Thomas Gleixner wrote:
> 
> > > And change the jmp at 1 between: JMP 3f, JMP 2f and NOP for
> > > mds={off,full,cond} resp.
> > 
> > Right. That'd be cute, but for backporting sake we should stick with the
> > tools we have now.
> 
> Thomas, thanks a lot for taking this into consideration. It'd indeed be an 
> annoyingly big issue for us.

Usually I do not care much about the kernel necrophilia cult, but indeed
all this is annoying enough as is, so there is no need that we annoy each
other even more :)

Thanks,

	tglx

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Peter Zijlstra]

On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
>  static inline void mds_user_clear_cpu_buffers(void)
>  {
> +	if (static_branch_likely(&mds_user_clear_cond)) {
> +		if (__this_cpu_read(mds_cond_clear)) {
> +			__this_cpu_write(mds_cond_clear, 0);
> +			mds_clear_cpu_buffers();
> +		}
> +	}
>  	if (static_branch_likely(&mds_user_clear_always))
>  		mds_clear_cpu_buffers();
>  }

Results in:

998:   e9 0b 00 00 00          jmpq   9a8 <prepare_exit_to_usermode+0x58>
99d:   65 8b 05 00 00 00 00    mov    %gs:0x0(%rip),%eax        # 9a4 <prepare_exit_to_usermode+0x54>
9a0: R_X86_64_PC32      mds_cond_clear-0x4
9a4:   85 c0                   test   %eax,%eax
9a6:   75 0f                   jne    9b7 <prepare_exit_to_usermode+0x67>
9a8:   e9 07 00 00 00          jmpq   9b4 <prepare_exit_to_usermode+0x64>
9ad:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9b4 <prepare_exit_to_usermode+0x64>
9b0: R_X86_64_PC32      .rodata-0x4
9b4:   5b                      pop    %rbx
9b5:   5d                      pop    %rbp
9b6:   c3                      retq
9b7:   65 c7 05 00 00 00 00    movl   $0x0,%gs:0x0(%rip)        # 9c2 <prepare_exit_to_usermode+0x72>
9be:   00 00 00 00
9ba: R_X86_64_PC32      mds_cond_clear-0x8
9c2:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9c9 <prepare_exit_to_usermode+0x79>
9c5: R_X86_64_PC32      .rodata-0x4
9c9:   eb dd                   jmp    9a8 <prepare_exit_to_usermode+0x58>


static inline void mds_user_clear_cpu_buffers(void)
{
       if (!static_branch_likely(&mds_user_clear_always))
               return;

       if (static_branch_likely(&mds_user_clear_cond)) {
               if (!__this_cpu_read(mds_cond_clear))
                       return;

               __this_cpu_write(mds_cond_clear, 0);
       }

       mds_clear_cpu_buffers();
}

results in:

998:   e9 22 00 00 00          jmpq   9bf <prepare_exit_to_usermode+0x6f>
99d:   e9 16 00 00 00          jmpq   9b8 <prepare_exit_to_usermode+0x68>
9a2:   65 8b 05 00 00 00 00    mov    %gs:0x0(%rip),%eax        # 9a9 <prepare_exit_to_usermode+0x59>
9a5: R_X86_64_PC32      mds_cond_clear-0x4
9a9:   85 c0                   test   %eax,%eax
9ab:   74 12                   je     9bf <prepare_exit_to_usermode+0x6f>
9ad:   65 c7 05 00 00 00 00    movl   $0x0,%gs:0x0(%rip)        # 9b8 <prepare_exit_to_usermode+0x68>
9b4:   00 00 00 00
9b0: R_X86_64_PC32      mds_cond_clear-0x8
9b8:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9bf <prepare_exit_to_usermode+0x6f>
9bb: R_X86_64_PC32      .rodata-0x4
9bf:


The only 'downside' is that means @mds_user_clear_always is now a
misnomer (and needs be enabled for cond too).

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Peter Zijlstra]

On Thu, Feb 21, 2019 at 12:04:44PM +0100, Peter Zijlstra wrote:
> On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
> >  static inline void mds_user_clear_cpu_buffers(void)
> >  {
> > +	if (static_branch_likely(&mds_user_clear_cond)) {
> > +		if (__this_cpu_read(mds_cond_clear)) {
> > +			__this_cpu_write(mds_cond_clear, 0);
> > +			mds_clear_cpu_buffers();
> > +		}
> > +	}
> >  	if (static_branch_likely(&mds_user_clear_always))
> >  		mds_clear_cpu_buffers();
> >  }
> 
> Results in:
> 
> 998:   e9 0b 00 00 00          jmpq   9a8 <prepare_exit_to_usermode+0x58>
> 99d:   65 8b 05 00 00 00 00    mov    %gs:0x0(%rip),%eax        # 9a4 <prepare_exit_to_usermode+0x54>
> 9a0: R_X86_64_PC32      mds_cond_clear-0x4
> 9a4:   85 c0                   test   %eax,%eax
> 9a6:   75 0f                   jne    9b7 <prepare_exit_to_usermode+0x67>
> 9a8:   e9 07 00 00 00          jmpq   9b4 <prepare_exit_to_usermode+0x64>
> 9ad:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9b4 <prepare_exit_to_usermode+0x64>
> 9b0: R_X86_64_PC32      .rodata-0x4
> 9b4:   5b                      pop    %rbx
> 9b5:   5d                      pop    %rbp
> 9b6:   c3                      retq
> 9b7:   65 c7 05 00 00 00 00    movl   $0x0,%gs:0x0(%rip)        # 9c2 <prepare_exit_to_usermode+0x72>
> 9be:   00 00 00 00
> 9ba: R_X86_64_PC32      mds_cond_clear-0x8
> 9c2:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9c9 <prepare_exit_to_usermode+0x79>
> 9c5: R_X86_64_PC32      .rodata-0x4
> 9c9:   eb dd                   jmp    9a8 <prepare_exit_to_usermode+0x58>
> 
> 
> static inline void mds_user_clear_cpu_buffers(void)
> {
>        if (!static_branch_likely(&mds_user_clear_always))
>                return;
> 
>        if (static_branch_likely(&mds_user_clear_cond)) {
>                if (!__this_cpu_read(mds_cond_clear))
>                        return;
> 
>                __this_cpu_write(mds_cond_clear, 0);
>        }
> 
>        mds_clear_cpu_buffers();
> }
> 
> results in:
> 
> 998:   e9 22 00 00 00          jmpq   9bf <prepare_exit_to_usermode+0x6f>
> 99d:   e9 16 00 00 00          jmpq   9b8 <prepare_exit_to_usermode+0x68>
> 9a2:   65 8b 05 00 00 00 00    mov    %gs:0x0(%rip),%eax        # 9a9 <prepare_exit_to_usermode+0x59>
> 9a5: R_X86_64_PC32      mds_cond_clear-0x4
> 9a9:   85 c0                   test   %eax,%eax
> 9ab:   74 12                   je     9bf <prepare_exit_to_usermode+0x6f>
> 9ad:   65 c7 05 00 00 00 00    movl   $0x0,%gs:0x0(%rip)        # 9b8 <prepare_exit_to_usermode+0x68>
> 9b4:   00 00 00 00
> 9b0: R_X86_64_PC32      mds_cond_clear-0x8
> 9b8:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9bf <prepare_exit_to_usermode+0x6f>
> 9bb: R_X86_64_PC32      .rodata-0x4
> 9bf:
> 
> 
> The only 'downside' is that means @mds_user_clear_always is now a
> misnomer (and needs be enabled for cond too).

static inline void mds_user_clear_cpu_buffers(void)
{
	if (!static_branch_likely(&mds_user_clear_always))
		return;

	if (static_branch_likely(&mds_user_clear_cond)) {
		if (!GEN_BINARY_RMWcc("btrl", mds_cond_clear, c, "Ir", 0, __percpu_arg([var])))
			return;
	}

	mds_clear_cpu_buffers();
}

998:   e9 17 00 00 00          jmpq   9b4 <prepare_exit_to_usermode+0x64>
99d:   e9 0b 00 00 00          jmpq   9ad <prepare_exit_to_usermode+0x5d>
9a2:   65 0f ba 35 00 00 00    btrl   $0x0,%gs:0x0(%rip)        # 9ab <prepare_exit_to_usermode+0x5b>
9a9:   00 00
9a6: R_X86_64_PC32      mds_cond_clear-0x5
9ab:   73 07                   jae    9b4 <prepare_exit_to_usermode+0x64>
9ad:   0f 00 2d 00 00 00 00    verw   0x0(%rip)        # 9b4 <prepare_exit_to_usermode+0x64>
9b0: R_X86_64_PC32      .rodata-0x4
9b4:

But then the nectro-cult people will hate you :-) Also it will fail to
compile for old GCC and Steve's insane if() redefinition.

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Borislav Petkov]

On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
> To avoid the expensive CPU buffer flushing on every transition from kernel
> to user space it is desired to provide a conditional mitigation mode.
> 
> Provide the infrastructure which is required to implement this:
> 
>  - A static key to enable conditional mode CPU buffer flushing.
> 
>  - A per CPU variable which indicates that CPU buffers need to
>    be flushed on return to user space. The variable is defined
>    next to __preempt_count to it ends up in a cacheline which

			    so

>    is required on return to user space anyway.
> 
>  - The conditinal flush mechanics on return to user space.

	conditional

> 
>  - A helper function to set the flush request. Is in processor.h for now to
>    avoid include hell, but might move to a separate header.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/entry/common.c              |    6 ++++++
>  arch/x86/include/asm/nospec-branch.h |    3 +++
>  arch/x86/include/asm/processor.h     |   13 +++++++++++++
>  arch/x86/kernel/cpu/bugs.c           |    1 +
>  arch/x86/kernel/cpu/common.c         |    7 +++++++
>  5 files changed, 30 insertions(+)
> 
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -183,6 +183,12 @@ static void exit_to_usermode_loop(struct
>  
>  static inline void mds_user_clear_cpu_buffers(void)
>  {
> +	if (static_branch_likely(&mds_user_clear_cond)) {
> +		if (__this_cpu_read(mds_cond_clear)) {
> +			__this_cpu_write(mds_cond_clear, 0);
> +			mds_clear_cpu_buffers();
> +		}
> +	}
>  	if (static_branch_likely(&mds_user_clear_always))
>  		mds_clear_cpu_buffers();
>  }
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -9,6 +9,7 @@
>  #include <asm/alternative-asm.h>
>  #include <asm/cpufeatures.h>
>  #include <asm/msr-index.h>
> +#include <asm/percpu.h>
>  
>  /*
>   * Fill the CPU return stack buffer.
> @@ -319,7 +320,9 @@ DECLARE_STATIC_KEY_FALSE(switch_mm_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
>  DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
> +DECLARE_STATIC_KEY_FALSE(mds_user_clear_cond);
>  DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
> +DECLARE_PER_CPU(unsigned int, mds_cond_clear);
>  
>  #include <asm/segment.h>
>  
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -24,6 +24,7 @@ struct vm86;
>  #include <asm/special_insns.h>
>  #include <asm/fpu/types.h>
>  #include <asm/unwind_hints.h>
> +#include <asm/nospec-branch.h>
>  
>  #include <linux/personality.h>
>  #include <linux/cache.h>
> @@ -998,4 +999,16 @@ enum mds_mitigations {
>  	MDS_MITIGATION_HOPE,
>  };
>  
> +/**
> + * mds_request_buffer_clear - Set the request to clear CPU buffers
> + *
> + * This is invoked from contexts which identify a necessarity to clear CPU
							^
							necessity

> + * buffers on the next return to user space.
> + */
> +static inline void mds_request_buffer_clear(void)
> +{
> +	if (static_branch_likely(&mds_user_clear_cond))
> +		this_cpu_write(mds_cond_clear, 1);
> +}
> +
>  #endif /* _ASM_X86_PROCESSOR_H */
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -66,6 +66,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_always
>  
>  /* Control MDS CPU buffer clear before returning to user space */
>  DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
> +DEFINE_STATIC_KEY_FALSE(mds_user_clear_cond);
>  /* Control MDS CPU buffer clear before idling (halt, mwait) */
>  DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
>  
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -8,6 +8,7 @@
>  #include <linux/export.h>
>  #include <linux/percpu.h>
>  #include <linux/string.h>
> +#include <linux/nospec.h>
>  #include <linux/ctype.h>
>  #include <linux/delay.h>
>  #include <linux/sched/mm.h>
> @@ -1544,6 +1545,9 @@ DEFINE_PER_CPU(unsigned int, irq_count)
>  DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
>  EXPORT_PER_CPU_SYMBOL(__preempt_count);
>  
> +/* Indicator for return to user space or VMENTER to clear CPU buffers */
> +DEFINE_PER_CPU(unsigned int, mds_cond_clear);
> +
>  /* May not be marked __init: used by software suspend */
>  void syscall_init(void)
>  {
> @@ -1617,6 +1621,9 @@ EXPORT_PER_CPU_SYMBOL(current_task);
>  DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
>  EXPORT_PER_CPU_SYMBOL(__preempt_count);
>  
> +/* Indicator for return to user space or VMENTER to clear CPU buffers */
> +DEFINE_PER_CPU(unsigned int, mds_cond_clear);
> +

Why not define it once in the bitness-agnostic place over #ifdef CONFIG_X86_64?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Kees Cook]

On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
>  - A helper function to set the flush request. Is in processor.h for now to
>    avoid include hell, but might move to a separate header.

So, this looks like a blacklisting approach? i.e. things that feel they are
sensitive must call this to make sure they don't leak.

I think we'd have safer coverage if we did this in reverse: we're likely
better able to reason about places where we know there's nothing
interesting happening and we don't need to flush. (i.e. whitelist and
flush by default)

Now, all that said, I think we need to always flush, but I'm paranoid and
I look at exploits too much. For example, even stack addresses themselves
should be considered secret, since they may be used by attackers to align
cross-stack attacks, etc. Take a look at the hoops that are needed to
pull this attack off: https://www.slideshare.net/scovetta/stackjacking
I don't think we should make this easier by default.

The same applies to all heap addresses, and basically everything. Just
blacklisting externally-defined "secrets" isn't going to protect much,
IMO. Discoverability of kernel memory layout (and I'm not talking text
ASLR here: I mean stack, heap, page table location, etc) is basically
the second step of modern attacks. (The first step is usually turning
off SMAP.)

So, for the paranoid: we need a flush-always. For people who think
attackers aren't going to use 0-day bugs and all the leaky info to perform
a "regular" memory corruption attack to gain access to "secrets", and want
to just stop direct leaks, I think whitelisting is the better option. How
can we know what someone thinks is a secret?

-- 
Kees Cook                                            @outflux.net

Re: [patch V2 09/10] MDS basics+ 9

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Kees Cook wrote:

> On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
> >  - A helper function to set the flush request. Is in processor.h for now to
> >    avoid include hell, but might move to a separate header.
> 
> So, this looks like a blacklisting approach? i.e. things that feel they are
> sensitive must call this to make sure they don't leak.
> 
> I think we'd have safer coverage if we did this in reverse: we're likely
> better able to reason about places where we know there's nothing
> interesting happening and we don't need to flush. (i.e. whitelist and
> flush by default)
> 
> Now, all that said, I think we need to always flush, but I'm paranoid and
> I look at exploits too much. For example, even stack addresses themselves
> should be considered secret, since they may be used by attackers to align
> cross-stack attacks, etc. Take a look at the hoops that are needed to
> pull this attack off: https://www.slideshare.net/scovetta/stackjacking
> I don't think we should make this easier by default.
> 
> The same applies to all heap addresses, and basically everything. Just
> blacklisting externally-defined "secrets" isn't going to protect much,
> IMO. Discoverability of kernel memory layout (and I'm not talking text
> ASLR here: I mean stack, heap, page table location, etc) is basically
> the second step of modern attacks. (The first step is usually turning
> off SMAP.)
> 
> So, for the paranoid: we need a flush-always. For people who think
> attackers aren't going to use 0-day bugs and all the leaky info to perform
> a "regular" memory corruption attack to gain access to "secrets", and want
> to just stop direct leaks, I think whitelisting is the better option. How
> can we know what someone thinks is a secret?

We have a flush always mode. That's there already and was done first.

While I understand the request to have more light weight mitigations, I'm
not convinced about the conditional thing either in the light of these
issues.

The problem with both white- and black-listing is that we might end up with
a large number of places which do the flagging. If it's just a few like
context switch, that'd be ok, but I fear that everyone and his dog will
come up with some other picture what needs to be protected.

I'll post the last iteration of the lot soonish and we clearly want to keep
the last two out of the git tree for now until we come to a conclusion on
that matter. Andi should be able to provide some insight from analysing
stuff.

TBH, I personaly would love to just to stick with [full|off], send the
conditional stuff to hell, go back and do useful and more pleasant work.

Alternatively we finally bite the bullet and set NR_CPUS=0. All problems
solved.

Thanks,

	tglx

Re: [patch V2 09/10] MDS basics+ 9

[Thomas Gleixner]

On Thu, 21 Feb 2019, speck for Thomas Gleixner wrote:
> On Thu, 21 Feb 2019, speck for Kees Cook wrote:
> I'll post the last iteration of the lot soonish and we clearly want to keep
> the last two out of the git tree for now until we come to a conclusion on
> that matter. Andi should be able to provide some insight from analysing
> stuff.

That said it would be also extremly useful to get the view on this from the
real world service providers with large deployments. What's their take on
this?

All this thought experiments, micro benchmark numbers and marketing
oriented wishful thinking might become moot when confronted with reality.

Thanks,

	tglx

[patch V2 10/10] MDS basics+ 10

[Thomas Gleixner]

Utilize the already existing information in switch_mm_irqs_off() to request
CPU buffer clearing on return to user space for conditional MDS mitigation:

  - Switching between two processes

  - Switching back and forth between process and kernel thread. This
    utilizes the lazy mm mechanism, which already provides all required
    conditionals.

In both cases the flush is requested to prevent potential leakage of data
either from the previous process or the kernel thread.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/mm/tlb.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -351,6 +351,16 @@ void switch_mm_irqs_off(struct mm_struct
 			return;
 
 		/*
+		 * The context switch sequence was: USER -> KERNEL -> USER.
+		 *
+		 * For CPUs which are affected by MDS this is a condition
+		 * to enforce flushing of CPU buffers before returning
+		 * to user space to prevent potential leakage of data which
+		 * was touched by the kernel thread.
+		 */
+		mds_request_buffer_clear();
+
+		/*
 		 * Read the tlb_gen to check whether a flush is needed.
 		 * If the TLB is up to date, just use it.
 		 * The barrier synchronizes with the tlb_gen increment in
@@ -376,6 +386,14 @@ void switch_mm_irqs_off(struct mm_struct
 		 */
 		cond_ibpb(tsk);
 
+		/*
+		 * Switching to a different process triggers flushing of
+		 * CPU buffers before returning to user space to prevent
+		 * potential leakage of data which was touched by the
+		 * previous process or by a kernel thread.
+		 */
+		mds_request_buffer_clear();
+
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
 			/*
 			 * If our current stack is in vmalloc space and isn't

[MODERATED] Re: [patch V2 00/10] MDS basics+ 0

[mark gross]

On Wed, Feb 20, 2019 at 04:07:53PM +0100, speck for Thomas Gleixner wrote:
> Hi!
> 
> This is an update to yesterdays series with the following changes:
> 
>    - Addressed review comments (on/off list)
> 
>    - Changed the approach with static keys slightly
> 
>    - Added "cc" clobber to the VERW asm magic (spotted by Peterz)
> 
>    - Added x86 specific documentation which explains the mitigation methods
>      and details on why particular code pathes are excluded.
> 
>    - Added an internal 'HOPE' mitigation mode to address the VMWare wish.
> 
>    - Added the basic infrastructure for conditional mode
> 
> Dropped the documentation patch for now as I'm not done with updating it
> and I have to run now and attend my grandson's birthday party.
> 
> Thanks,
> 
> 	tglx
> 
> 
>
bumped into a link time issue with an allmodconfig compile.  I see a new
version is out.  I'll check it out shortly but, FWIW:


From c8f9c06339e76b03b5c9e50c3075476ce6e79823 Mon Sep 17 00:00:00 2001
From: Mark Gross <mgross@linux.intel.com>
Date: Fri, 22 Feb 2019 07:59:22 -0800
Subject: [PATCH] minor fix up to get the mds mitigation to compile with
 allmodconfig

added EXPORT_SYMBOL_GPL(mds_idle_clear); to bugs.c

Signed-off-by: Mark Gross <mgross@linux.intel.com>
---
 arch/x86/kernel/cpu/bugs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index a4da89834d4b..d862881721ce 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -69,6 +69,7 @@ DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
 DEFINE_STATIC_KEY_FALSE(mds_user_clear_cond);
 /* Control MDS CPU buffer clear before idling (halt, mwait) */
 DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
+EXPORT_SYMBOL_GPL(mds_idle_clear);
 
 void __init check_bugs(void)
 {
-- 
2.20.1

Re: [patch V2 00/10] MDS basics+ 0

[Thomas Gleixner]

On Fri, 22 Feb 2019, speck for mark gross wrote:
> bumped into a link time issue with an allmodconfig compile.  I see a new
> version is out.  I'll check it out shortly but, FWIW:

Indeed. That's still there because the drivers/ stuff can be built modular.

I'll fold that in.

Thanks!

	tglx