All of lore.kernel.org
 help / color / mirror / Atom feed
* [MODERATED] [PATCH v3] Linux Patch 1/1
@ 2018-04-28  4:14 Tim Chen
  2018-04-28 16:07 ` [MODERATED] [PATCH v4] " Tim Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tim Chen @ 2018-04-28  4:14 UTC (permalink / raw)
  To: speck


[-- Attachment #1.1: Type: text/plain, Size: 15595 bytes --]


From 4b1ded256713497b71343eca65422711616b76b4 Mon Sep 17 00:00:00 2001
From: Tim Chen <tim.c.chen@linux.intel.com>
Date: Fri, 27 Apr 2018 03:33:04 -0700
Subject: [PATCH v3] x86/ssb: Enable option to mitigate SSB by explicit prctl

Processes that are unable to use other software mitigations and execute
untrusted code may want to disable speculative store bypass. This patch
adds a new prctl : PR_SET_SPEC_CTRL_MODE.
The prctl allows the control of speculation mode
to mitigate speculative store bypass vulnerability of the
current process.  When PR_SPEC_CTRL_MODE_RDS is set, the reduced data
speculation will be set for the process to provide mitigation.

The prctl mode is inherited to children to handle threads and allow wrappers,
but it is not broadcast to all threads in the same process. It is generally
safe to inherit because it doesn't allow any new attacks, but just prevents
them.

It also adds a matching prctl PR_GET_RDS_MODE to retrieve that state.

We allow PR_SPEC_CTRL_MODE_NONE to clear the mode after
PR_SPEC_CTRL_MODE_RDS is set. We expect that the attack will be mostly from
code within a JITed sandbox.  We want to prevent the JITed code in sandbox
from reading data it shouldn't read from its JITed sandbox.  A JIT can
turn on PR_SPEC_CTRL_MODE_RDS mode before running code in the JIT sandbox,
and turn it off when returning.  JIT sandboxed code should be unable to do system
calls so there is no danger of it turning off PR_SPEC_CTRL_MODE_RDS. 
So allowing the prctl to disable the mode later should be safe. If
the JIT'ed code can do system call, we have greater security issue on our hand.

We are not trying to protect native code processes in the same address space,
or threads within the same process. They already can access data of each other.

To Do:
1. For older kernel that does not support this prctl interface, we'll get
EINVAL returned.  We will like to return some a more informative value
if we issue this prctl on a kernel that support this interface, but
lack support on the platform.
2. Change the prctl name to PR_SET_SPEC_CTRL_MODE

v3:
1. Get rid of static key usage and code clean up from patch review.

v2:
1. Switch from using RDS on seccomp processes to using an explicit
prctl to enable RDS.

This patch is applied on top of v6 of Konrad's SSB patchset.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |   4 +
 arch/x86/include/asm/msr-index.h                |   3 +-
 arch/x86/include/asm/nospec-branch.h            |   4 +
 arch/x86/include/asm/thread_info.h              |   4 +-
 arch/x86/kernel/cpu/bugs.c                      | 105 +++++++++++++++++++++++-
 arch/x86/kernel/cpu/intel.c                     |   2 +-
 arch/x86/kernel/process.c                       |  24 ++++++
 include/uapi/linux/prctl.h                      |  13 +++
 kernel/sys.c                                    |  16 ++++
 9 files changed, 168 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a2d337b..387c14a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4053,6 +4053,10 @@
 			auto - kernel detects whether your CPU model contains a
 			       vulnerable implementation of Speculative Store
 			       Bypass and picks the most appropriate mitigation
+			prctl - disable Speculative Store Bypass for processes
+				via prctl. Processes run normally with
+				Seculative Store bypass by default unless
+				told otherwise by prctl.
 
 			Not specifying this option is equivalent to
 			spec_store_bypass_disable=auto.
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4c6b8f3..5bee7a2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -42,7 +42,8 @@
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
-#define SPEC_CTRL_RDS			(1 << 2)   /* Reduced Data Speculation */
+#define SPEC_CTRL_RDS_SHIFT		2	   /* Reduced Data Speculation bit */
+#define SPEC_CTRL_RDS			(1 << SPEC_CTRL_RDS_SHIFT)   /* Reduced Data Speculation */
 
 #define MSR_IA32_PRED_CMD		0x00000049 /* Prediction Command */
 #define PRED_CMD_IBPB			(1 << 0)   /* Indirect Branch Prediction Barrier */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index b3c6dbf..5bb0ac9 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -3,10 +3,12 @@
 #ifndef _ASM_X86_NOSPEC_BRANCH_H_
 #define _ASM_X86_NOSPEC_BRANCH_H_
 
+#include <linux/jump_label.h>
 #include <asm/alternative.h>
 #include <asm/alternative-asm.h>
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
+#include <asm/percpu.h>
 
 /*
  * Fill the CPU return stack buffer.
@@ -242,6 +244,7 @@ extern void x86_restore_host_spec_ctrl(u64);
 enum ssb_mitigation {
 	SPEC_STORE_BYPASS_NONE,
 	SPEC_STORE_BYPASS_DISABLE,
+	SPEC_STORE_BYPASS_PRCTL,
 };
 
 extern char __indirect_thunk_start[];
@@ -310,6 +313,7 @@ do {									\
 	preempt_enable();						\
 } while (0)
 
+extern bool specctrl_dynamic_rds;
 #endif /* __ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index a5d9521..e5c26cc 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -79,6 +79,7 @@ struct thread_info {
 #define TIF_SIGPENDING		2	/* signal pending */
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_SINGLESTEP		4	/* reenable singlestep on user return*/
+#define TIF_RDS			5	/* Reduced data speculation */
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
@@ -105,6 +106,7 @@ struct thread_info {
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1 << TIF_SINGLESTEP)
+#define _TIF_RDS		(1 << TIF_RDS)
 #define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
@@ -144,7 +146,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_RDS)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 191293e..4be7dbbe 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -12,6 +12,7 @@
 #include <linux/utsname.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
+#include <linux/prctl.h>
 
 #include <asm/nospec-branch.h>
 #include <asm/cmdline.h>
@@ -151,10 +152,16 @@ EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
 
 void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
 {
-	if (x86_get_default_spec_ctrl() == guest_spec_ctrl)
+	u64 host_val = x86_get_default_spec_ctrl();
+
+	if (test_tsk_thread_flag(current, TIF_RDS) &&
+	    boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		host_val |= SPEC_CTRL_RDS;
+
+	if (host_val == guest_spec_ctrl)
 		return;
 	else
-		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+		wrmsrl(MSR_IA32_SPEC_CTRL, host_val);
 }
 EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
 
@@ -379,16 +386,21 @@ static void __init spectre_v2_select_mitigation(void)
 
 static enum ssb_mitigation ssb_mode = SPEC_STORE_BYPASS_NONE;
 
+/* dynamic reduced data speculation in use */
+bool specctrl_dynamic_rds;
+
 /* The kernel command line selection */
 enum ssb_mitigation_cmd {
 	SPEC_STORE_BYPASS_CMD_NONE,
 	SPEC_STORE_BYPASS_CMD_AUTO,
 	SPEC_STORE_BYPASS_CMD_ON,
+	SPEC_STORE_BYPASS_CMD_PRCTL,
 };
 
 static const char *ssb_strings[] = {
 	[SPEC_STORE_BYPASS_NONE]	= "Vulnerable",
-	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled"
+	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled",
+	[SPEC_STORE_BYPASS_PRCTL]	= "Mitigation: Speculative Store Bypass disabled via prctl"
 };
 
 static const struct {
@@ -398,6 +410,7 @@ static const struct {
 	{ "auto",	SPEC_STORE_BYPASS_CMD_AUTO }, /* Platform decides */
 	{ "on",		SPEC_STORE_BYPASS_CMD_ON },   /* Disable Speculative Store Bypass */
 	{ "off",	SPEC_STORE_BYPASS_CMD_NONE }, /* Don't touch Speculative Store Bypass */
+	{ "prctl",	SPEC_STORE_BYPASS_CMD_PRCTL }, /* Disable Speculative Store Bypass via prctl */
 };
 
 static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void)
@@ -458,9 +471,20 @@ static void __init ssb_select_mitigation(void)
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
 			break;
+		/* Choose prctl as the default mode */
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_ON:
 		mode = SPEC_STORE_BYPASS_DISABLE;
 		break;
+	case SPEC_STORE_BYPASS_CMD_PRCTL:
+		/*
+		 * AMD platforms by default don't need SSB mitigation.
+		 */
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+			break;
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_NONE:
 		break;
 	}
@@ -481,13 +505,86 @@ static void __init ssb_select_mitigation(void)
 		 * a completely different MSR.
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
-			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
+			if (mode == SPEC_STORE_BYPASS_PRCTL)
+				specctrl_dynamic_rds = true;
+			else
+				x86_spec_ctrl_base |= SPEC_CTRL_RDS;
 			x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
 		}
 
 	}
 }
 
+static void start_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+               wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_RDS | x86_spec_ctrl_base);
+}
+
+static void stop_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+               wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
+int arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long val)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		/*
+		 * Reduced Data Speculation is set for whole platform,
+		 * only valid choice is PR_SPEC_CTRL_MODE_RDS.
+		 */
+		if (val == PR_SPEC_CTRL_MODE_RDS)
+			return 0;
+		else
+			return -EINVAL;
+
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (val == PR_SPEC_CTRL_MODE_RDS) {
+			set_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				start_speculative_store_bypass_mitigation();
+		} else if (val == PR_SPEC_CTRL_MODE_NONE) {
+			clear_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				stop_speculative_store_bypass_mitigation();
+		} else {
+			return -EINVAL;
+		}
+		return 0;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		if (val == PR_SPEC_CTRL_MODE_NONE)
+			return 0;
+		else
+			return -EINVAL;
+		break;
+	}
+	return -EINVAL;
+}
+
+int arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		return PR_SPEC_CTRL_MODE_RDS;
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (test_tsk_thread_flag(p, TIF_RDS))
+			return PR_SPEC_CTRL_MODE_RDS;
+		else
+			return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	}
+
+	return PR_SPEC_CTRL_MODE_NONE;
+}
+
 #undef pr_fmt
 
 #ifdef CONFIG_SYSFS
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8d6d6f4..d433c0f 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -773,7 +773,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 	init_intel_misc_features(c);
 
 	if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
-		x86_set_spec_ctrl(SPEC_CTRL_RDS);
+		x86_set_spec_ctrl(x86_get_default_spec_ctrl());
 }
 
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 03408b9..2253527 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -38,6 +38,7 @@
 #include <asm/switch_to.h>
 #include <asm/desc.h>
 #include <asm/prctl.h>
+#include <asm/nospec-branch.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -278,6 +279,26 @@ static inline void switch_to_bitmap(struct tss_struct *tss,
 	}
 }
 
+static inline void speculative_bypass_update(unsigned long tifn)
+{
+	u64 msr;
+
+	if (!specctrl_dynamic_rds)
+		return;
+
+	/*
+	 * Note: TIF_RDS bit position (5) is greater than
+	 * SPEC_CTRL_RDS bit position (2). The shift value
+	 * (TIF_RDS - SPEC_CTRL_RDS_SHIFT) used to convert from TIF_RDS
+	 * bit position to SPEC_CTRL_RDS bit position
+	 * below is positive and valid.
+	 */
+	msr = x86_get_default_spec_ctrl() |
+			((tifn & _TIF_RDS) >> (TIF_RDS - SPEC_CTRL_RDS_SHIFT));
+
+	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -309,6 +330,9 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 
 	if ((tifp ^ tifn) & _TIF_NOCPUID)
 		set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
+
+	if ((tifp ^ tifn) & _TIF_RDS)
+		speculative_bypass_update(tifn);
 }
 
 /*
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index af5f8c2..1ab361d 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -207,4 +207,17 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
+/*
+ * Speculation Control Mode knobs
+ *
+ * Set this on a task to control speculation mode.
+ * Initial use is to mitigate against speculative store bypass attack using
+ * reduced data speculation mode.
+ */
+#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
+#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
+#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
+
+#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index ad69218..6c8366b 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2242,6 +2242,16 @@ static int propagate_has_child_subreaper(struct task_struct *p, void *data)
 	return 1;
 }
 
+int __weak arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long arg)
+{
+	return 0;
+}
+
+int __weak arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	return 0;
+}
+
 SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		unsigned long, arg4, unsigned long, arg5)
 {
@@ -2450,6 +2460,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_SVE_GET_VL:
 		error = SVE_GET_VL();
 		break;
+	case PR_SET_SPEC_CTRL_MODE:
+		error = arch_prctl_set_spec_ctrl(me, arg2);
+		break;
+	case PR_GET_SPEC_CTRL_MODE:
+		error = arch_prctl_get_spec_ctrl(me);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.9.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-x86-ssb-Enable-option-to-mitigate-SSB-by-explicit-pr.patch --]
[-- Type: text/x-patch; name="0001-x86-ssb-Enable-option-to-mitigate-SSB-by-explicit-pr.patch", Size: 15534 bytes --]

From 4b1ded256713497b71343eca65422711616b76b4 Mon Sep 17 00:00:00 2001
From: Tim Chen <tim.c.chen@linux.intel.com>
Date: Fri, 27 Apr 2018 03:33:04 -0700
Subject: [PATCH v3] Linux Patch 1/1 

x86/ssb: Enable option to mitigate SSB by explicit prctl

Processes that are unable to use other software mitigations and execute
untrusted code may want to disable speculative store bypass. This patch
adds a new prctl : PR_SET_SPEC_CTRL_MODE.
The prctl allows the control of speculation mode
to mitigate speculative store bypass vulnerability of the
current process.  When PR_SPEC_CTRL_MODE_RDS is set, the reduced data
speculation will be set for the process to provide mitigation.

The prctl mode is inherited to children to handle threads and allow wrappers,
but it is not broadcast to all threads in the same process. It is generally
safe to inherit because it doesn't allow any new attacks, but just prevents
them.

It also adds a matching prctl PR_GET_RDS_MODE to retrieve that state.

We allow PR_SPEC_CTRL_MODE_NONE to clear the mode after
PR_SPEC_CTRL_MODE_RDS is set. We expect that the attack will be mostly from
code within a JITed sandbox.  We want to prevent the JITed code in sandbox
from reading data it shouldn't read from its JITed sandbox.  A JIT can
turn on PR_SPEC_CTRL_MODE_RDS mode before running code in the JIT sandbox,
and turn it off when returning.  JIT sandboxed code should be unable to do system
calls so there is no danger of it turning off PR_SPEC_CTRL_MODE_RDS. If
it can do system call, we have greater security issue on our hand.

We are not trying to protect native code processes in the same address space,
or threads within the same process. They already can access data of each other.

To Do:
1. For older kernel that does not support this prctl interface, we'll get
EINVAL returned.  We will like to return some a more informative value
if we issue this prctl on a kernel that support this interface, but
lack support on the platform.
2. Change the prctl name to PR_SET_SPEC_CTRL_MODE

v3:
1. Get rid of static key usage and code clean up from patch review.

v2:
1. Switch from using RDS on seccomp processes to using an explicit
prctl to enable RDS.

This patch is applied on top of v6 of Konrad's SSB patchset.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |   4 +
 arch/x86/include/asm/msr-index.h                |   3 +-
 arch/x86/include/asm/nospec-branch.h            |   4 +
 arch/x86/include/asm/thread_info.h              |   4 +-
 arch/x86/kernel/cpu/bugs.c                      | 105 +++++++++++++++++++++++-
 arch/x86/kernel/cpu/intel.c                     |   2 +-
 arch/x86/kernel/process.c                       |  24 ++++++
 include/uapi/linux/prctl.h                      |  13 +++
 kernel/sys.c                                    |  16 ++++
 9 files changed, 168 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a2d337b..387c14a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4053,6 +4053,10 @@
 			auto - kernel detects whether your CPU model contains a
 			       vulnerable implementation of Speculative Store
 			       Bypass and picks the most appropriate mitigation
+			prctl - disable Speculative Store Bypass for processes
+				via prctl. Processes run normally with
+				Seculative Store bypass by default unless
+				told otherwise by prctl.
 
 			Not specifying this option is equivalent to
 			spec_store_bypass_disable=auto.
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4c6b8f3..5bee7a2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -42,7 +42,8 @@
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
-#define SPEC_CTRL_RDS			(1 << 2)   /* Reduced Data Speculation */
+#define SPEC_CTRL_RDS_SHIFT		2	   /* Reduced Data Speculation bit */
+#define SPEC_CTRL_RDS			(1 << SPEC_CTRL_RDS_SHIFT)   /* Reduced Data Speculation */
 
 #define MSR_IA32_PRED_CMD		0x00000049 /* Prediction Command */
 #define PRED_CMD_IBPB			(1 << 0)   /* Indirect Branch Prediction Barrier */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index b3c6dbf..5bb0ac9 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -3,10 +3,12 @@
 #ifndef _ASM_X86_NOSPEC_BRANCH_H_
 #define _ASM_X86_NOSPEC_BRANCH_H_
 
+#include <linux/jump_label.h>
 #include <asm/alternative.h>
 #include <asm/alternative-asm.h>
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
+#include <asm/percpu.h>
 
 /*
  * Fill the CPU return stack buffer.
@@ -242,6 +244,7 @@ extern void x86_restore_host_spec_ctrl(u64);
 enum ssb_mitigation {
 	SPEC_STORE_BYPASS_NONE,
 	SPEC_STORE_BYPASS_DISABLE,
+	SPEC_STORE_BYPASS_PRCTL,
 };
 
 extern char __indirect_thunk_start[];
@@ -310,6 +313,7 @@ do {									\
 	preempt_enable();						\
 } while (0)
 
+extern bool specctrl_dynamic_rds;
 #endif /* __ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index a5d9521..e5c26cc 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -79,6 +79,7 @@ struct thread_info {
 #define TIF_SIGPENDING		2	/* signal pending */
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_SINGLESTEP		4	/* reenable singlestep on user return*/
+#define TIF_RDS			5	/* Reduced data speculation */
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
@@ -105,6 +106,7 @@ struct thread_info {
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1 << TIF_SINGLESTEP)
+#define _TIF_RDS		(1 << TIF_RDS)
 #define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
@@ -144,7 +146,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_RDS)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 191293e..4be7dbbe 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -12,6 +12,7 @@
 #include <linux/utsname.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
+#include <linux/prctl.h>
 
 #include <asm/nospec-branch.h>
 #include <asm/cmdline.h>
@@ -151,10 +152,16 @@ EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
 
 void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
 {
-	if (x86_get_default_spec_ctrl() == guest_spec_ctrl)
+	u64 host_val = x86_get_default_spec_ctrl();
+
+	if (test_tsk_thread_flag(current, TIF_RDS) &&
+	    boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		host_val |= SPEC_CTRL_RDS;
+
+	if (host_val == guest_spec_ctrl)
 		return;
 	else
-		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+		wrmsrl(MSR_IA32_SPEC_CTRL, host_val);
 }
 EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
 
@@ -379,16 +386,21 @@ static void __init spectre_v2_select_mitigation(void)
 
 static enum ssb_mitigation ssb_mode = SPEC_STORE_BYPASS_NONE;
 
+/* dynamic reduced data speculation in use */
+bool specctrl_dynamic_rds;
+
 /* The kernel command line selection */
 enum ssb_mitigation_cmd {
 	SPEC_STORE_BYPASS_CMD_NONE,
 	SPEC_STORE_BYPASS_CMD_AUTO,
 	SPEC_STORE_BYPASS_CMD_ON,
+	SPEC_STORE_BYPASS_CMD_PRCTL,
 };
 
 static const char *ssb_strings[] = {
 	[SPEC_STORE_BYPASS_NONE]	= "Vulnerable",
-	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled"
+	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled",
+	[SPEC_STORE_BYPASS_PRCTL]	= "Mitigation: Speculative Store Bypass disabled via prctl"
 };
 
 static const struct {
@@ -398,6 +410,7 @@ static const struct {
 	{ "auto",	SPEC_STORE_BYPASS_CMD_AUTO }, /* Platform decides */
 	{ "on",		SPEC_STORE_BYPASS_CMD_ON },   /* Disable Speculative Store Bypass */
 	{ "off",	SPEC_STORE_BYPASS_CMD_NONE }, /* Don't touch Speculative Store Bypass */
+	{ "prctl",	SPEC_STORE_BYPASS_CMD_PRCTL }, /* Disable Speculative Store Bypass via prctl */
 };
 
 static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void)
@@ -458,9 +471,20 @@ static void __init ssb_select_mitigation(void)
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
 			break;
+		/* Choose prctl as the default mode */
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_ON:
 		mode = SPEC_STORE_BYPASS_DISABLE;
 		break;
+	case SPEC_STORE_BYPASS_CMD_PRCTL:
+		/*
+		 * AMD platforms by default don't need SSB mitigation.
+		 */
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+			break;
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_NONE:
 		break;
 	}
@@ -481,13 +505,86 @@ static void __init ssb_select_mitigation(void)
 		 * a completely different MSR.
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
-			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
+			if (mode == SPEC_STORE_BYPASS_PRCTL)
+				specctrl_dynamic_rds = true;
+			else
+				x86_spec_ctrl_base |= SPEC_CTRL_RDS;
 			x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
 		}
 
 	}
 }
 
+static void start_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+               wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_RDS | x86_spec_ctrl_base);
+}
+
+static void stop_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+               wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
+int arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long val)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		/*
+		 * Reduced Data Speculation is set for whole platform,
+		 * only valid choice is PR_SPEC_CTRL_MODE_RDS.
+		 */
+		if (val == PR_SPEC_CTRL_MODE_RDS)
+			return 0;
+		else
+			return -EINVAL;
+
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (val == PR_SPEC_CTRL_MODE_RDS) {
+			set_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				start_speculative_store_bypass_mitigation();
+		} else if (val == PR_SPEC_CTRL_MODE_NONE) {
+			clear_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				stop_speculative_store_bypass_mitigation();
+		} else {
+			return -EINVAL;
+		}
+		return 0;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		if (val == PR_SPEC_CTRL_MODE_NONE)
+			return 0;
+		else
+			return -EINVAL;
+		break;
+	}
+	return -EINVAL;
+}
+
+int arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		return PR_SPEC_CTRL_MODE_RDS;
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (test_tsk_thread_flag(p, TIF_RDS))
+			return PR_SPEC_CTRL_MODE_RDS;
+		else
+			return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	}
+
+	return PR_SPEC_CTRL_MODE_NONE;
+}
+
 #undef pr_fmt
 
 #ifdef CONFIG_SYSFS
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8d6d6f4..d433c0f 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -773,7 +773,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 	init_intel_misc_features(c);
 
 	if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
-		x86_set_spec_ctrl(SPEC_CTRL_RDS);
+		x86_set_spec_ctrl(x86_get_default_spec_ctrl());
 }
 
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 03408b9..2253527 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -38,6 +38,7 @@
 #include <asm/switch_to.h>
 #include <asm/desc.h>
 #include <asm/prctl.h>
+#include <asm/nospec-branch.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -278,6 +279,26 @@ static inline void switch_to_bitmap(struct tss_struct *tss,
 	}
 }
 
+static inline void speculative_bypass_update(unsigned long tifn)
+{
+	u64 msr;
+
+	if (!specctrl_dynamic_rds)
+		return;
+
+	/*
+	 * Note: TIF_RDS bit position (5) is greater than
+	 * SPEC_CTRL_RDS bit position (2). The shift value
+	 * (TIF_RDS - SPEC_CTRL_RDS_SHIFT) used to convert from TIF_RDS
+	 * bit position to SPEC_CTRL_RDS bit position
+	 * below is positive and valid.
+	 */
+	msr = x86_get_default_spec_ctrl() |
+			((tifn & _TIF_RDS) >> (TIF_RDS - SPEC_CTRL_RDS_SHIFT));
+
+	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -309,6 +330,9 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 
 	if ((tifp ^ tifn) & _TIF_NOCPUID)
 		set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
+
+	if ((tifp ^ tifn) & _TIF_RDS)
+		speculative_bypass_update(tifn);
 }
 
 /*
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index af5f8c2..1ab361d 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -207,4 +207,17 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
+/*
+ * Speculation Control Mode knobs
+ *
+ * Set this on a task to control speculation mode.
+ * Initial use is to mitigate against speculative store bypass attack using
+ * reduced data speculation mode.
+ */
+#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
+#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
+#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
+
+#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index ad69218..6c8366b 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2242,6 +2242,16 @@ static int propagate_has_child_subreaper(struct task_struct *p, void *data)
 	return 1;
 }
 
+int __weak arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long arg)
+{
+	return 0;
+}
+
+int __weak arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	return 0;
+}
+
 SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		unsigned long, arg4, unsigned long, arg5)
 {
@@ -2450,6 +2460,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_SVE_GET_VL:
 		error = SVE_GET_VL();
 		break;
+	case PR_SET_SPEC_CTRL_MODE:
+		error = arch_prctl_set_spec_ctrl(me, arg2);
+		break;
+	case PR_GET_SPEC_CTRL_MODE:
+		error = arch_prctl_get_spec_ctrl(me);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MODERATED] [PATCH v4] Linux Patch 1/1
  2018-04-28  4:14 [MODERATED] [PATCH v3] Linux Patch 1/1 Tim Chen
@ 2018-04-28 16:07 ` Tim Chen
  2018-04-28 18:15 ` [MODERATED] Re: [PATCH v3] " Jon Masters
  2018-04-28 19:20 ` Thomas Gleixner
  2 siblings, 0 replies; 10+ messages in thread
From: Tim Chen @ 2018-04-28 16:07 UTC (permalink / raw)
  To: speck


[-- Attachment #1.1: Type: text/plain, Size: 15942 bytes --]

From 78fbf3237435a14cdebeb354e890029f7243aa58 Mon Sep 17 00:00:00 2001
From: Tim Chen <tim.c.chen@linux.intel.com>
Date: Fri, 27 Apr 2018 03:33:04 -0700
Subject: [PATCH] x86/ssb: Enable option to mitigate SSB by explicit prctl

Processes that are unable to use other software mitigations and execute
untrusted code may want to disable speculative store bypass. This patch
adds a new prctl : PR_SET_SPEC_CTRL_MODE.
The prctl allows the control of speculation mode
to mitigate speculative store bypass vulnerability of the
current process.  When PR_SPEC_CTRL_MODE_RDS is set, the reduced data
speculation will be set for the process to provide mitigation.

The prctl mode is inherited to children to handle threads and allow wrappers,
but it is not broadcast to all threads in the same process. It is generally
safe to inherit because it doesn't allow any new attacks, but just prevents
them.

It also adds a matching prctl PR_GET_SPEC_CTRL_MODE to retrieve that state.

We allow using PR_SPEC_CTRL_MODE_NONE to clear the mode after
PR_SPEC_CTRL_MODE_RDS is set. We expect that the attack will be from
code within a JITed sandbox.  We want to prevent the JITed code in sandbox
from reading data it shouldn't read from its JITed sandbox.  A JIT can
turn on PR_SPEC_CTRL_MODE_RDS mode before running code in the JIT sandbox,
and turn it off when returning.  JIT sandboxed code should be unable to do system
calls so there is no danger of it turning off PR_SPEC_CTRL_MODE_RDS.
So allowing the prctl to disable the mode later should be safe. If
the JIT'ed code can do system call, we have greater security issue on our hand.

We are not trying to protect native code processes in the same address space,
or threads within the same process. They already can access data of each other.

To Do:
1. For older kernel that does not support this prctl interface, we'll get
EINVAL returned.  We will like to return some a more informative value
if we issue this prctl on a kernel that support this interface, but
lack support on the platform.

v4:
1. Fix incorrect condition for applying RDS in x86_restore_host_spec_ctrl and
x86_set_guest_spec_ctrl.
2. Fix up a few unnecessary vendor checks after checking specctrl_dynamic_rds,
which is set only for Intel cpu.

v3:
1. Get rid of static key usage and code clean up from patch review.

v2:
1. Switch from using RDS on seccomp processes to using an explicit
prctl to enable RDS.

This patch is applied on top of v6 of Konrad's SSB patchset.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |   4 +
 arch/x86/include/asm/msr-index.h                |   3 +-
 arch/x86/include/asm/nospec-branch.h            |   3 +
 arch/x86/include/asm/thread_info.h              |   4 +-
 arch/x86/kernel/cpu/bugs.c                      | 111 ++++++++++++++++++++++--
 arch/x86/kernel/cpu/intel.c                     |   2 +-
 arch/x86/kernel/process.c                       |  24 +++++
 include/uapi/linux/prctl.h                      |  13 +++
 kernel/sys.c                                    |  16 ++++
 9 files changed, 172 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a2d337b..387c14a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4053,6 +4053,10 @@
 			auto - kernel detects whether your CPU model contains a
 			       vulnerable implementation of Speculative Store
 			       Bypass and picks the most appropriate mitigation
+			prctl - disable Speculative Store Bypass for processes
+				via prctl. Processes run normally with
+				Seculative Store bypass by default unless
+				told otherwise by prctl.
 
 			Not specifying this option is equivalent to
 			spec_store_bypass_disable=auto.
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4c6b8f3..5bee7a2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -42,7 +42,8 @@
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
-#define SPEC_CTRL_RDS			(1 << 2)   /* Reduced Data Speculation */
+#define SPEC_CTRL_RDS_SHIFT		2	   /* Reduced Data Speculation bit */
+#define SPEC_CTRL_RDS			(1 << SPEC_CTRL_RDS_SHIFT)   /* Reduced Data Speculation */
 
 #define MSR_IA32_PRED_CMD		0x00000049 /* Prediction Command */
 #define PRED_CMD_IBPB			(1 << 0)   /* Indirect Branch Prediction Barrier */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index b3c6dbf..b5c594e 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -7,6 +7,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.
@@ -242,6 +243,7 @@ extern void x86_restore_host_spec_ctrl(u64);
 enum ssb_mitigation {
 	SPEC_STORE_BYPASS_NONE,
 	SPEC_STORE_BYPASS_DISABLE,
+	SPEC_STORE_BYPASS_PRCTL,
 };
 
 extern char __indirect_thunk_start[];
@@ -310,6 +312,7 @@ do {									\
 	preempt_enable();						\
 } while (0)
 
+extern bool specctrl_dynamic_rds;
 #endif /* __ASSEMBLY__ */
 
 /*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index a5d9521..e5c26cc 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -79,6 +79,7 @@ struct thread_info {
 #define TIF_SIGPENDING		2	/* signal pending */
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_SINGLESTEP		4	/* reenable singlestep on user return*/
+#define TIF_RDS			5	/* Reduced data speculation */
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
@@ -105,6 +106,7 @@ struct thread_info {
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1 << TIF_SINGLESTEP)
+#define _TIF_RDS		(1 << TIF_RDS)
 #define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
@@ -144,7 +146,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_RDS)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 191293e..6a2e9d2 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -12,6 +12,7 @@
 #include <linux/utsname.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
+#include <linux/prctl.h>
 
 #include <asm/nospec-branch.h>
 #include <asm/cmdline.h>
@@ -142,7 +143,12 @@ EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl);
 
 void x86_set_guest_spec_ctrl(u64 guest_spec_ctrl)
 {
-	if (x86_get_default_spec_ctrl() == guest_spec_ctrl)
+	u64 host_val = x86_get_default_spec_ctrl();
+
+	if (specctrl_dynamic_rds && test_tsk_thread_flag(current, TIF_RDS))
+		host_val |= SPEC_CTRL_RDS;
+
+	if (host_val == guest_spec_ctrl)
 		return;
 	else
 		wrmsrl(MSR_IA32_SPEC_CTRL, guest_spec_ctrl);
@@ -151,10 +157,15 @@ EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
 
 void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
 {
-	if (x86_get_default_spec_ctrl() == guest_spec_ctrl)
+	u64 host_val = x86_get_default_spec_ctrl();
+
+	if (specctrl_dynamic_rds && test_tsk_thread_flag(current, TIF_RDS))
+		host_val |= SPEC_CTRL_RDS;
+
+	if (host_val == guest_spec_ctrl)
 		return;
 	else
-		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+		wrmsrl(MSR_IA32_SPEC_CTRL, host_val);
 }
 EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
 
@@ -379,16 +390,21 @@ static void __init spectre_v2_select_mitigation(void)
 
 static enum ssb_mitigation ssb_mode = SPEC_STORE_BYPASS_NONE;
 
+/* dynamic reduced data speculation in use */
+bool specctrl_dynamic_rds;
+
 /* The kernel command line selection */
 enum ssb_mitigation_cmd {
 	SPEC_STORE_BYPASS_CMD_NONE,
 	SPEC_STORE_BYPASS_CMD_AUTO,
 	SPEC_STORE_BYPASS_CMD_ON,
+	SPEC_STORE_BYPASS_CMD_PRCTL,
 };
 
 static const char *ssb_strings[] = {
 	[SPEC_STORE_BYPASS_NONE]	= "Vulnerable",
-	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled"
+	[SPEC_STORE_BYPASS_DISABLE]	= "Mitigation: Speculative Store Bypass disabled",
+	[SPEC_STORE_BYPASS_PRCTL]	= "Mitigation: Speculative Store Bypass disabled via prctl"
 };
 
 static const struct {
@@ -398,6 +414,7 @@ static const struct {
 	{ "auto",	SPEC_STORE_BYPASS_CMD_AUTO }, /* Platform decides */
 	{ "on",		SPEC_STORE_BYPASS_CMD_ON },   /* Disable Speculative Store Bypass */
 	{ "off",	SPEC_STORE_BYPASS_CMD_NONE }, /* Don't touch Speculative Store Bypass */
+	{ "prctl",	SPEC_STORE_BYPASS_CMD_PRCTL }, /* Disable Speculative Store Bypass via prctl */
 };
 
 static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void)
@@ -458,9 +475,20 @@ static void __init ssb_select_mitigation(void)
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
 			break;
+		/* Choose prctl as the default mode */
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_ON:
 		mode = SPEC_STORE_BYPASS_DISABLE;
 		break;
+	case SPEC_STORE_BYPASS_CMD_PRCTL:
+		/*
+		 * AMD platforms by default don't need SSB mitigation.
+		 */
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+			break;
+		mode = SPEC_STORE_BYPASS_PRCTL;
+		break;
 	case SPEC_STORE_BYPASS_CMD_NONE:
 		break;
 	}
@@ -481,13 +509,86 @@ static void __init ssb_select_mitigation(void)
 		 * a completely different MSR.
 		 */
 		if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
-			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
+			if (mode == SPEC_STORE_BYPASS_PRCTL)
+				specctrl_dynamic_rds = true;
+			else
+				x86_spec_ctrl_base |= SPEC_CTRL_RDS;
 			x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
 		}
 
 	}
 }
 
+static void start_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds)
+               wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_RDS | x86_spec_ctrl_base);
+}
+
+static void stop_speculative_store_bypass_mitigation(void)
+{
+       if (specctrl_dynamic_rds)
+               wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
+int arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long val)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		/*
+		 * Reduced Data Speculation is set for whole platform,
+		 * only valid choice is PR_SPEC_CTRL_MODE_RDS.
+		 */
+		if (val == PR_SPEC_CTRL_MODE_RDS)
+			return 0;
+		else
+			return -EINVAL;
+
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (val == PR_SPEC_CTRL_MODE_RDS) {
+			set_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				start_speculative_store_bypass_mitigation();
+		} else if (val == PR_SPEC_CTRL_MODE_NONE) {
+			clear_tsk_thread_flag(p, TIF_RDS);
+			if (p == current)
+				stop_speculative_store_bypass_mitigation();
+		} else {
+			return -EINVAL;
+		}
+		return 0;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		if (val == PR_SPEC_CTRL_MODE_NONE)
+			return 0;
+		else
+			return -EINVAL;
+		break;
+	}
+	return -EINVAL;
+}
+
+int arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	switch (ssb_mode) {
+	case SPEC_STORE_BYPASS_DISABLE:
+		return PR_SPEC_CTRL_MODE_RDS;
+		break;
+	case SPEC_STORE_BYPASS_PRCTL:
+		if (test_tsk_thread_flag(p, TIF_RDS))
+			return PR_SPEC_CTRL_MODE_RDS;
+		else
+			return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	case SPEC_STORE_BYPASS_NONE:
+		return PR_SPEC_CTRL_MODE_NONE;
+		break;
+	}
+
+	return PR_SPEC_CTRL_MODE_NONE;
+}
+
 #undef pr_fmt
 
 #ifdef CONFIG_SYSFS
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8d6d6f4..d433c0f 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -773,7 +773,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 	init_intel_misc_features(c);
 
 	if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
-		x86_set_spec_ctrl(SPEC_CTRL_RDS);
+		x86_set_spec_ctrl(x86_get_default_spec_ctrl());
 }
 
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 03408b9..2253527 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -38,6 +38,7 @@
 #include <asm/switch_to.h>
 #include <asm/desc.h>
 #include <asm/prctl.h>
+#include <asm/nospec-branch.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -278,6 +279,26 @@ static inline void switch_to_bitmap(struct tss_struct *tss,
 	}
 }
 
+static inline void speculative_bypass_update(unsigned long tifn)
+{
+	u64 msr;
+
+	if (!specctrl_dynamic_rds)
+		return;
+
+	/*
+	 * Note: TIF_RDS bit position (5) is greater than
+	 * SPEC_CTRL_RDS bit position (2). The shift value
+	 * (TIF_RDS - SPEC_CTRL_RDS_SHIFT) used to convert from TIF_RDS
+	 * bit position to SPEC_CTRL_RDS bit position
+	 * below is positive and valid.
+	 */
+	msr = x86_get_default_spec_ctrl() |
+			((tifn & _TIF_RDS) >> (TIF_RDS - SPEC_CTRL_RDS_SHIFT));
+
+	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -309,6 +330,9 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 
 	if ((tifp ^ tifn) & _TIF_NOCPUID)
 		set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
+
+	if ((tifp ^ tifn) & _TIF_RDS)
+		speculative_bypass_update(tifn);
 }
 
 /*
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index af5f8c2..1ab361d 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -207,4 +207,17 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
+/*
+ * Speculation Control Mode knobs
+ *
+ * Set this on a task to control speculation mode.
+ * Initial use is to mitigate against speculative store bypass attack using
+ * reduced data speculation mode.
+ */
+#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
+#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
+#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
+
+#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index ad69218..6c8366b 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2242,6 +2242,16 @@ static int propagate_has_child_subreaper(struct task_struct *p, void *data)
 	return 1;
 }
 
+int __weak arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long arg)
+{
+	return 0;
+}
+
+int __weak arch_prctl_get_spec_ctrl(struct task_struct *p)
+{
+	return 0;
+}
+
 SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		unsigned long, arg4, unsigned long, arg5)
 {
@@ -2450,6 +2460,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_SVE_GET_VL:
 		error = SVE_GET_VL();
 		break;
+	case PR_SET_SPEC_CTRL_MODE:
+		error = arch_prctl_set_spec_ctrl(me, arg2);
+		break;
+	case PR_GET_SPEC_CTRL_MODE:
+		error = arch_prctl_get_spec_ctrl(me);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3] Linux Patch 1/1
  2018-04-28  4:14 [MODERATED] [PATCH v3] Linux Patch 1/1 Tim Chen
  2018-04-28 16:07 ` [MODERATED] [PATCH v4] " Tim Chen
@ 2018-04-28 18:15 ` Jon Masters
  2018-04-28 21:20   ` Thomas Gleixner
  2018-04-28 19:20 ` Thomas Gleixner
  2 siblings, 1 reply; 10+ messages in thread
From: Jon Masters @ 2018-04-28 18:15 UTC (permalink / raw)
  To: speck

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

On 04/28/2018 12:14 AM, speck for Tim Chen wrote:

> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -207,4 +207,17 @@ struct prctl_mm_map {
>  # define PR_SVE_VL_LEN_MASK		0xffff
>  # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
>  
> +/*
> + * Speculation Control Mode knobs
> + *
> + * Set this on a task to control speculation mode.
> + * Initial use is to mitigate against speculative store bypass attack using
> + * reduced data speculation mode.
> + */
> +#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
> +#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
> +#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
> +
> +#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */

Can we use a more generic term like "SSB" for the non-arch specific
code? I expect at least Arm to also wire up the prctl shortly.

For those of us on the distro end, we mostly care that the ABI is locked
down in the next day or two. If the actual constant name changes in the
header but the numbers remain the same, we can (ugly) go and rebuild
stuff a second time later to clean that up, so the main thing I really
care about is that the prctl numbers/semantics get locked asap.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Linux Patch 1/1
  2018-04-28  4:14 [MODERATED] [PATCH v3] Linux Patch 1/1 Tim Chen
  2018-04-28 16:07 ` [MODERATED] [PATCH v4] " Tim Chen
  2018-04-28 18:15 ` [MODERATED] Re: [PATCH v3] " Jon Masters
@ 2018-04-28 19:20 ` Thomas Gleixner
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-28 19:20 UTC (permalink / raw)
  To: speck

On Fri, 27 Apr 2018, speck for Tim Chen wrote:
>  
> +extern bool specctrl_dynamic_rds;

Whatfor?

>  #include <asm/nospec-branch.h>
>  #include <asm/cmdline.h>
> @@ -151,10 +152,16 @@ EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
>  
>  void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
>  {
> -	if (x86_get_default_spec_ctrl() == guest_spec_ctrl)
> +	u64 host_val = x86_get_default_spec_ctrl();
> +
> +	if (test_tsk_thread_flag(current, TIF_RDS) &&
> +	    boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> +		host_val |= SPEC_CTRL_RDS;

Why can't you make that right away in a way that the AMD stuff is simple to
integrate? Because its simpler to create mess in the first place and let
others mop it up.

> +
> +	if (host_val == guest_spec_ctrl)
>  		return;
>  	else
> -		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> +		wrmsrl(MSR_IA32_SPEC_CTRL, host_val);
>  }
>  EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
>  
> @@ -379,16 +386,21 @@ static void __init spectre_v2_select_mitigation(void)
  
>  static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void)
> @@ -458,9 +471,20 @@ static void __init ssb_select_mitigation(void)
>  		 */
>  		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>  			break;
> +		/* Choose prctl as the default mode */
> +		mode = SPEC_STORE_BYPASS_PRCTL;
> +		break;
>  	case SPEC_STORE_BYPASS_CMD_ON:
>  		mode = SPEC_STORE_BYPASS_DISABLE;
>  		break;
> +	case SPEC_STORE_BYPASS_CMD_PRCTL:
> +		/*
> +		 * AMD platforms by default don't need SSB mitigation.
> +		 */
> +		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> +			break;

If the user selects PRCTL then why are you refusing it for AMD? Just
because mindless copy and paste is so wonderful.

> +		mode = SPEC_STORE_BYPASS_PRCTL;
> +		break;
>  	case SPEC_STORE_BYPASS_CMD_NONE:
>  		break;
>  	}
> @@ -481,13 +505,86 @@ static void __init ssb_select_mitigation(void)
>  		 * a completely different MSR.
>  		 */
>  		if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
> -			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
> +			if (mode == SPEC_STORE_BYPASS_PRCTL)
> +				specctrl_dynamic_rds = true;

Why on earth is this needed?

> +			else
> +				x86_spec_ctrl_base |= SPEC_CTRL_RDS;
>  			x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
>  		}
>  
>  	}
>  }
>  
> +static void start_speculative_store_bypass_mitigation(void)
> +{
> +       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> +               wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_RDS | x86_spec_ctrl_base);
> +}
> +
> +static void stop_speculative_store_bypass_mitigation(void)
> +{
> +       if (specctrl_dynamic_rds && boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> +               wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> +}

Groan. This is not needed and all and I demonstrated it.

> +int arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long val)
> +{
> +	switch (ssb_mode) {
> +	case SPEC_STORE_BYPASS_DISABLE:
> +		/*
> +		 * Reduced Data Speculation is set for whole platform,
> +		 * only valid choice is PR_SPEC_CTRL_MODE_RDS.
> +		 */
> +		if (val == PR_SPEC_CTRL_MODE_RDS)
> +			return 0;
> +		else
> +			return -EINVAL;
> +
> +		break;
> +	case SPEC_STORE_BYPASS_PRCTL:
> +		if (val == PR_SPEC_CTRL_MODE_RDS) {
> +			set_tsk_thread_flag(p, TIF_RDS);
> +			if (p == current)
> +				start_speculative_store_bypass_mitigation();
> +		} else if (val == PR_SPEC_CTRL_MODE_NONE) {
> +			clear_tsk_thread_flag(p, TIF_RDS);
> +			if (p == current)
> +				stop_speculative_store_bypass_mitigation();
> +		} else {
> +			return -EINVAL;
> +		}
> +		return 0;
> +		break;
> +	case SPEC_STORE_BYPASS_NONE:
> +		if (val == PR_SPEC_CTRL_MODE_NONE)
> +			return 0;
> +		else
> +			return -EINVAL;
> +		break;
> +	}
> +	return -EINVAL;

There surely was no more convoluted way to write that.

> +}
> +
> +int arch_prctl_get_spec_ctrl(struct task_struct *p)
> +{
> +	switch (ssb_mode) {
> +	case SPEC_STORE_BYPASS_DISABLE:
> +		return PR_SPEC_CTRL_MODE_RDS;
> +		break;

And break after return serves which purpose? 

> +	case SPEC_STORE_BYPASS_PRCTL:
> +		if (test_tsk_thread_flag(p, TIF_RDS))
> +			return PR_SPEC_CTRL_MODE_RDS;
> +		else
> +			return PR_SPEC_CTRL_MODE_NONE;

And why can't this just fall through? Because more lines make a better
patch?

> +		break;
> +	case SPEC_STORE_BYPASS_NONE:
> +		return PR_SPEC_CTRL_MODE_NONE;
> +		break;
> +	}
> +
> +	return PR_SPEC_CTRL_MODE_NONE;
> +}
> +
>  #undef pr_fmt
>  
>  #ifdef CONFIG_SYSFS
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 8d6d6f4..d433c0f 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -773,7 +773,7 @@ static void init_intel(struct cpuinfo_x86 *c)
>  	init_intel_misc_features(c);
>  
>  	if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
> -		x86_set_spec_ctrl(SPEC_CTRL_RDS);
> +		x86_set_spec_ctrl(x86_get_default_spec_ctrl());

Did anyone notice by now that this is just wrong? Because the way how this
works is not functional for PRCTL on AMD. And it's just a stupid idea in
general. The SPEC CTRL MSR initilization belongs into the mitigation
code. The original order of calls was entirely correct and the AMD stuff
can be made to work very easily on top of that and the required data is
also necessary to make the TIF thing work on AMD nicely as well.

> +static inline void speculative_bypass_update(unsigned long tifn)
> +{
> +	u64 msr;
> +
> +	if (!specctrl_dynamic_rds)
> +		return;

No. No. No. Did you even read what I wrote?

This code is NEVER reached when the PRCTL mode is disabled simple because
nothing can ever set TIP_RDS on any task.

Do you really think I write reviews with code examples just because I'm
bored to death? If you think I'm wrong with my example or review comment,
then f*cking please tell me in a reply instead of silently ignoring me.

> +	/*
> +	 * Note: TIF_RDS bit position (5) is greater than
> +	 * SPEC_CTRL_RDS bit position (2). The shift value
> +	 * (TIF_RDS - SPEC_CTRL_RDS_SHIFT) used to convert from TIF_RDS
> +	 * bit position to SPEC_CTRL_RDS bit position
> +	 * below is positive and valid.

What a crappy comment. In the example I gave you I made that a separate
define on purpose. And then this needs a BUILD_BUG_ON(TIF_RDS <
SPEC....SHIFT). That's a gazillion times more valuable than this comment.

> +	 */
> +	msr = x86_get_default_spec_ctrl() |
> +			((tifn & _TIF_RDS) >> (TIF_RDS - SPEC_CTRL_RDS_SHIFT));

I said:

        u64 msr = x86_spec_ctrl_base | ((tifn & _TIF_RDS) << TIF_TO_RDS_SHIFT);

again on purpose. Because it makes no sense to have a function call to read
that variable. Performance matters at least to me.

> +
> +	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
> +}
> +
>  
> +int __weak arch_prctl_set_spec_ctrl(struct task_struct *p, unsigned long arg)
> +{
> +	return 0;

Yuck? So on any arch which does not implement that it returns success?
Really interesting logic.

I'm just tired of this sloppy hackery. I told you yesterday to take your
time and do it proper. But no, you just ignore most of my comments, come up
with even more convoluted crap and send the lot out in a hurry.

What's the purpose of this? That I waste another few hours reviewing it and
then you reply with 'will fix' and go off and do something completely
different.

No, this just sucks and we do not have another 2 weeks to play this
game.

Yours grumpy

      tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Linux Patch 1/1
  2018-04-28 18:15 ` [MODERATED] Re: [PATCH v3] " Jon Masters
@ 2018-04-28 21:20   ` Thomas Gleixner
  2018-04-28 22:25     ` Thomas Gleixner
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-28 21:20 UTC (permalink / raw)
  To: speck

On Sat, 28 Apr 2018, speck for Jon Masters wrote:

> On 04/28/2018 12:14 AM, speck for Tim Chen wrote:
> 
> > --- a/include/uapi/linux/prctl.h
> > +++ b/include/uapi/linux/prctl.h
> > @@ -207,4 +207,17 @@ struct prctl_mm_map {
> >  # define PR_SVE_VL_LEN_MASK		0xffff
> >  # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
> >  
> > +/*
> > + * Speculation Control Mode knobs
> > + *
> > + * Set this on a task to control speculation mode.
> > + * Initial use is to mitigate against speculative store bypass attack using
> > + * reduced data speculation mode.
> > + */
> > +#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
> > +#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
> > +#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
> > +
> > +#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */
> 
> Can we use a more generic term like "SSB" for the non-arch specific
> code? I expect at least Arm to also wire up the prctl shortly.

Sure. We long ago agreed on SSB for the generic parts, but why do you
expect that anyone @intel gives a shit?

Here are the defines I have in my version of this as I'm not going to waste
even a split second on reviewing that Intel mess once more:

#define PR_SET_SPEC_STORE_BYPASS_CTRL	52
#define PR_SPEC_STORE_BYPASS_NONE	0
#define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 0)
#define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 1)

#define PR_GET_SPEC_STORE_BYPASS_CTRL	52

NONE: 	    Not affected (returned by PR_GET_....) Cannot be set obviously
ENABLE:     SSB enabled
DISABLE:    SSB disabled

And then the whole return value semantics:

The weak functions return -EINVAL. No dicsussion about that, because that's
the standard return value for not implemented prctls.

get() returns one of the above values if implemented.

set() returns:

 -ENODEV - The prctl mode is not enabled
 -ERANGE - The prctl value is out of range, i,e. not ENABLE/DISABLE
 0	 - Success

That's the best I could come up with and we surely can bikeshed that to
death.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Linux Patch 1/1
  2018-04-28 21:20   ` Thomas Gleixner
@ 2018-04-28 22:25     ` Thomas Gleixner
  2018-04-28 23:18       ` Thomas Gleixner
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-28 22:25 UTC (permalink / raw)
  To: speck

On Sat, 28 Apr 2018, speck for Thomas Gleixner wrote:
> On Sat, 28 Apr 2018, speck for Jon Masters wrote:
> 
> > On 04/28/2018 12:14 AM, speck for Tim Chen wrote:
> > 
> > > --- a/include/uapi/linux/prctl.h
> > > +++ b/include/uapi/linux/prctl.h
> > > @@ -207,4 +207,17 @@ struct prctl_mm_map {
> > >  # define PR_SVE_VL_LEN_MASK		0xffff
> > >  # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
> > >  
> > > +/*
> > > + * Speculation Control Mode knobs
> > > + *
> > > + * Set this on a task to control speculation mode.
> > > + * Initial use is to mitigate against speculative store bypass attack using
> > > + * reduced data speculation mode.
> > > + */
> > > +#define PR_SET_SPEC_CTRL_MODE	52	/* set speculation control mode of process */
> > > +#define PR_SPEC_CTRL_MODE_NONE	0       /* no speculation control mode in use */
> > > +#define PR_SPEC_CTRL_MODE_RDS	(1 << 0)/* reduced data speculation mode */
> > > +
> > > +#define PR_GET_SPEC_CTRL_MODE	53	/* get speculation control mode of process */
> > 
> > Can we use a more generic term like "SSB" for the non-arch specific
> > code? I expect at least Arm to also wire up the prctl shortly.
> 
> Sure. We long ago agreed on SSB for the generic parts, but why do you
> expect that anyone @intel gives a shit?
> 
> Here are the defines I have in my version of this as I'm not going to waste
> even a split second on reviewing that Intel mess once more:
> 
> #define PR_SET_SPEC_STORE_BYPASS_CTRL	52
> #define PR_SPEC_STORE_BYPASS_NONE	0
> #define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 0)
> #define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 1)
> 
> #define PR_GET_SPEC_STORE_BYPASS_CTRL	52

Just thought about it a bit more. If the intention was to have eventually
more speculation related knobs for a task, then we surely can make it:

#define PR_SET_SPEC_CTRL		52
#define PR_SPEC_NOT_AFFECTED		0
#define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 0)
#define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 1)

#define PR_GET_SPEC_CTRL		53

That would make it simple to add new bit pairs where each bit reflects the
mitigation state on affected systems and if both bits are 0 then the system
is not affected by this particular issue. I fear we will need that sooner
than we want it.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Linux Patch 1/1
  2018-04-28 22:25     ` Thomas Gleixner
@ 2018-04-28 23:18       ` Thomas Gleixner
  2018-04-29  3:21         ` [MODERATED] " Jon Masters
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-28 23:18 UTC (permalink / raw)
  To: speck

On Sun, 29 Apr 2018, speck for Thomas Gleixner wrote:
> On Sat, 28 Apr 2018, speck for Thomas Gleixner wrote:
> > On Sat, 28 Apr 2018, speck for Jon Masters wrote:
> > > Can we use a more generic term like "SSB" for the non-arch specific
> > > code? I expect at least Arm to also wire up the prctl shortly.
> > 
> > Sure. We long ago agreed on SSB for the generic parts, but why do you
> > expect that anyone @intel gives a shit?
> > 
> > Here are the defines I have in my version of this as I'm not going to waste
> > even a split second on reviewing that Intel mess once more:
> > 
> > #define PR_SET_SPEC_STORE_BYPASS_CTRL	52
> > #define PR_SPEC_STORE_BYPASS_NONE	0
> > #define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 0)
> > #define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 1)
> > 
> > #define PR_GET_SPEC_STORE_BYPASS_CTRL	52
> 
> Just thought about it a bit more. If the intention was to have eventually
> more speculation related knobs for a task, then we surely can make it:
> 
> #define PR_SET_SPEC_CTRL		52
> #define PR_SPEC_NOT_AFFECTED		0
> #define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 0)
> #define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 1)
> 
> #define PR_GET_SPEC_CTRL		53
> 
> That would make it simple to add new bit pairs where each bit reflects the
> mitigation state on affected systems and if both bits are 0 then the system
> is not affected by this particular issue. I fear we will need that sooner
> than we want it.

Though having more than one vulnerabilty/mitigation controlled by a single
prctl has a downside. If the PRCTL mode for one is enabled and disabled for
the other then the -ENODEV (or whatever) error code is not possible.

There are two options:

  1) Use separate prctls per vulnerability/mitigation

  2) Add a third bit per vulnerabilty which reflects the controlability

     #define PR_SPEC_STORE_BYPASS_PRCTL		(1 << 0)
     #define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 1)
     #define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 2)

    The GET prctl would reflect the controlability in the XXX_PRCTL bit.

    If XXX_PRTCL is not set in the prctl(SET, value) then the particular
    vulnerabilty is ignored. If it is set, then the implementation checks
    whether that particular vulnerability is controlable. Still the error
    code is not distigushiable, but prctl(GET) allows to spot the fail in
    the value handed to prctl(SET) easily.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3] Linux Patch 1/1
  2018-04-28 23:18       ` Thomas Gleixner
@ 2018-04-29  3:21         ` Jon Masters
  2018-04-29  7:17           ` Thomas Gleixner
  0 siblings, 1 reply; 10+ messages in thread
From: Jon Masters @ 2018-04-29  3:21 UTC (permalink / raw)
  To: speck

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

Hi Thomas,

On 04/28/2018 07:18 PM, speck for Thomas Gleixner wrote:
> On Sun, 29 Apr 2018, speck for Thomas Gleixner wrote:
>> On Sat, 28 Apr 2018, speck for Thomas Gleixner wrote:
>>> On Sat, 28 Apr 2018, speck for Jon Masters wrote:
>>>> Can we use a more generic term like "SSB" for the non-arch specific
>>>> code? I expect at least Arm to also wire up the prctl shortly.
>>>
>>> Sure. We long ago agreed on SSB for the generic parts, but why do you
>>> expect that anyone @intel gives a shit?
>>>
>>> Here are the defines I have in my version of this as I'm not going to waste
>>> even a split second on reviewing that Intel mess once more:

First, thanks for this. It's great. To be fair to Tim and the Intel crew
I've been highly impressed with how they've tried to play nice with the
others when given feedback. I know he had a late night last night again.

> Though having more than one vulnerabilty/mitigation controlled by a single
> prctl has a downside. If the PRCTL mode for one is enabled and disabled for
> the other then the -ENODEV (or whatever) error code is not possible.
> 
> There are two options:
> 
>   1) Use separate prctls per vulnerability/mitigation
> 
>   2) Add a third bit per vulnerabilty which reflects the controlability
> 
>      #define PR_SPEC_STORE_BYPASS_PRCTL		(1 << 0)
>      #define PR_SPEC_STORE_BYPASS_ENABLE	(1 << 1)
>      #define PR_SPEC_STORE_BYPASS_DISABLE	(1 << 2)
> 
>     The GET prctl would reflect the controlability in the XXX_PRCTL bit.
> 
>     If XXX_PRTCL is not set in the prctl(SET, value) then the particular
>     vulnerabilty is ignored. If it is set, then the implementation checks
>     whether that particular vulnerability is controlable. Still the error
>     code is not distigushiable, but prctl(GET) allows to spot the fail in
>     the value handed to prctl(SET) easily.

I might be missing something here. Not to bikeshed too much to death but
when I first pondered this, I was thinking that prctl has multiple args
so we could simply have a top-level PR_SET_SPECULATION (or SPEC_CTRL),
and a PR_GET_SPECULATION (or whatever), then the second long argument
would be the specific thing. To make userspace easier to understand(?)

So you'd have:

prctl(PR_SET_SPECULATION, SPEC_STORE_BYPASS, SPEC_STORE_BYPASS_ENABLE)
prctl(PR_GET_SPECULATION, SPEC_STORE_BYPASS)

etc. There's up to 5 args permitted, so we just have the second level
one be for the type of speculation mitigation from this week and there's
still room for additional args if one of them needs something exotic.

But I'm probably being an idiot and I'm sorry.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Linux Patch 1/1
  2018-04-29  3:21         ` [MODERATED] " Jon Masters
@ 2018-04-29  7:17           ` Thomas Gleixner
  2018-04-29 16:22             ` [MODERATED] " Jon Masters
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-29  7:17 UTC (permalink / raw)
  To: speck

On Sat, 28 Apr 2018, speck for Jon Masters wrote:
> On 04/28/2018 07:18 PM, speck for Thomas Gleixner wrote:
> I might be missing something here. Not to bikeshed too much to death but
> when I first pondered this, I was thinking that prctl has multiple args
> so we could simply have a top-level PR_SET_SPECULATION (or SPEC_CTRL),
> and a PR_GET_SPECULATION (or whatever), then the second long argument
> would be the specific thing. To make userspace easier to understand(?)
> 
> So you'd have:
> 
> prctl(PR_SET_SPECULATION, SPEC_STORE_BYPASS, SPEC_STORE_BYPASS_ENABLE)
> prctl(PR_GET_SPECULATION, SPEC_STORE_BYPASS)
> 
> etc. There's up to 5 args permitted, so we just have the second level
> one be for the type of speculation mitigation from this week and there's
> still room for additional args if one of them needs something exotic.
> 
> But I'm probably being an idiot and I'm sorry.

Nah, that makes a lot of sense and I was just too tired to see the
obvious.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3] Linux Patch 1/1
  2018-04-29  7:17           ` Thomas Gleixner
@ 2018-04-29 16:22             ` Jon Masters
  0 siblings, 0 replies; 10+ messages in thread
From: Jon Masters @ 2018-04-29 16:22 UTC (permalink / raw)
  To: speck

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

On 04/29/2018 03:17 AM, speck for Thomas Gleixner wrote:
> On Sat, 28 Apr 2018, speck for Jon Masters wrote:
>> On 04/28/2018 07:18 PM, speck for Thomas Gleixner wrote:
>> I might be missing something here. Not to bikeshed too much to death but
>> when I first pondered this, I was thinking that prctl has multiple args
>> so we could simply have a top-level PR_SET_SPECULATION (or SPEC_CTRL),
>> and a PR_GET_SPECULATION (or whatever), then the second long argument
>> would be the specific thing. To make userspace easier to understand(?)
>>
>> So you'd have:
>>
>> prctl(PR_SET_SPECULATION, SPEC_STORE_BYPASS, SPEC_STORE_BYPASS_ENABLE)
>> prctl(PR_GET_SPECULATION, SPEC_STORE_BYPASS)
>>
>> etc. There's up to 5 args permitted, so we just have the second level
>> one be for the type of speculation mitigation from this week and there's
>> still room for additional args if one of them needs something exotic.
>>
>> But I'm probably being an idiot and I'm sorry.
> 
> Nah, that makes a lot of sense and I was just too tired to see the
> obvious.

;)

I'll wait to hear from you but will assume something like the above. If
you get held up testing, again happy to help along with Konrad. To get
the distro backports moving quickly I'm wanting to make sure we lock the
ABI down pretty much before tomorrow if possible. We're going to make
sure everything is done on our end and tested before end of this week.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-29 16:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-28  4:14 [MODERATED] [PATCH v3] Linux Patch 1/1 Tim Chen
2018-04-28 16:07 ` [MODERATED] [PATCH v4] " Tim Chen
2018-04-28 18:15 ` [MODERATED] Re: [PATCH v3] " Jon Masters
2018-04-28 21:20   ` Thomas Gleixner
2018-04-28 22:25     ` Thomas Gleixner
2018-04-28 23:18       ` Thomas Gleixner
2018-04-29  3:21         ` [MODERATED] " Jon Masters
2018-04-29  7:17           ` Thomas Gleixner
2018-04-29 16:22             ` [MODERATED] " Jon Masters
2018-04-28 19:20 ` Thomas Gleixner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.