Skip to contentSkip to conversation

x86, mpx: Workaround for MPX erratum

PATCH New Dave Hansen 2 participants 4 messages May 4, 2016 – May 5, 2016 x86

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 x86, mpx: Workaround for MPX erratum

Changes from v1:
* Unconditionally enable workaround on all CPUs with MPX despite whether we know it to be affected or not
* Add a pr_warn() when the workaround is active

--

This erratum essentially causes the CPU to forget which privilege mode it is operating in (kernel vs. user) for the purposes of MPX.

This erratum can only be triggered when a system is not using Supervisor Mode Execution Prevention (SMEP). Our workaround for the erratum is to ensure that MPX can only be used in cases where SMEP is present in the processor and enabled.

This erratum only affects Core processors. Atom is unaffected. But, there is no architectural way to determine Atom vs. Core. So, we just apply this workaround to all processors. It's possible that it will mistakenly disable MPX on some Atom processsors or future unaffected Core processors. There are currently no processors that have MPX and not SMEP. It would take something akin to a hypervisor masking SMEP out on an Atom processor for this to present itself on current hardware.

More details:

http://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/desktop-6th-gen-core-family-spec-update.pdf

SKD046 Branch Instructions May Initialize MPX Bound Registers Incorrectly

Problem:

Depending on the current Intel MPX (Memory Protection Extensions) configuration, execution of certain branch instructions (near CALL, near RET, near JMP, and Jcc instructions) without a BND prefix (F2H) initialize the MPX bound registers. Due to this erratum, such a branch instruction that is executed both with CPL = 3 and with CPL < 3 may not use the correct MPX configuration register (BNDCFGU or BNDCFGS, respectively) for determining whether to initialize the bound registers; it may thus initialize the bound registers when it should not, or fail to initialize them when it should.

Implication:

A branch instruction that has executed both in user mode and in supervisor mode (from the same linear address) may cause a #BR (bound range fault) when it should not have or may not cause a #BR when it should have. Workaround An operating system can avoid this erratum by setting CR4.SMEP[bit 20] to enable supervisor-mode execution prevention (SMEP). When SMEP is enabled, no code can be executed both with CPL = 3 and with CPL < 3.

---

 b/arch/x86/include/asm/bugs.h  |    9 +++++++--
 b/arch/x86/kernel/cpu/common.c |    3 +++
 b/arch/x86/kernel/cpu/intel.c  |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)
arch/x86/include/asm/bugs.h renamed from arch/x86/include/asm/bugs.h~neversend-skl-mpx-errata-simple +7 -2
diff -puN arch/x86/include/asm/bugs.h~neversend-skl-mpx-errata-simple arch/x86/include/asm/bugs.h
@@ -3,10 +3,15 @@
 
 extern void check_bugs(void);
 
-#if defined(CONFIG_CPU_SUP_INTEL) && defined(CONFIG_X86_32)
+#if defined(CONFIG_CPU_SUP_INTEL)
+void check_mpx_erratum(struct cpuinfo_x86 *c);
+#else
+static inline void check_mpx_erratum(struct cpuinfo_x86 *c) {}
+#if defined(CONFIG_X86_32)
 int ppro_with_ram_bug(void);
 #else
 static inline int ppro_with_ram_bug(void) { return 0; }
-#endif
+#endif /* CONFIG_X86_32 */
+#endif /* CONFIG_CPU_SUP_INTEL */
 
 #endif /* _ASM_X86_BUGS_H */
arch/x86/kernel/cpu/common.c renamed from arch/x86/kernel/cpu/common.c~neversend-skl-mpx-errata-simple +3 -0
diff -puN arch/x86/kernel/cpu/common.c~neversend-skl-mpx-errata-simple arch/x86/kernel/cpu/common.c
@@ -37,6 +37,7 @@
 #include <asm/mtrr.h>
 #include <linux/numa.h>
 #include <asm/asm.h>
+#include <asm/bugs.h>
 #include <asm/cpu.h>
 #include <asm/mce.h>
 #include <asm/msr.h>
@@ -270,6 +271,8 @@ static inline void squash_the_stupid_ser
 static __init int setup_disable_smep(char *arg)
 {
 	setup_clear_cpu_cap(X86_FEATURE_SMEP);
+	/* also check for things that depend on SMEP being enabled */
+	check_mpx_erratum(&boot_cpu_data);
 	return 1;
 }
 __setup("nosmep", setup_disable_smep);
arch/x86/kernel/cpu/intel.c renamed from arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple +37 -0
diff -puN arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple arch/x86/kernel/cpu/intel.c
@@ -25,6 +25,41 @@
 #include <asm/apic.h>
 #endif
 
+/*
+ * Just in case our CPU detection goes bad, allow a way to
+ * override the disabling of MPX.
+ */
+static int forcempx;
+static int __init forcempx_setup(char *__unused)
+{
+	forcempx = 1;
+	return 1;
+}
+__setup("intel-skd-046-workaround=disable", forcempx_setup);
+
+void check_mpx_erratum(struct cpuinfo_x86 *c)
+{
+	if (forcempx)
+	       return;
+	/*
+	 * Turn off MPX feature on CPUs where SMEP is not
+	 * available or disabled.
+	 *
+	 * Works around Intel Erratum: SKD046 Branch Instructions
+	 * May Initialize MPX Bound Registers Incorrectly.
+	 *
+	 * This might falsely-disable MPX on systems without
+	 * SMEP, like Atom processors without SMEP.  But, there
+	 * are not any known places where that happens with
+	 * actual hardware.
+	 */
+	if (cpu_has(c, X86_FEATURE_MPX) &&
+	    !cpu_has(c, X86_FEATURE_SMEP)) {
+		setup_clear_cpu_cap(X86_FEATURE_MPX);
+		pr_warn("x86/mpx: disabling since SMEP not present\n");
+	}
+}
+
 static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
@@ -173,6 +208,8 @@ static void early_init_intel(struct cpui
 		if (edx & (1U << 28))
 			c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff);
 	}
+
+	check_mpx_erratum(c);
 }
 
 #ifdef CONFIG_X86_32

_

#1 lore raw

PATCH v3 1/1 x86/mm/mpx: Work around MPX erratum SKD046

Quoted 112 lines · Dave Hansen

Changes from v1:
* Unconditionally enable workaround on all CPUs with MPX despite whether we know it to be affected or not
* Add a pr_warn() when the workaround is active

--

This erratum essentially causes the CPU to forget which privilege mode it is operating in (kernel vs. user) for the purposes of MPX.

This erratum can only be triggered when a system is not using Supervisor Mode Execution Prevention (SMEP). Our workaround for the erratum is to ensure that MPX can only be used in cases where SMEP is present in the processor and enabled.

This erratum only affects Core processors. Atom is unaffected. But, there is no architectural way to determine Atom vs. Core. So, we just apply this workaround to all processors. It's possible that it will mistakenly disable MPX on some Atom processsors or future unaffected Core processors. There are currently no processors that have MPX and not SMEP. It would take something akin to a hypervisor masking SMEP out on an Atom processor for this to present itself on current hardware.

More details:

http://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/desktop-6th-gen-core-family-spec-update.pdf

SKD046 Branch Instructions May Initialize MPX Bound Registers Incorrectly

Problem:

Depending on the current Intel MPX (Memory Protection Extensions) configuration, execution of certain branch instructions (near CALL, near RET, near JMP, and Jcc instructions) without a BND prefix (F2H) initialize the MPX bound registers. Due to this erratum, such a branch instruction that is executed both with CPL = 3 and with CPL < 3 may not use the correct MPX configuration register (BNDCFGU or BNDCFGS, respectively) for determining whether to initialize the bound registers; it may thus initialize the bound registers when it should not, or fail to initialize them when it should.

Implication:

A branch instruction that has executed both in user mode and in supervisor mode (from the same linear address) may cause a #BR (bound range fault) when it should not have or may not cause a #BR when it should have. Workaround An operating system can avoid this erratum by setting CR4.SMEP[bit 20] to enable supervisor-mode execution prevention (SMEP). When SMEP is enabled, no code can be executed both with CPL = 3 and with CPL < 3.

---

 b/arch/x86/include/asm/bugs.h  |    9 +++++++--
 b/arch/x86/kernel/cpu/common.c |    3 +++
 b/arch/x86/kernel/cpu/intel.c  |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff -puN arch/x86/include/asm/bugs.h~neversend-skl-mpx-errata-simple arch/x86/include/asm/bugs.h

arch/x86/include/asm/bugs.h renamed from arch/x86/include/asm/bugs.h~neversend-skl-mpx-errata-simple +7 -2
@@ -3,10 +3,15 @@
 
 extern void check_bugs(void);
 
-#if defined(CONFIG_CPU_SUP_INTEL) && defined(CONFIG_X86_32)
+#if defined(CONFIG_CPU_SUP_INTEL)
+void check_mpx_erratum(struct cpuinfo_x86 *c);
+#else
+static inline void check_mpx_erratum(struct cpuinfo_x86 *c) {}
+#if defined(CONFIG_X86_32)
 int ppro_with_ram_bug(void);
 #else
 static inline int ppro_with_ram_bug(void) { return 0; }
-#endif
+#endif /* CONFIG_X86_32 */
+#endif /* CONFIG_CPU_SUP_INTEL */
 
 #endif /* _ASM_X86_BUGS_H */

diff -puN arch/x86/kernel/cpu/common.c~neversend-skl-mpx-errata-simple arch/x86/kernel/cpu/common.c

arch/x86/kernel/cpu/common.c renamed from arch/x86/kernel/cpu/common.c~neversend-skl-mpx-errata-simple +3 -0
@@ -37,6 +37,7 @@
 #include <asm/mtrr.h>
 #include <linux/numa.h>
 #include <asm/asm.h>
+#include <asm/bugs.h>
 #include <asm/cpu.h>
 #include <asm/mce.h>
 #include <asm/msr.h>
@@ -270,6 +271,8 @@ static inline void squash_the_stupid_ser
 static __init int setup_disable_smep(char *arg)
 {
 	setup_clear_cpu_cap(X86_FEATURE_SMEP);
+	/* also check for things that depend on SMEP being enabled */
+	check_mpx_erratum(&boot_cpu_data);
 	return 1;
 }
 __setup("nosmep", setup_disable_smep);

diff -puN arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple arch/x86/kernel/cpu/intel.c

arch/x86/kernel/cpu/intel.c line 33

Quoted 9 lines
arch/x86/kernel/cpu/intel.c renamed from arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple +6 -0
@@ -25,6 +25,41 @@
 #include <asm/apic.h>
 #endif
 
+/*
+ * Just in case our CPU detection goes bad, allow a way to
+ * override the disabling of MPX.
+ */
+static int forcempx;
+static int __init forcempx_setup(char *__unused)

I've added a newline after the static variable.

arch/x86/kernel/cpu/intel.c line 38

Quoted 5 lines
arch/x86/kernel/cpu/intel.c renamed from arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple +5 -0
+{
+	forcempx = 1;
+	return 1;
+}
+__setup("intel-skd-046-workaround=disable", forcempx_setup);

I guess this is good - but our __setup() logic is sad - people who use intel-skd-046-workaround=0 won't even get a warning.

Quoted 12 lines
arch/x86/kernel/cpu/intel.c renamed from arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple +12 -0
+
+void check_mpx_erratum(struct cpuinfo_x86 *c)
+{
+	if (forcempx)
+	       return;
+	/*
+	 * Turn off MPX feature on CPUs where SMEP is not
+	 * available or disabled.
+	 *
+	 * Works around Intel Erratum: SKD046 Branch Instructions
+	 * May Initialize MPX Bound Registers Incorrectly.
+	 *

arch/x86/kernel/cpu/intel.c line 60

Quoted 10 lines
arch/x86/kernel/cpu/intel.c renamed from arch/x86/kernel/cpu/intel.c~neversend-skl-mpx-errata-simple +10 -0
+	 * This might falsely-disable MPX on systems without
+	 * SMEP, like Atom processors without SMEP.  But, there
+	 * are not any known places where that happens with
+	 * actual hardware.
+	 */
+	if (cpu_has(c, X86_FEATURE_MPX) &&
+	    !cpu_has(c, X86_FEATURE_SMEP)) {
+		setup_clear_cpu_cap(X86_FEATURE_MPX);
+		pr_warn("x86/mpx: disabling since SMEP not present\n");
+	}

I also slightly twiddled the wording, removed the linebreak from the condition (it was not over col80), removed whitespace noise and added your Signed-off-by, which I suppose you intended to add? I also tidied up the changelog a bit.

Final patch from tip:x86/urgent attached - please holler if you disagree with it!

Thanks,

Ingo

===================>

Quoted 1 line

From 41a057b30fe1c27c0a6b89b2f6bfea6a9ca43a6b Mon Sep 17 00:00:00 2001

From: Dave Hansen <[email protected]>
Date: Wed, 4 May 2016 13:53:59 -0700
Subject: [PATCH] x86/mm/mpx: Work around Intel MPX erratum SKD046

This erratum essentially causes the CPU to forget which privilege level it is operating on (kernel vs. user) for the purposes of MPX.

This erratum can only be triggered when a system is not using Supervisor Mode Execution Prevention (SMEP). Our workaround for the erratum is to ensure that MPX can only be used in cases where SMEP is present in the processor and is enabled.

This erratum only affects Core processors. Atom is unaffected. But, there is no architectural way to determine Atom vs. Core. So, we just apply this workaround to all processors. It's possible that it will mistakenly disable MPX on some Atom processsors or future unaffected Core processors. There are currently no processors that have MPX and not SMEP. It would take something akin to a hypervisor masking SMEP out on an Atom processor for this to present itself on current hardware.

More details can be found at:

http://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/desktop-6th-gen-core-family-spec-update.pdf

"
SKD046 Branch Instructions May Initialize MPX Bound Registers Incorrectly

Problem:

Depending on the current Intel MPX (Memory Protection Extensions) configuration, execution of certain branch instructions (near CALL, near RET, near JMP, and Jcc instructions) without a BND prefix (F2H) initialize the MPX bound registers. Due to this erratum, such a branch instruction that is executed both with CPL = 3 and with CPL < 3 may not use the correct MPX configuration register (BNDCFGU or BNDCFGS, respectively) for determining whether to initialize the bound registers; it may thus initialize the bound registers when it should not, or fail to initialize them when it should.

Implication:

A branch instruction that has executed both in user mode and in supervisor mode (from the same linear address) may cause a #BR (bound range fault) when it should not have or may not cause a #BR when it should have. Workaround An operating system can avoid this erratum by setting CR4.SMEP[bit 20] to enable supervisor-mode execution prevention (SMEP). When SMEP is enabled, no code can be executed both with CPL = 3 and with CPL < 3.
"

Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
---
 arch/x86/include/asm/bugs.h  |  7 ++++++-
 arch/x86/kernel/cpu/common.c |  3 +++
 arch/x86/kernel/cpu/intel.c  | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)
arch/x86/include/asm/bugs.h +6 -1
index 08abf639075f..9247f2f05a2b 100644
@@ -3,10 +3,15 @@
 
 extern void check_bugs(void);
 
-#if defined(CONFIG_CPU_SUP_INTEL) && defined(CONFIG_X86_32)
+#if defined(CONFIG_CPU_SUP_INTEL)
+void check_mpx_erratum(struct cpuinfo_x86 *c);
+#else
+static inline void check_mpx_erratum(struct cpuinfo_x86 *c) {}
+#ifdef CONFIG_X86_32
 int ppro_with_ram_bug(void);
 #else
 static inline int ppro_with_ram_bug(void) { return 0; }
 #endif
+#endif
 
 #endif /* _ASM_X86_BUGS_H */
arch/x86/kernel/cpu/common.c +3 -0
index 8394b3d1f94f..5782c6d87a33 100644
@@ -37,6 +37,7 @@
 #include <asm/mtrr.h>
 #include <linux/numa.h>
 #include <asm/asm.h>
+#include <asm/bugs.h>
 #include <asm/cpu.h>
 #include <asm/mce.h>
 #include <asm/msr.h>
@@ -270,6 +271,8 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
 static __init int setup_disable_smep(char *arg)
 {
 	setup_clear_cpu_cap(X86_FEATURE_SMEP);
+	/* Check for things that depend on SMEP being enabled: */
+	check_mpx_erratum(&boot_cpu_data);
 	return 1;
 }
 __setup("nosmep", setup_disable_smep);
arch/x86/kernel/cpu/intel.c +37 -0
index 1f7fdb91a818..6a80ffd17336 100644
@@ -25,6 +25,41 @@
 #include <asm/apic.h>
 #endif
 
+/*
+ * Just in case our CPU detection goes bad, or you have a weird system,
+ * allow a way to override the automatic disabling of MPX.
+ */
+static int forcempx;
+
+static int __init forcempx_setup(char *__unused)
+{
+	forcempx = 1;
+
+	return 1;
+}
+__setup("intel-skd-046-workaround=disable", forcempx_setup);
+
+void check_mpx_erratum(struct cpuinfo_x86 *c)
+{
+	if (forcempx)
+		return;
+	/*
+	 * Turn off the MPX feature on CPUs where SMEP is not
+	 * available or disabled.
+	 *
+	 * Works around Intel Erratum SKD046: "Branch Instructions
+	 * May Initialize MPX Bound Registers Incorrectly".
+	 *
+	 * This might falsely disable MPX on systems without
+	 * SMEP, like Atom processors without SMEP.  But there
+	 * is no such hardware known at the moment.
+	 */
+	if (cpu_has(c, X86_FEATURE_MPX) && !cpu_has(c, X86_FEATURE_SMEP)) {
+		setup_clear_cpu_cap(X86_FEATURE_MPX);
+		pr_warn("x86/mpx: Disabling MPX since SMEP not present\n");
+	}
+}
+
 static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
@@ -173,6 +208,8 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 		if (edx & (1U << 28))
 			c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff);
 	}
+
+	check_mpx_erratum(c);
 }
 
 #ifdef CONFIG_X86_32
#2 lore raw

Re: [PATCH v3] x86/mm/mpx: Work around MPX erratum SKD046

Quoted 4 lines · Ingo Molnar
Quoted 4 lines
arch/x86/include/asm/bugs.h +3 -1
-#if defined(CONFIG_CPU_SUP_INTEL) && defined(CONFIG_X86_32)
+#if defined(CONFIG_CPU_SUP_INTEL)
+void check_mpx_erratum(struct cpuinfo_x86 *c);
+#else
Quoted 8 lines

arch/x86/include/asm/bugs.h

Quoted 8 lines
arch/x86/include/asm/bugs.h +4 -1
+static inline void check_mpx_erratum(struct cpuinfo_x86 *c) {}
+#if defined(CONFIG_X86_32)
 int ppro_with_ram_bug(void);
 #else
 static inline int ppro_with_ram_bug(void) { return 0; }
-#endif
+#endif /* CONFIG_X86_32 */
+#endif /* CONFIG_CPU_SUP_INTEL */

Sigh. So this broke the ppro_with_ram_bug() definition, because in the new code you put it into the wrong branch, breaking the build ...

I've dropped this for now, please fix and test the v3 version of the patch I sent and re-send.

Thanks,

Ingo

#3 lore raw

Re: [PATCH v3] x86/mm/mpx: Work around MPX erratum SKD046

In reply to Ingo Molnar
Quoted 5 lines · Ingo Molnar

I also slightly twiddled the wording, removed the linebreak from the condition (it was not over col80), removed whitespace noise and added your Signed-off-by, which I suppose you intended to add? I also tidied up the changelog a bit.

Final patch from tip:x86/urgent attached - please holler if you disagree with it!

This all looks fine to me. I also tested that it works. Thanks!