[PATCH v5 0/2] Harden spectrev2 userspace-userspace protection

[PATCH v5 0/2] Harden spectrev2 userspace-userspace protection

[Jiri Kosina]

Currently, linux kernel is basically not preventing userspace-userspace 
spectrev2 attack, because:

- IBPB is basically unused (issued only for tasks that marked themselves 
  explicitly non-dumpable, which is absolutely negligible minority of all 
  software out there), therefore cross-process branch buffer posioning 
  using spectrev2 is possible

- STIBP is completely unused, therefore cross-process branch buffer 
  poisoning using spectrev2 between processess running on two HT siblings 
  thread s is possible

This patchset changes IBPB semantics, so that it's now applied whenever 
context-switching between processess that can't use ptrace() to achieve 
the same. This admittedly comes with extra overhad on a context switch; 
systems that don't care about could disable the mitigation using 
nospectre_v2 boot option.
The IBPB implementaion is heavily based on original patches by Tim Chen.

In addition to that, we unconditionally turn STIBP on so that HT siblings 
always have separate branch buffers.

We've been carrying IBPB implementation with the same semantics in our 
(SUSE) trees since january disclosure; STIBP was more or less ignored up 
to today.

v1->v2:
        include IBPB changes
v2->v3: 
        fix IBPB 'who can trace who' semantics
        wire up STIBP flipping to SMT hotplug
v3->v4:
	dropped ___ptrace_may_access(), as it's not needed
	fixed deadlock with LSM/audit/selinux (Andrea Arcangeli)
	statically patch out the ptrace check if !IBPB

v4->v5:
	fix MSR writing logic (Thomas Gleixner, Josh Poimboeuf)

Jiri Kosina (2):
      x86/speculation: apply IBPB more strictly to avoid cross-process data leak
      x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

 arch/x86/kernel/cpu/bugs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 arch/x86/mm/tlb.c          | 31 ++++++++++++++++++++-----------
 include/linux/ptrace.h     |  4 ++++
 kernel/cpu.c               | 11 ++++++++++-
 kernel/ptrace.c            | 12 ++++++++----
 5 files changed, 86 insertions(+), 21 deletions(-)

-- 
Jiri Kosina
SUSE Labs

[PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Currently, we are issuing IBPB only in cases when switching into a non-dumpable
process, the rationale being to protect such 'important and security sensitive'
processess (such as GPG) from data leak into a different userspace process via
spectre v2.

This is however completely insufficient to provide proper userspace-to-userpace
spectrev2 protection, as any process can poison branch buffers before being
scheduled out, and the newly scheduled process immediately becomes spectrev2
victim.

In order to minimize the performance impact (for usecases that do require
spectrev2 protection), issue the barrier only in cases when switching between
processess where the victim can't be ptraced by the potential attacker (as in
such cases, the attacker doesn't have to bother with branch buffers at all).

Fixes: 18bf3c3ea8 ("x86/speculation: Use Indirect Branch Prediction Barrier in context switch")
Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/mm/tlb.c      | 31 ++++++++++++++++++++-----------
 include/linux/ptrace.h |  4 ++++
 kernel/ptrace.c        | 12 ++++++++----
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e96b99eb800c..ed4444402441 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
+#include <linux/ptrace.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(struct mm_struct *mm)
 	}
 }
 
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+	/*
+	 * Check if the current (previous) task has access to the memory
+	 * of the @tsk (next) task. If access is denied, make sure to
+	 * issue a IBPB to stop user->user Spectre-v2 attacks.
+	 *
+	 * Note: __ptrace_may_access() returns 0 or -ERRNO.
+	 */
+	return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+			__ptrace_may_access(tsk, PTRACE_MODE_IBPB));
+}
+
 void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			struct task_struct *tsk)
 {
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		 * one process from doing Spectre-v2 attacks on another.
 		 *
 		 * As an optimization, flush indirect branches only when
-		 * switching into processes that disable dumping. This
-		 * protects high value processes like gpg, without having
-		 * too high performance overhead. IBPB is *expensive*!
-		 *
-		 * This will not flush branches when switching into kernel
-		 * threads. It will also not flush if we switch to idle
-		 * thread and back to the same process. It will flush if we
-		 * switch to a different non-dumpable process.
+		 * switching into a processes that can't be ptrace by the
+		 * current one (as in such case, attacker has much more
+		 * convenient way how to tamper with the next process than
+		 * branch buffer poisoning).
 		 */
-		if (tsk && tsk->mm &&
-		    tsk->mm->context.ctx_id != last_ctx_id &&
-		    get_dumpable(tsk->mm) != SUID_DUMP_USER)
+		if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+				ibpb_needed(tsk, last_ctx_id))
 			indirect_branch_prediction_barrier();
 
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 4f36431c380b..983d3f5545a8 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -64,12 +64,15 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
 #define PTRACE_MODE_NOAUDIT	0x04
 #define PTRACE_MODE_FSCREDS 0x08
 #define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_NOACCESS_CHK 0x20
 
 /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
 #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
 #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_IBPB (PTRACE_MODE_ATTACH | PTRACE_MODE_NOAUDIT \
+			  | PTRACE_MODE_NOACCESS_CHK | PTRACE_MODE_REALCREDS)
 
 /**
  * ptrace_may_access - check whether the caller is permitted to access
@@ -86,6 +89,7 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
  * process_vm_writev or ptrace (and should use the real credentials).
  */
 extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+extern int __ptrace_may_access(struct task_struct *task, unsigned int mode);
 
 static inline int ptrace_reparented(struct task_struct *child)
 {
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 21fec73d45d4..5c5e7cb597cd 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -268,7 +268,7 @@ static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
 }
 
 /* Returns 0 on success, -errno on denial. */
-static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
+int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
 	const struct cred *cred = current_cred(), *tcred;
 	struct mm_struct *mm;
@@ -316,7 +316,8 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 	    gid_eq(caller_gid, tcred->sgid) &&
 	    gid_eq(caller_gid, tcred->gid))
 		goto ok;
-	if (ptrace_has_cap(tcred->user_ns, mode))
+	if (!(mode & PTRACE_MODE_NOACCESS_CHK) &&
+	     ptrace_has_cap(tcred->user_ns, mode))
 		goto ok;
 	rcu_read_unlock();
 	return -EPERM;
@@ -325,10 +326,13 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 	mm = task->mm;
 	if (mm &&
 	    ((get_dumpable(mm) != SUID_DUMP_USER) &&
-	     !ptrace_has_cap(mm->user_ns, mode)))
+	     ((mode & PTRACE_MODE_NOACCESS_CHK) ||
+	       !ptrace_has_cap(mm->user_ns, mode))))
 	    return -EPERM;
 
-	return security_ptrace_access_check(task, mode);
+	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
+		return security_ptrace_access_check(task, mode);
+	return 0;
 }
 
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)

-- 
Jiri Kosina
SUSE Labs

[PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

STIBP is a feature provided by certain Intel ucodes / CPUs. This feature
(once enabled) prevents cross-hyperthread control of decisions made by
indirect branch predictors.

Enable this feature if

- the CPU is vulnerable to spectre v2
- the CPU supports SMT and has SMT siblings online
- spectre_v2 mitigation autoselection is enabled (default)

After some previous discussion, this patch leaves STIBP on all the time,
as wrmsr on crossing kernel boundary is a no-no. This could perhaps later
be a bit more optimized (like disabling it in NOHZ, experiment with
disabling it in idle, etc) if needed.

Note that the synchronization of the mask manipulation via newly added
spec_ctrl_mutex is currently not strictly needed, as the only updater is
already being serialized by cpu_add_remove_lock, but let's make this a
little bit more future-proof.

Cc: stable@vger.kernel.org
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/kernel/cpu/bugs.c | 49 +++++++++++++++++++++++++++++++++++++++++-----
 kernel/cpu.c               | 11 ++++++++++-
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 40bdaea97fe7..ccc5e6b7fe40 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -35,12 +35,10 @@ static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
 static void __init l1tf_select_mitigation(void);
 
-/*
- * Our boot-time value of the SPEC_CTRL MSR. We read it once so that any
- * writes to SPEC_CTRL contain whatever reserved bits have been set.
- */
-u64 __ro_after_init x86_spec_ctrl_base;
+/* The base value of the SPEC_CTRL MSR that always has to be preserved. */
+u64 x86_spec_ctrl_base;
 EXPORT_SYMBOL_GPL(x86_spec_ctrl_base);
+static DEFINE_MUTEX(spec_ctrl_mutex);
 
 /*
  * The vendor and possibly platform specific bits which can be modified in
@@ -325,6 +323,44 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
 	return cmd;
 }
 
+static bool stibp_needed(void)
+{
+	if (spectre_v2_enabled == SPECTRE_V2_NONE)
+		return false;
+
+	if (!boot_cpu_has(X86_FEATURE_STIBP))
+		return false;
+
+	return true;
+}
+
+static void update_stibp_msr(void *info)
+{
+	wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
+void arch_smt_update(void)
+{
+	if (stibp_needed()) {
+		u64 mask;
+		mutex_lock(&spec_ctrl_mutex);
+		mask = x86_spec_ctrl_base;
+		if (cpu_smt_control == CPU_SMT_ENABLED)
+			mask |= SPEC_CTRL_STIBP;
+		else
+			mask &= ~SPEC_CTRL_STIBP;
+
+		if (mask != x86_spec_ctrl_base) {
+			pr_info("Spectre v2 cross-process SMT mitigation: %s STIBP\n",
+					cpu_smt_control == CPU_SMT_ENABLED ?
+						"Enabling" : "Disabling");
+			x86_spec_ctrl_base = mask;
+			on_each_cpu(update_stibp_msr, NULL, 1);
+		}
+		mutex_unlock(&spec_ctrl_mutex);
+	}
+}
+
 static void __init spectre_v2_select_mitigation(void)
 {
 	enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
@@ -424,6 +460,9 @@ static void __init spectre_v2_select_mitigation(void)
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
+
+	/* Enable STIBP if appropriate */
+	arch_smt_update();
 }
 
 #undef pr_fmt
diff --git a/kernel/cpu.c b/kernel/cpu.c
index aa7fe85ad62e..4bba5071d61e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2025,6 +2025,12 @@ static void cpuhp_online_cpu_device(unsigned int cpu)
 	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
 }
 
+/*
+ * Architectures that need SMT-specific errata handling during SMT hotplug
+ * should override these.
+ */
+void __weak arch_smt_update(void) { };
+
 static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
 {
 	int cpu, ret = 0;
@@ -2051,8 +2057,10 @@ static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
 		 */
 		cpuhp_offline_cpu_device(cpu);
 	}
-	if (!ret)
+	if (!ret) {
 		cpu_smt_control = ctrlval;
+		arch_smt_update();
+	}
 	cpu_maps_update_done();
 	return ret;
 }
@@ -2063,6 +2071,7 @@ static int cpuhp_smt_enable(void)
 
 	cpu_maps_update_begin();
 	cpu_smt_control = CPU_SMT_ENABLED;
+	arch_smt_update();
 	for_each_present_cpu(cpu) {
 		/* Skip online CPUs and CPUs on offline nodes */
 		if (cpu_online(cpu) || !node_online(cpu_to_node(cpu)))

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Thomas Gleixner]

On Mon, 10 Sep 2018, Jiri Kosina wrote:
> +static void update_stibp_msr(void *info)
> +{
> +	wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> +}
> +
> +void arch_smt_update(void)
> +{
> +	if (stibp_needed()) {

  	if (!stib_needed())
		return;

spares you an indentation level.

> +		u64 mask;

Newline between declarations and code please.

> +		mutex_lock(&spec_ctrl_mutex);
> +		mask = x86_spec_ctrl_base;
> +		if (cpu_smt_control == CPU_SMT_ENABLED)
> +			mask |= SPEC_CTRL_STIBP;
> +		else
> +			mask &= ~SPEC_CTRL_STIBP;
> +
> +		if (mask != x86_spec_ctrl_base) {
> +			pr_info("Spectre v2 cross-process SMT mitigation: %s STIBP\n",
> +					cpu_smt_control == CPU_SMT_ENABLED ?
> +						"Enabling" : "Disabling");
> +			x86_spec_ctrl_base = mask;
> +			on_each_cpu(update_stibp_msr, NULL, 1);
> +		}
> +		mutex_unlock(&spec_ctrl_mutex);
> +	}
> +}
> +

That looks much more palatable. One missing piece is the sysfs mitigation
file for spectre v2. That should reflect STIPB state as well.

Thanks,

	tglx

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

On Mon, 10 Sep 2018, Thomas Gleixner wrote:

> That looks much more palatable. One missing piece is the sysfs 
> mitigation file for spectre v2. That should reflect STIPB state as well.

FWIW, we're missing a bit more in that area, namely RSB stuffing on 
context switch, IBRS (even through only around fw) and IBPB; those are 
only signalled in dmesg during bootup.

Want to add all this stuff there?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

On Mon, 10 Sep 2018, Jiri Kosina wrote:

> > That looks much more palatable. One missing piece is the sysfs 
> > mitigation file for spectre v2. That should reflect STIPB state as well.
> 
> FWIW, we're missing a bit more in that area, namely RSB stuffing on 
> context switch, IBRS (even through only around fw) and IBPB; those are 
> only signalled in dmesg during bootup.

Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
the missing reporting of RSB_CTXSW for v6.

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Monday, September 10, 2018 2:24 AM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> Tim Chen <tim.c.chen@linux.intel.com>; Schaufler, Casey
> <casey.schaufler@intel.com>
> Cc: linux-kernel@vger.kernel.org; x86@kernel.org
> Subject: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid
> cross-process data leak
> 
> From: Jiri Kosina <jkosina@suse.cz>
> 
> Currently, we are issuing IBPB only in cases when switching into a non-
> dumpable
> process, the rationale being to protect such 'important and security sensitive'
> processess (such as GPG) from data leak into a different userspace process via
> spectre v2.
> 
> This is however completely insufficient to provide proper userspace-to-
> userpace
> spectrev2 protection, as any process can poison branch buffers before being
> scheduled out, and the newly scheduled process immediately becomes
> spectrev2
> victim.
> 
> In order to minimize the performance impact (for usecases that do require
> spectrev2 protection), issue the barrier only in cases when switching between
> processess where the victim can't be ptraced by the potential attacker (as in
> such cases, the attacker doesn't have to bother with branch buffers at all).
> 
> Fixes: 18bf3c3ea8 ("x86/speculation: Use Indirect Branch Prediction Barrier in
> context switch")
> Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  arch/x86/mm/tlb.c      | 31 ++++++++++++++++++++-----------
>  include/linux/ptrace.h |  4 ++++
>  kernel/ptrace.c        | 12 ++++++++----
>  3 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index e96b99eb800c..ed4444402441 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -7,6 +7,7 @@
>  #include <linux/export.h>
>  #include <linux/cpu.h>
>  #include <linux/debugfs.h>
> +#include <linux/ptrace.h>
> 
>  #include <asm/tlbflush.h>
>  #include <asm/mmu_context.h>
> @@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(struct
> mm_struct *mm)
>  	}
>  }
> 
> +static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
> +{
> +	/*
> +	 * Check if the current (previous) task has access to the memory
> +	 * of the @tsk (next) task. If access is denied, make sure to
> +	 * issue a IBPB to stop user->user Spectre-v2 attacks.
> +	 *
> +	 * Note: __ptrace_may_access() returns 0 or -ERRNO.
> +	 */
> +	return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
> +			__ptrace_may_access(tsk, PTRACE_MODE_IBPB));
> +}
> +
>  void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  			struct task_struct *tsk)
>  {
> @@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct *prev,
> struct mm_struct *next,
>  		 * one process from doing Spectre-v2 attacks on another.
>  		 *
>  		 * As an optimization, flush indirect branches only when
> -		 * switching into processes that disable dumping. This
> -		 * protects high value processes like gpg, without having
> -		 * too high performance overhead. IBPB is *expensive*!
> -		 *
> -		 * This will not flush branches when switching into kernel
> -		 * threads. It will also not flush if we switch to idle
> -		 * thread and back to the same process. It will flush if we
> -		 * switch to a different non-dumpable process.
> +		 * switching into a processes that can't be ptrace by the
> +		 * current one (as in such case, attacker has much more
> +		 * convenient way how to tamper with the next process than
> +		 * branch buffer poisoning).
>  		 */
> -		if (tsk && tsk->mm &&
> -		    tsk->mm->context.ctx_id != last_ctx_id &&
> -		    get_dumpable(tsk->mm) != SUID_DUMP_USER)
> +		if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
> +				ibpb_needed(tsk, last_ctx_id))
>  			indirect_branch_prediction_barrier();
> 
>  		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 4f36431c380b..983d3f5545a8 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -64,12 +64,15 @@ extern void exit_ptrace(struct task_struct *tracer,
> struct list_head *dead);
>  #define PTRACE_MODE_NOAUDIT	0x04
>  #define PTRACE_MODE_FSCREDS 0x08
>  #define PTRACE_MODE_REALCREDS 0x10
> +#define PTRACE_MODE_NOACCESS_CHK 0x20
> 
>  /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
>  #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ |
> PTRACE_MODE_FSCREDS)
>  #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ |
> PTRACE_MODE_REALCREDS)
>  #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH |
> PTRACE_MODE_FSCREDS)
>  #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH |
> PTRACE_MODE_REALCREDS)
> +#define PTRACE_MODE_IBPB (PTRACE_MODE_ATTACH |
> PTRACE_MODE_NOAUDIT \
> +			  | PTRACE_MODE_NOACCESS_CHK |
> PTRACE_MODE_REALCREDS)
> 
>  /**
>   * ptrace_may_access - check whether the caller is permitted to access
> @@ -86,6 +89,7 @@ extern void exit_ptrace(struct task_struct *tracer, struct
> list_head *dead);
>   * process_vm_writev or ptrace (and should use the real credentials).
>   */
>  extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
> +extern int __ptrace_may_access(struct task_struct *task, unsigned int mode);
> 
>  static inline int ptrace_reparented(struct task_struct *child)
>  {
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 21fec73d45d4..5c5e7cb597cd 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -268,7 +268,7 @@ static int ptrace_has_cap(struct user_namespace *ns,
> unsigned int mode)
>  }
> 
>  /* Returns 0 on success, -errno on denial. */
> -static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  {
>  	const struct cred *cred = current_cred(), *tcred;
>  	struct mm_struct *mm;
> @@ -316,7 +316,8 @@ static int __ptrace_may_access(struct task_struct
> *task, unsigned int mode)
>  	    gid_eq(caller_gid, tcred->sgid) &&
>  	    gid_eq(caller_gid, tcred->gid))
>  		goto ok;
> -	if (ptrace_has_cap(tcred->user_ns, mode))
> +	if (!(mode & PTRACE_MODE_NOACCESS_CHK) &&
> +	     ptrace_has_cap(tcred->user_ns, mode))
>  		goto ok;
>  	rcu_read_unlock();
>  	return -EPERM;
> @@ -325,10 +326,13 @@ static int __ptrace_may_access(struct task_struct
> *task, unsigned int mode)
>  	mm = task->mm;
>  	if (mm &&
>  	    ((get_dumpable(mm) != SUID_DUMP_USER) &&
> -	     !ptrace_has_cap(mm->user_ns, mode)))
> +	     ((mode & PTRACE_MODE_NOACCESS_CHK) ||
> +	       !ptrace_has_cap(mm->user_ns, mode))))
>  	    return -EPERM;
> 
> -	return security_ptrace_access_check(task, mode);
> +	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> +		return security_ptrace_access_check(task, mode);

Why are you dropping the LSM check here, when in v4 you fixed the
SELinux audit locking issue? We can avoid introducing an LSM hook
and all the baggage around it if you can do the security_ptrace_access_check()
here.


> +	return 0;
>  }
> 
>  bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> 
> --
> Jiri Kosina
> SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Mon, 10 Sep 2018, Schaufler, Casey wrote:

> Why are you dropping the LSM check here, when in v4 you fixed the
> SELinux audit locking issue? We can avoid introducing an LSM hook
> and all the baggage around it if you can do the security_ptrace_access_check()
> here.

So what guarantees that none of the hooks that 
security_ptrace_access_check() is invoking will not be taking locks (from 
scheduler context in this case)?

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Monday, September 10, 2018 12:14 PM
> To: Schaufler, Casey <casey.schaufler@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> Tim Chen <tim.c.chen@linux.intel.com>; linux-kernel@vger.kernel.org;
> x86@kernel.org
> Subject: RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid
> cross-process data leak
> 
> On Mon, 10 Sep 2018, Schaufler, Casey wrote:
> 
> > Why are you dropping the LSM check here, when in v4 you fixed the
> > SELinux audit locking issue? We can avoid introducing an LSM hook
> > and all the baggage around it if you can do the
> security_ptrace_access_check()
> > here.
> 
> So what guarantees that none of the hooks that
> security_ptrace_access_check() is invoking will not be taking locks (from
> scheduler context in this case)?

The locking issue in the security modules is the same regardless of
whether the call of security_ptrace_access_check() comes from the
__ptrace_access_check() you're adding here or from a new security
hook (I have proposed security_task_safe_sidechannel) that gets added
in the same place later on. Adding a new hook results in duplication,
because there now has to be code that does exactly the same thing as
__ptrace_access_check() but without the new NOACCESS_CHECK mode.

Yes, It would require that this patch be tested against all the existing
security modules that provide a ptrace_access_check hook. It's not like
the security module writers don't have a bunch of locking issues to deal with. 

> Thanks,
> 
> --
> Jiri Kosina
> SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Mon, 10 Sep 2018, Schaufler, Casey wrote:

> Yes, It would require that this patch be tested against all the existing 
> security modules that provide a ptrace_access_check hook. It's not like 
> the security module writers don't have a bunch of locking issues to deal 
> with.

Yeah, that was indeed my concern.

So can we agree on doing this in the 2nd envisioned step, when this is 
going to be replaced by LSM as discussed [1] previously?

[1] http://lkml.kernel.org/r/99FC4B6EFCEFD44486C35F4C281DC67321447094@ORSMSX107.amr.corp.intel.com

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Monday, September 10, 2018 12:36 PM
> To: Schaufler, Casey <casey.schaufler@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> Tim Chen <tim.c.chen@linux.intel.com>; linux-kernel@vger.kernel.org;
> x86@kernel.org
> Subject: RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid
> cross-process data leak
> 
> On Mon, 10 Sep 2018, Schaufler, Casey wrote:
> 
> > Yes, It would require that this patch be tested against all the existing
> > security modules that provide a ptrace_access_check hook. It's not like
> > the security module writers don't have a bunch of locking issues to deal
> > with.
> 
> Yeah, that was indeed my concern.
> 
> So can we agree on doing this in the 2nd envisioned step, when this is
> going to be replaced by LSM as discussed [1] previously?

It you're going to call __ptrace_access_check(), which already includes
an LSM hook, it makes a whole lot of sense to make that the path for doing
any module specific checks. It seems wrong to disable the LSM hook there,
then turn around and introduce a new one that does the check you just
disabled. The patches I had proposed created a new LSM hook because there
was not path to an existing hook. With your addition of __ptrace_access_check()
that is no longer an issue once any locking problems are resolved. Rather than
use a new hook, the existing ptrace hooks ought to work just fine, and any new
checks can be added in a new module that has its own ptrace_access_check hook.

> [1]
> http://lkml.kernel.org/r/99FC4B6EFCEFD44486C35F4C281DC67321447094@OR
> SMSX107.amr.corp.intel.com
> 
> Thanks,
> 
> --
> Jiri Kosina
> SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Mon, 10 Sep 2018, Schaufler, Casey wrote:

> It you're going to call __ptrace_access_check(), 

I guess you mean __ptrace_may_access() here.

> which already includes an LSM hook, it makes a whole lot of sense to 
> make that the path for doing any module specific checks. It seems wrong 
> to disable the LSM hook there, then turn around and introduce a new one 
> that does the check you just disabled. The patches I had proposed 
> created a new LSM hook because there was not path to an existing hook. 
> With your addition of __ptrace_access_check() that is no longer an issue 
> once any locking problems are resolved. Rather than use a new hook, the 
> existing ptrace hooks ought to work just fine, and any new checks can be 
> added in a new module that has its own ptrace_access_check hook.

Sorry for being dense, but what exactly are you proposing here then?

This patch (v4 and v5) explicitly avoids calling out to ptrace_has_cap() 
(which calls out to LSM through has_ns_capability_*() -> 
security_capable()) in PTRACE_MODE_IBPB case, exactly to avoid locking 
issues with security modules; there are known callchains that lead to 
deadlock.

With the same reasoning, security_ptrace_access_check() call is avoided, 
only there is no know particular callchain that'd lead to a lock being 
taken, but noone has done such audit (yet), as it's all hidden behind LSM 
callbacks.

So please tell me what exactly you'd like to see changed in the IBPB patch 
and why exactly, I am not seeing it yet.

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Monday, September 10, 2018 1:42 PM
> To: Schaufler, Casey <casey.schaufler@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> Tim Chen <tim.c.chen@linux.intel.com>; linux-kernel@vger.kernel.org;
> x86@kernel.org
> Subject: RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid
> cross-process data leak
> 
> On Mon, 10 Sep 2018, Schaufler, Casey wrote:
> 
> > It you're going to call __ptrace_access_check(),
> 
> I guess you mean __ptrace_may_access() here.

Sorry, yes. Too much code, too little brain.

> > which already includes an LSM hook, it makes a whole lot of sense to
> > make that the path for doing any module specific checks. It seems wrong
> > to disable the LSM hook there, then turn around and introduce a new one
> > that does the check you just disabled. The patches I had proposed
> > created a new LSM hook because there was not path to an existing hook.
> > With your addition of __ptrace_access_check() that is no longer an issue
> > once any locking problems are resolved. Rather than use a new hook, the
> > existing ptrace hooks ought to work just fine, and any new checks can be
> > added in a new module that has its own ptrace_access_check hook.
> 
> Sorry for being dense, but what exactly are you proposing here then?

Sorry for not being clear.

You added a call to __ptrace_may_access(). The security modules that get
called from __ptrace_may_access() via security_ptrace_access_check()
have known and/or suspected locking issues. So far so good. ...

> This patch (v4 and v5) explicitly avoids calling out to ptrace_has_cap()
> (which calls out to LSM through has_ns_capability_*() ->
> security_capable()) in PTRACE_MODE_IBPB case, exactly to avoid locking
> issues with security modules; there are known callchains that lead to
> deadlock.

So rather than avoiding calling these, why not avoid doing the things inside
the module specific code that cause the locking issues? Move the checks you
put in the ptrace code into the security module hooks.  I can't say 100%, but from
what I've seen so far, it's locking in the audit sub-system that causes the issues,
and that is pretty easy to work around.

> With the same reasoning, security_ptrace_access_check() call is avoided,
> only there is no know particular callchain that'd lead to a lock being
> taken, but noone has done such audit (yet), as it's all hidden behind LSM
> callbacks.

I've had a pretty good look, and there's nothing magic about the LSM callbacks.

> So please tell me what exactly you'd like to see changed in the IBPB patch
> and why exactly, I am not seeing it yet.

Short of a patch to show the changes (which I wish I could do today, but really can't)
what I want to see is:

	- Put ptrace back to using the security module interfaces.
	- Identify where this causes locking issues and work with the module
	  owners (a reasonable lot, all) to provide lock safe paths for the IBPB case.

Otherwise, I have to add a new LSM hook right after your ptrace call and duplicate
a whole lot of what you've just turned off, plus creating lock safe code that duplicates
what ptrace already does. While I would rather have the side-channel checks be
separate from the ptrace checks I can't justify doing both.

I'm sorry if that isn't clearer. I am trying.

> 
> Thanks,
> 
> --
> Jiri Kosina
> SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Mon, 10 Sep 2018, Schaufler, Casey wrote:

> > So please tell me what exactly you'd like to see changed in the IBPB patch
> > and why exactly, I am not seeing it yet.
> 
> Short of a patch to show the changes (which I wish I could do today, but really can't)
> what I want to see is:
> 
> 	- Put ptrace back to using the security module interfaces.
> 	- Identify where this causes locking issues and work with the module
> 	  owners (a reasonable lot, all) to provide lock safe paths for the IBPB case.
> 
> Otherwise, I have to add a new LSM hook right after your ptrace call and duplicate
> a whole lot of what you've just turned off, plus creating lock safe code that duplicates
> what ptrace already does. While I would rather have the side-channel checks be
> separate from the ptrace checks I can't justify doing both.

So why can't this be then done as 2nd step, once you've audited the LSM 
callbacks and worked around the locking in LSM callbacks/audit code?

Once that is taken care of, of course feel free to undo the changes my 
patch is doing so that you don't have to duplicate any ptrace code.

But before all that is fixed / worked around in LSM/audit (and I don't 
have spare cycles for doing that myself), why not take the simple aproach 
for now?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Tim Chen]

On 09/10/2018 04:46 AM, Jiri Kosina wrote:
> On Mon, 10 Sep 2018, Jiri Kosina wrote:
> 
>>> That looks much more palatable. One missing piece is the sysfs 
>>> mitigation file for spectre v2. That should reflect STIPB state as well.
>>
>> FWIW, we're missing a bit more in that area, namely RSB stuffing on 
>> context switch, IBRS (even through only around fw) and IBPB; those are 
>> only signalled in dmesg during bootup.
> 
> Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
> the missing reporting of RSB_CTXSW for v6.
> 

I anticipate that STIBP could affect workloads with a lot of indirect
branches (see previous discussion with Andrea).  We should have a 
knob for people to opt in or opt out of STIBP.

Thanks.

Tim

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Thomas Gleixner]

On Mon, 10 Sep 2018, Schaufler, Casey wrote:
> > -----Original Message-----
> > From: Jiri Kosina [mailto:jikos@kernel.org]
> > Sent: Monday, September 10, 2018 1:42 PM
> > To: Schaufler, Casey <casey.schaufler@intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> > Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> > <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> > Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> > Tim Chen <tim.c.chen@linux.intel.com>; linux-kernel@vger.kernel.org;
> > x86@kernel.org

Casey, can you please spare us the completely redundant copy of the mail
header?

> Short of a patch to show the changes (which I wish I could do today, but
> really can't) what I want to see is:
>
> 	- Put ptrace back to using the security module interfaces.
> 	- Identify where this causes locking issues and work with the module
> 	  owners (a reasonable lot, all) to provide lock safe paths for the IBPB case.
> 
> Otherwise, I have to add a new LSM hook right after your ptrace call and
> duplicate a whole lot of what you've just turned off, plus creating lock
> safe code that duplicates what ptrace already does. While I would rather
> have the side-channel checks be separate from the ptrace checks I can't
> justify doing both.

I'm not yet convinced that making these decisions purely based on LSM is a
good idea. The LSM based decision can optionally replace the built in
default decision at runtime, but it cannot replace it because its simpler
for most people to install a new kernel than fiddling with LSM.

We want a halfways sane default solution for this ASAP and Jiri's patch is
straight forward providing one without preventing you from adding your
magic LSM stuff once you have time to work on it including all (locking)
problems it creates. Nice try to offload that to Jiri.

Thanks,

	tglx

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Thomas Gleixner]

On Tue, 11 Sep 2018, Tim Chen wrote:
> On 09/10/2018 04:46 AM, Jiri Kosina wrote:
> > Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
> > the missing reporting of RSB_CTXSW for v6.
> > 
> 
> I anticipate that STIBP could affect workloads with a lot of indirect
> branches (see previous discussion with Andrea).  We should have a 
> knob for people to opt in or opt out of STIBP.

Feel free to send a patch to that effect.

Thanks,

	tglx

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Thomas Gleixner]

On Tue, 11 Sep 2018, Thomas Gleixner wrote:

> On Tue, 11 Sep 2018, Tim Chen wrote:
> > On 09/10/2018 04:46 AM, Jiri Kosina wrote:
> > > Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
> > > the missing reporting of RSB_CTXSW for v6.
> > > 
> > 
> > I anticipate that STIBP could affect workloads with a lot of indirect
> > branches (see previous discussion with Andrea).  We should have a 
> > knob for people to opt in or opt out of STIBP.
> 
> Feel free to send a patch to that effect.

Along with a proper explanation why it makes sense to disable this
particular piece separate from the rest of the V2 mitigations. You surely
can come up with a rationale based on facts rather than on anticipations.

Thanks,

	tglx

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
...
> 
> Casey, can you please spare us the completely redundant copy of the mail
> header?

Sorry to be a bother.

> > Short of a patch to show the changes (which I wish I could do today, but
> > really can't) what I want to see is:
> >
> > 	- Put ptrace back to using the security module interfaces.
> > 	- Identify where this causes locking issues and work with the module
> > 	  owners (a reasonable lot, all) to provide lock safe paths for the IBPB
> case.
> >
> > Otherwise, I have to add a new LSM hook right after your ptrace call and
> > duplicate a whole lot of what you've just turned off, plus creating lock
> > safe code that duplicates what ptrace already does. While I would rather
> > have the side-channel checks be separate from the ptrace checks I can't
> > justify doing both.
> 
> I'm not yet convinced that making these decisions purely based on LSM is a
> good idea.

The current patch-under-discussion uses the ptrace access policy as the
criteria for deciding that side-channel attacks are worrisome. This patch
excludes the LSM checks that are usually called by the ptrace code. If it
retained the LSM checks the need to do anything special in an LSM would
be limited to things that the ptrace checks don't already do.

> The LSM based decision can optionally replace the built in
> default decision at runtime,

LSM modules can add restrictions, but do not replace existing decisions.

> but it cannot replace it because its simpler
> for most people to install a new kernel than fiddling with LSM.

Um, installing a new kernel is the usual way to fiddle with LSM.
It's done by configuration options, so I don't see the distinction you're making.

> We want a halfways sane default solution for this ASAP and Jiri's patch is
> straight forward providing one without preventing you from adding your
> magic LSM stuff once you have time to work on it including all (locking)
> problems it creates.

There's actually one locking issue in the SELinux ptrace hook, and that's
due to auditing. It's easy to fix. I did it in the side channel LSM that I proposed.
There's nothing magic about the "LSM stuff", especially compared to what goes
on in ptrace.

> Nice try to offload that to Jiri.

Oh, come off it. If ptrace is the right way to do this check, that's great.
What I'm saying is that rather than turning off the LSM checks in ptrace
because it will take some work to get it right for the environment it gets
called from the right thing to do is to fix the LSM code.

How about this? Take Jiri's patch as written. You get everything except checks
on the security blobs and any "magic" that my safesidechannel module did. I
will propose a follow on patch that fixes the SELinux code to eliminate the locking
issue and enables the LSM hooks in the IBPB case. I can then do a revised "magic"
safesidechannel security module that uses the ptrace hook instead of adding a
new hook explicitly for IBPB. There is some danger that in the future ptrace and
IBPB criteria will diverge sufficiently that a common hook becomes nonsensical.
As no one else seems concerned about this possibility, I won't lose any sleep over
it either.

> Thanks,
> 
> 	tglx

[PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Jiri Kosina]

Currently, linux kernel is basically not preventing userspace-userspace 
spectrev2 attack, because:

- IBPB is basically unused (issued only for tasks that marked themselves 
  explicitly non-dumpable, which is absolutely negligible minority of all 
  software out there), therefore cross-process branch buffer posioning 
  using spectrev2 is possible

- STIBP is completely unused, therefore cross-process branch buffer 
  poisoning using spectrev2 between processess running on two HT siblings 
  thread s is possible

This patchset changes IBPB semantics, so that it's now applied whenever 
context-switching between processess that can't use ptrace() to achieve 
the same. This admittedly comes with extra overhad on a context switch; 
systems that don't care about could disable the mitigation using 
nospectre_v2 boot option.
The IBPB implementaion is heavily based on original patches by Tim Chen.

In addition to that, we unconditionally turn STIBP on so that HT siblings 
always have separate branch buffers.

We've been carrying IBPB implementation with the same semantics in our 
(SUSE) trees since january disclosure; STIBP was more or less ignored up 
to today.

v1->v2:
        include IBPB changes
v2->v3: 
        fix IBPB 'who can trace who' semantics
        wire up STIBP flipping to SMT hotplug
v3->v4:
	dropped ___ptrace_may_access(), as it's not needed
	fixed deadlock with LSM/audit/selinux (Andrea Arcangeli)
	statically patch out the ptrace check if !IBPB

v4->v5:
	fix MSR writing logic (Thomas Gleixner, Josh Poimboeuf)

v5->v6:
	propagate X86_FEATURE_RSB_CTXSW setting to sysfs
	propagate STIBP setting to sysfs (Thomas Gleixner)
	simplify arch_smt_update() (Thomas Gleixner)

Jiri Kosina (3):
      x86/speculation: apply IBPB more strictly to avoid cross-process data leak
      x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation
      x86/speculation: Propagate information about RSB filling mitigation to sysfs

 arch/x86/kernel/cpu/bugs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 arch/x86/mm/tlb.c          | 31 ++++++++++++++++++++-----------
 include/linux/ptrace.h     |  4 ++++
 kernel/cpu.c               | 11 ++++++++++-
 kernel/ptrace.c            | 12 ++++++++----
 5 files changed, 96 insertions(+), 22 deletions(-)

-- 
Jiri Kosina
SUSE Labs

[PATCH v6 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Currently, we are issuing IBPB only in cases when switching into a non-dumpable
process, the rationale being to protect such 'important and security sensitive'
processess (such as GPG) from data leak into a different userspace process via
spectre v2.

This is however completely insufficient to provide proper userspace-to-userpace
spectrev2 protection, as any process can poison branch buffers before being
scheduled out, and the newly scheduled process immediately becomes spectrev2
victim.

In order to minimize the performance impact (for usecases that do require
spectrev2 protection), issue the barrier only in cases when switching between
processess where the victim can't be ptraced by the potential attacker (as in
such cases, the attacker doesn't have to bother with branch buffers at all).

Fixes: 18bf3c3ea8 ("x86/speculation: Use Indirect Branch Prediction Barrier in context switch")
Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/mm/tlb.c      | 31 ++++++++++++++++++++-----------
 include/linux/ptrace.h |  4 ++++
 kernel/ptrace.c        | 12 ++++++++----
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e96b99eb800c..ed4444402441 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
+#include <linux/ptrace.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(struct mm_struct *mm)
 	}
 }
 
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+	/*
+	 * Check if the current (previous) task has access to the memory
+	 * of the @tsk (next) task. If access is denied, make sure to
+	 * issue a IBPB to stop user->user Spectre-v2 attacks.
+	 *
+	 * Note: __ptrace_may_access() returns 0 or -ERRNO.
+	 */
+	return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+			__ptrace_may_access(tsk, PTRACE_MODE_IBPB));
+}
+
 void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			struct task_struct *tsk)
 {
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		 * one process from doing Spectre-v2 attacks on another.
 		 *
 		 * As an optimization, flush indirect branches only when
-		 * switching into processes that disable dumping. This
-		 * protects high value processes like gpg, without having
-		 * too high performance overhead. IBPB is *expensive*!
-		 *
-		 * This will not flush branches when switching into kernel
-		 * threads. It will also not flush if we switch to idle
-		 * thread and back to the same process. It will flush if we
-		 * switch to a different non-dumpable process.
+		 * switching into a processes that can't be ptrace by the
+		 * current one (as in such case, attacker has much more
+		 * convenient way how to tamper with the next process than
+		 * branch buffer poisoning).
 		 */
-		if (tsk && tsk->mm &&
-		    tsk->mm->context.ctx_id != last_ctx_id &&
-		    get_dumpable(tsk->mm) != SUID_DUMP_USER)
+		if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+				ibpb_needed(tsk, last_ctx_id))
 			indirect_branch_prediction_barrier();
 
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 4f36431c380b..983d3f5545a8 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -64,12 +64,15 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
 #define PTRACE_MODE_NOAUDIT	0x04
 #define PTRACE_MODE_FSCREDS 0x08
 #define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_NOACCESS_CHK 0x20
 
 /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
 #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
 #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_IBPB (PTRACE_MODE_ATTACH | PTRACE_MODE_NOAUDIT \
+			  | PTRACE_MODE_NOACCESS_CHK | PTRACE_MODE_REALCREDS)
 
 /**
  * ptrace_may_access - check whether the caller is permitted to access
@@ -86,6 +89,7 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
  * process_vm_writev or ptrace (and should use the real credentials).
  */
 extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+extern int __ptrace_may_access(struct task_struct *task, unsigned int mode);
 
 static inline int ptrace_reparented(struct task_struct *child)
 {
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 21fec73d45d4..5c5e7cb597cd 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -268,7 +268,7 @@ static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
 }
 
 /* Returns 0 on success, -errno on denial. */
-static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
+int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
 	const struct cred *cred = current_cred(), *tcred;
 	struct mm_struct *mm;
@@ -316,7 +316,8 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 	    gid_eq(caller_gid, tcred->sgid) &&
 	    gid_eq(caller_gid, tcred->gid))
 		goto ok;
-	if (ptrace_has_cap(tcred->user_ns, mode))
+	if (!(mode & PTRACE_MODE_NOACCESS_CHK) &&
+	     ptrace_has_cap(tcred->user_ns, mode))
 		goto ok;
 	rcu_read_unlock();
 	return -EPERM;
@@ -325,10 +326,13 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 	mm = task->mm;
 	if (mm &&
 	    ((get_dumpable(mm) != SUID_DUMP_USER) &&
-	     !ptrace_has_cap(mm->user_ns, mode)))
+	     ((mode & PTRACE_MODE_NOACCESS_CHK) ||
+	       !ptrace_has_cap(mm->user_ns, mode))))
 	    return -EPERM;
 
-	return security_ptrace_access_check(task, mode);
+	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
+		return security_ptrace_access_check(task, mode);
+	return 0;
 }
 
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)

-- 
Jiri Kosina
SUSE Labs

[PATCH v6 2/3] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

STIBP is a feature provided by certain Intel ucodes / CPUs. This feature
(once enabled) prevents cross-hyperthread control of decisions made by
indirect branch predictors.

Enable this feature if

- the CPU is vulnerable to spectre v2
- the CPU supports SMT and has SMT siblings online
- spectre_v2 mitigation autoselection is enabled (default)

After some previous discussion, this patch leaves STIBP on all the time,
as wrmsr on crossing kernel boundary is a no-no. This could perhaps later
be a bit more optimized (like disabling it in NOHZ, experiment with
disabling it in idle, etc) if needed.

Note that the synchronization of the mask manipulation via newly added
spec_ctrl_mutex is currently not strictly needed, as the only updater is
already being serialized by cpu_add_remove_lock, but let's make this a
little bit more future-proof.

Cc: stable@vger.kernel.org
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/kernel/cpu/bugs.c | 59 +++++++++++++++++++++++++++++++++++++++++-----
 kernel/cpu.c               | 11 ++++++++-
 2 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 40bdaea97fe7..6bc76bdf5a0a 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -35,12 +35,10 @@ static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
 static void __init l1tf_select_mitigation(void);
 
-/*
- * Our boot-time value of the SPEC_CTRL MSR. We read it once so that any
- * writes to SPEC_CTRL contain whatever reserved bits have been set.
- */
-u64 __ro_after_init x86_spec_ctrl_base;
+/* The base value of the SPEC_CTRL MSR that always has to be preserved. */
+u64 x86_spec_ctrl_base;
 EXPORT_SYMBOL_GPL(x86_spec_ctrl_base);
+static DEFINE_MUTEX(spec_ctrl_mutex);
 
 /*
  * The vendor and possibly platform specific bits which can be modified in
@@ -325,6 +323,46 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
 	return cmd;
 }
 
+static bool stibp_needed(void)
+{
+	if (spectre_v2_enabled == SPECTRE_V2_NONE)
+		return false;
+
+	if (!boot_cpu_has(X86_FEATURE_STIBP))
+		return false;
+
+	return true;
+}
+
+static void update_stibp_msr(void *info)
+{
+	wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
+void arch_smt_update(void)
+{
+	u64 mask;
+
+	if (!stibp_needed())
+		return;
+
+	mutex_lock(&spec_ctrl_mutex);
+	mask = x86_spec_ctrl_base;
+	if (cpu_smt_control == CPU_SMT_ENABLED)
+		mask |= SPEC_CTRL_STIBP;
+	else
+		mask &= ~SPEC_CTRL_STIBP;
+
+	if (mask != x86_spec_ctrl_base) {
+		pr_info("Spectre v2 cross-process SMT mitigation: %s STIBP\n",
+				cpu_smt_control == CPU_SMT_ENABLED ?
+				"Enabling" : "Disabling");
+		x86_spec_ctrl_base = mask;
+		on_each_cpu(update_stibp_msr, NULL, 1);
+	}
+	mutex_unlock(&spec_ctrl_mutex);
+}
+
 static void __init spectre_v2_select_mitigation(void)
 {
 	enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
@@ -424,6 +462,9 @@ static void __init spectre_v2_select_mitigation(void)
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
+
+	/* Enable STIBP if appropriate */
+	arch_smt_update();
 }
 
 #undef pr_fmt
@@ -814,6 +855,8 @@ static ssize_t l1tf_show_state(char *buf)
 static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
 			       char *buf, unsigned int bug)
 {
+	int ret;
+
 	if (!boot_cpu_has_bug(bug))
 		return sprintf(buf, "Not affected\n");
 
@@ -831,10 +874,14 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 		return sprintf(buf, "Mitigation: __user pointer sanitization\n");
 
 	case X86_BUG_SPECTRE_V2:
-		return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
+		mutex_lock(&spec_ctrl_mutex);
+		ret = sprintf(buf, "%s%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
 			       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
 			       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
+			       (x86_spec_ctrl_base & SPEC_CTRL_STIBP) ? ", STIBP" : "",
 			       spectre_v2_module_string());
+		mutex_unlock(&spec_ctrl_mutex);
+		return ret;
 
 	case X86_BUG_SPEC_STORE_BYPASS:
 		return sprintf(buf, "%s\n", ssb_strings[ssb_mode]);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index aa7fe85ad62e..4bba5071d61e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2025,6 +2025,12 @@ static void cpuhp_online_cpu_device(unsigned int cpu)
 	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
 }
 
+/*
+ * Architectures that need SMT-specific errata handling during SMT hotplug
+ * should override these.
+ */
+void __weak arch_smt_update(void) { };
+
 static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
 {
 	int cpu, ret = 0;
@@ -2051,8 +2057,10 @@ static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
 		 */
 		cpuhp_offline_cpu_device(cpu);
 	}
-	if (!ret)
+	if (!ret) {
 		cpu_smt_control = ctrlval;
+		arch_smt_update();
+	}
 	cpu_maps_update_done();
 	return ret;
 }
@@ -2063,6 +2071,7 @@ static int cpuhp_smt_enable(void)
 
 	cpu_maps_update_begin();
 	cpu_smt_control = CPU_SMT_ENABLED;
+	arch_smt_update();
 	for_each_present_cpu(cpu) {
 		/* Skip online CPUs and CPUs on offline nodes */
 		if (cpu_online(cpu) || !node_online(cpu_to_node(cpu)))

-- 
Jiri Kosina
SUSE Labs

[PATCH v6 3/3] x86/speculation: Propagate information about RSB filling mitigation to sysfs

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

If spectrev2 mitigation has been enabled, we're filling RSB on context switch
in order to protect from various classess of spectrev2 attacks.

If this mitigation is enabled, say so in sysfs for spectrev2.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/kernel/cpu/bugs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 6bc76bdf5a0a..ee46dcbae5fa 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -875,10 +875,11 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 
 	case X86_BUG_SPECTRE_V2:
 		mutex_lock(&spec_ctrl_mutex);
-		ret = sprintf(buf, "%s%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
+		ret = sprintf(buf, "%s%s%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
 			       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
 			       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
 			       (x86_spec_ctrl_base & SPEC_CTRL_STIBP) ? ", STIBP" : "",
+			       boot_cpu_has(X86_FEATURE_RSB_CTXSW) ? ", RSB filling" : "",
 			       spectre_v2_module_string());
 		mutex_unlock(&spec_ctrl_mutex);
 		return ret;

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Thomas Gleixner]

On Tue, 11 Sep 2018, Schaufler, Casey wrote:
> How about this? Take Jiri's patch as written. You get everything except checks
> on the security blobs and any "magic" that my safesidechannel module did. I
> will propose a follow on patch that fixes the SELinux code to eliminate the locking
> issue and enables the LSM hooks in the IBPB case. I can then do a revised "magic"
> safesidechannel security module that uses the ptrace hook instead of adding a
> new hook explicitly for IBPB. There is some danger that in the future ptrace and
> IBPB criteria will diverge sufficiently that a common hook becomes nonsensical.
> As no one else seems concerned about this possibility, I won't lose any sleep over
> it either.

Sounds like a plan.

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Tom Lendacky]

On 09/11/2018 04:16 PM, Thomas Gleixner wrote:
> On Tue, 11 Sep 2018, Tim Chen wrote:
>> On 09/10/2018 04:46 AM, Jiri Kosina wrote:
>>> Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
>>> the missing reporting of RSB_CTXSW for v6.
>>>
>>
>> I anticipate that STIBP could affect workloads with a lot of indirect
>> branches (see previous discussion with Andrea).  We should have a 
>> knob for people to opt in or opt out of STIBP.
> 
> Feel free to send a patch to that effect.

Tim, are you planning on sending a patch for this?  If so, what type of
opt in/out are you thinking about, something similar to SSBD?

Thanks,
Tom

> 
> Thanks,
> 
> 	tglx
> 

Re: [PATCH v6 2/3] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Thomas Gleixner]

On Wed, 12 Sep 2018, Jiri Kosina wrote:
>  	case X86_BUG_SPECTRE_V2:
> -		return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
> +		mutex_lock(&spec_ctrl_mutex);
> +		ret = sprintf(buf, "%s%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
>  			       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
>  			       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
> +			       (x86_spec_ctrl_base & SPEC_CTRL_STIBP) ? ", STIBP" : "",
>  			       spectre_v2_module_string());
> +		mutex_unlock(&spec_ctrl_mutex);

The mutex for this printing is overkill. It's a read after all and if there
is a concurrent SMT control fiddling going on then you have a chance of
getting the wrong information as well. I'll zap it.

Thanks,

	tglx

Re: [PATCH v6 2/3] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

On Wed, 12 Sep 2018, Thomas Gleixner wrote:

> >  	case X86_BUG_SPECTRE_V2:
> > -		return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
> > +		mutex_lock(&spec_ctrl_mutex);
> > +		ret = sprintf(buf, "%s%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
> >  			       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
> >  			       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
> > +			       (x86_spec_ctrl_base & SPEC_CTRL_STIBP) ? ", STIBP" : "",
> >  			       spectre_v2_module_string());
> > +		mutex_unlock(&spec_ctrl_mutex);
> 
> The mutex for this printing is overkill. It's a read after all and if there
> is a concurrent SMT control fiddling going on then you have a chance of
> getting the wrong information as well. 

Yeah; I was just happy to be able to stick second use of it there, with 
the first one being basically useless as well :)

> I'll zap it.

Absolutely feel free to. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Tim Chen]

On 09/12/2018 10:16 AM, Tom Lendacky wrote:
> 
> 
> On 09/11/2018 04:16 PM, Thomas Gleixner wrote:
>> On Tue, 11 Sep 2018, Tim Chen wrote:
>>> On 09/10/2018 04:46 AM, Jiri Kosina wrote:
>>>> Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
>>>> the missing reporting of RSB_CTXSW for v6.
>>>>
>>>
>>> I anticipate that STIBP could affect workloads with a lot of indirect
>>> branches (see previous discussion with Andrea).  We should have a 
>>> knob for people to opt in or opt out of STIBP.
>>
>> Feel free to send a patch to that effect.
> 
> Tim, are you planning on sending a patch for this?  If so, what type of
> opt in/out are you thinking about, something similar to SSBD?
> 

I'm working on a patch for choosing the Spectre v2 app to app
mitigation option.

Something like the following:

enum spectre_v2_app2app_mitigation {
        SPECTRE_V2_APP2APP_NONE,
        SPECTRE_V2_APP2APP_LITE,
        SPECTRE_V2_APP2APP_IBPB,
        SPECTRE_V2_APP2APP_STIBP,
        SPECTRE_V2_APP2APP_STRICT,
};

static const char *spectre_v2_app2app_strings[] = {
        [SPECTRE_V2_APP2APP_NONE]               = "App-App Vulnerable",
        [SPECTRE_V2_APP2APP_LITE]               = "App-App Mitigation: Protect only non-dumpable process",
        [SPECTRE_V2_APP2APP_IBPB]               = "App-App Mitigation: Protect app against attack from same cpu",
        [SPECTRE_V2_APP2APP_STIBP]              = "App-App Mitigation: Protect app against attack from sibling cpu",
        [SPECTRE_V2_APP2APP_STRICT]             = "App-App Mitigation: Full app to app attack protection",
};

So the APP2APP_LITE protection's intention is to turn on STIBP and IBPB for non-dumpable
process.  But in my first version I may limit it to IBPB as choosing
STIBP based on process characteristics will require some frobbing of
the flags as what we've done in SSBD.  That will require more careful
work and tests.

The STRICT option will turn STIBP on always and IBPB always on
non-ptraceable context switches.

Is this something reasonable?

Tom, if you already have a patch, feel free to post.

Tim

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Jiri Kosina]

On Wed, 12 Sep 2018, Tim Chen wrote:

> I'm working on a patch for choosing the Spectre v2 app to app
> mitigation option.
> 
> Something like the following:
> 
> enum spectre_v2_app2app_mitigation {
>         SPECTRE_V2_APP2APP_NONE,
>         SPECTRE_V2_APP2APP_LITE,
>         SPECTRE_V2_APP2APP_IBPB,
>         SPECTRE_V2_APP2APP_STIBP,
>         SPECTRE_V2_APP2APP_STRICT,
> };
> 
> static const char *spectre_v2_app2app_strings[] = {
>         [SPECTRE_V2_APP2APP_NONE]               = "App-App Vulnerable",
>         [SPECTRE_V2_APP2APP_LITE]               = "App-App Mitigation: Protect only non-dumpable process",
>         [SPECTRE_V2_APP2APP_IBPB]               = "App-App Mitigation: Protect app against attack from same cpu",
>         [SPECTRE_V2_APP2APP_STIBP]              = "App-App Mitigation: Protect app against attack from sibling cpu",
>         [SPECTRE_V2_APP2APP_STRICT]             = "App-App Mitigation: Full app to app attack protection",
> };
> 
> So the APP2APP_LITE protection's intention is to turn on STIBP and IBPB for non-dumpable
> process.  But in my first version I may limit it to IBPB as choosing
> STIBP based on process characteristics will require some frobbing of
> the flags as what we've done in SSBD.  That will require more careful
> work and tests.
> 
> The STRICT option will turn STIBP on always and IBPB always on
> non-ptraceable context switches.
> 
> Is this something reasonable?

It's probably 100% correct, but it's also 100% super-complex at the same 
time if you ask me.

Try to imagine you're a very advanced senior sysadmin, who has heard that 
spectre and meltdown existed of course, but figured out that updating to 
latest kernel/distro vendor update fixes all the security issues (and it 
actually indeed did).

Now, all of a sudden, this new option pops up, and the poor sysadmin has 
to make a decision again.

	"Do you care only about security across non-dumpable process 
	 boundaries?"

	"Scheduled to same CPU at the time of attack? Can you guarantee that this 
	 is (not) happening?"

	"If the processess can actually ptrace/debug each other, are you okay with 
	 them attacking each other?"

	 "Shared HT siblings return target buffer, do you want it or 
	  not?"

These are the questions that even an excellent sysadmin might not have 
qualified answers to so far. Now, all of a sudden, he/her has to make 
these decisions?

I don't think that's how it should work. It all should be digestible by 
"linux end-users" (where users are also super-advanced sysadmins) easily.

We currently have "I do care about spectrev2 / I don't care about 
spectrev2" boot-time switch, and I don't see us going any deeper / more 
fine-grained without sacrificing clarity and sanity.

Or do you see a way how to do that nicely?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Tim Chen]

On 09/12/2018 02:45 PM, Jiri Kosina wrote:
> On Wed, 12 Sep 2018, Tim Chen wrote:
> 
>> I'm working on a patch for choosing the Spectre v2 app to app
>> mitigation option.
>>
>> Something like the following:
>>
>> enum spectre_v2_app2app_mitigation {
>>         SPECTRE_V2_APP2APP_NONE,
>>         SPECTRE_V2_APP2APP_LITE,
>>         SPECTRE_V2_APP2APP_IBPB,
>>         SPECTRE_V2_APP2APP_STIBP,
>>         SPECTRE_V2_APP2APP_STRICT,
>> };
>>
>> static const char *spectre_v2_app2app_strings[] = {
>>         [SPECTRE_V2_APP2APP_NONE]               = "App-App Vulnerable",
>>         [SPECTRE_V2_APP2APP_LITE]               = "App-App Mitigation: Protect only non-dumpable process",
>>         [SPECTRE_V2_APP2APP_IBPB]               = "App-App Mitigation: Protect app against attack from same cpu",
>>         [SPECTRE_V2_APP2APP_STIBP]              = "App-App Mitigation: Protect app against attack from sibling cpu",
>>         [SPECTRE_V2_APP2APP_STRICT]             = "App-App Mitigation: Full app to app attack protection",
>> };
>>
>> So the APP2APP_LITE protection's intention is to turn on STIBP and IBPB for non-dumpable
>> process.  But in my first version I may limit it to IBPB as choosing
>> STIBP based on process characteristics will require some frobbing of
>> the flags as what we've done in SSBD.  That will require more careful
>> work and tests.
>>
>> The STRICT option will turn STIBP on always and IBPB always on
>> non-ptraceable context switches.
>>
>> Is this something reasonable?
> 
> It's probably 100% correct, but it's also 100% super-complex at the same 
> time if you ask me.
> 
> Try to imagine you're a very advanced senior sysadmin, who has heard that 
> spectre and meltdown existed of course, but figured out that updating to 
> latest kernel/distro vendor update fixes all the security issues (and it 
> actually indeed did).
> 
> Now, all of a sudden, this new option pops up, and the poor sysadmin has 
> to make a decision again.
> 
> 	"Do you care only about security across non-dumpable process 
> 	 boundaries?"
> 
> 	"Scheduled to same CPU at the time of attack? Can you guarantee that this 
> 	 is (not) happening?"
> 
> 	"If the processess can actually ptrace/debug each other, are you okay with 
> 	 them attacking each other?"
> 
> 	 "Shared HT siblings return target buffer, do you want it or 
> 	  not?"
> 
> These are the questions that even an excellent sysadmin might not have 
> qualified answers to so far. Now, all of a sudden, he/her has to make 
> these decisions?
> 
> I don't think that's how it should work. It all should be digestible by 
> "linux end-users" (where users are also super-advanced sysadmins) easily.
> 
> We currently have "I do care about spectrev2 / I don't care about 
> spectrev2" boot-time switch, and I don't see us going any deeper / more 
> fine-grained without sacrificing clarity and sanity.
> 
> Or do you see a way how to do that nicely?
> 

How about just these options:

static const char *spectre_v2_app2app_strings[] = {
        [SPECTRE_V2_APP2APP_NONE]               = "App-App Vulnerable",
        [SPECTRE_V2_APP2APP_LITE]               = "App-App Mitigation: Protect only non-dumpable process",
        [SPECTRE_V2_APP2APP_STRICT]             = "App-App Mitigation: Full app to app attack protection",
};

Tim

RE: [PATCH v6 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> 
> 

> @@ -325,10 +326,13 @@ static int __ptrace_may_access(struct task_struct
> *task, unsigned int mode)
>  	mm = task->mm;
>  	if (mm &&
>  	    ((get_dumpable(mm) != SUID_DUMP_USER) &&
> -	     !ptrace_has_cap(mm->user_ns, mode)))
> +	     ((mode & PTRACE_MODE_NOACCESS_CHK) ||
> +	       !ptrace_has_cap(mm->user_ns, mode))))
>  	    return -EPERM;
> 
> -	return security_ptrace_access_check(task, mode);
> +	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> +		return security_ptrace_access_check(task, mode);
> +	return 0;

Because PTRACE_MODE_IBPB includes PTRACE_MODE_NOAUDIT you
shouldn't need this change. Do you have a good way to exercise this code
path? I'm having trouble getting to the check, and have yet to get a case
where PTRACE_MODE_NOACCESS_CHK is set.

>  }
> 
>  bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> 
> --
> Jiri Kosina
> SUSE Labs

Re: [PATCH v5 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Tom Lendacky]

On 09/12/2018 04:26 PM, Tim Chen wrote:
> On 09/12/2018 10:16 AM, Tom Lendacky wrote:
>>
>>
>> On 09/11/2018 04:16 PM, Thomas Gleixner wrote:
>>> On Tue, 11 Sep 2018, Tim Chen wrote:
>>>> On 09/10/2018 04:46 AM, Jiri Kosina wrote:
>>>>> Nah, IBPB is actuall there, sorry. So I'll add reporting of STIBP + fixup 
>>>>> the missing reporting of RSB_CTXSW for v6.
>>>>>
>>>>
>>>> I anticipate that STIBP could affect workloads with a lot of indirect
>>>> branches (see previous discussion with Andrea).  We should have a 
>>>> knob for people to opt in or opt out of STIBP.
>>>
>>> Feel free to send a patch to that effect.
>>
>> Tim, are you planning on sending a patch for this?  If so, what type of
>> opt in/out are you thinking about, something similar to SSBD?
>>
> 
> I'm working on a patch for choosing the Spectre v2 app to app
> mitigation option.
> 
> Something like the following:
> 
> enum spectre_v2_app2app_mitigation {
>         SPECTRE_V2_APP2APP_NONE,
>         SPECTRE_V2_APP2APP_LITE,
>         SPECTRE_V2_APP2APP_IBPB,
>         SPECTRE_V2_APP2APP_STIBP,
>         SPECTRE_V2_APP2APP_STRICT,
> };
> 
> static const char *spectre_v2_app2app_strings[] = {
>         [SPECTRE_V2_APP2APP_NONE]               = "App-App Vulnerable",
>         [SPECTRE_V2_APP2APP_LITE]               = "App-App Mitigation: Protect only non-dumpable process",
>         [SPECTRE_V2_APP2APP_IBPB]               = "App-App Mitigation: Protect app against attack from same cpu",
>         [SPECTRE_V2_APP2APP_STIBP]              = "App-App Mitigation: Protect app against attack from sibling cpu",
>         [SPECTRE_V2_APP2APP_STRICT]             = "App-App Mitigation: Full app to app attack protection",
> };
> 
> So the APP2APP_LITE protection's intention is to turn on STIBP and IBPB for non-dumpable
> process.  But in my first version I may limit it to IBPB as choosing
> STIBP based on process characteristics will require some frobbing of
> the flags as what we've done in SSBD.  That will require more careful
> work and tests.
> 
> The STRICT option will turn STIBP on always and IBPB always on
> non-ptraceable context switches.
> 
> Is this something reasonable?
> 
> Tom, if you already have a patch, feel free to post.

No, I don't have anything.  I just like the idea of opt in/out for STIBP
and thought it should be similar to SSBD to provide consistency.

Thanks,
Tom

> 
> Tim
> 

RE: [PATCH v6 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Thu, 13 Sep 2018, Schaufler, Casey wrote:

> > -	return security_ptrace_access_check(task, mode);
> > +	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> > +		return security_ptrace_access_check(task, mode);
> > +	return 0;
> 
> Because PTRACE_MODE_IBPB includes PTRACE_MODE_NOAUDIT you
> shouldn't need this change. 

That is true, but that's not my concern here. 

	security_ptrace_access_check() -> call_int_hook() -> P->hook.FUNC().

If it's somehow guaranteed that all functions called this ways are fine to 
be called from scheduler context (wrt. locks), then it's all fine and I'll 
happily drop that check.

Is it guaranteed?

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH v6 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Thomas Gleixner]

On Fri, 14 Sep 2018, Jiri Kosina wrote:
> On Thu, 13 Sep 2018, Schaufler, Casey wrote:
> 
> > > -	return security_ptrace_access_check(task, mode);
> > > +	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> > > +		return security_ptrace_access_check(task, mode);
> > > +	return 0;
> > 
> > Because PTRACE_MODE_IBPB includes PTRACE_MODE_NOAUDIT you
> > shouldn't need this change. 
> 
> That is true, but that's not my concern here. 
> 
> 	security_ptrace_access_check() -> call_int_hook() -> P->hook.FUNC().
> 
> If it's somehow guaranteed that all functions called this ways are fine to 
> be called from scheduler context (wrt. locks), then it's all fine and I'll 
> happily drop that check.
> 
> Is it guaranteed?

The related question is whether it is guaranteed for backports. We don't
want to end up with a separate hell there.

Thanks,

	tglx

RE: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Schaufler, Casey]

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

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@kernel.org]
> Sent: Wednesday, September 12, 2018 2:05 AM
> To: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>;
> Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@redhat.com>; Andrea Arcangeli <aarcange@redhat.com>;
> Woodhouse, David <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>;
> Tim Chen <tim.c.chen@linux.intel.com>; Schaufler, Casey
> <casey.schaufler@intel.com>
> Cc: linux-kernel@vger.kernel.org; x86@kernel.org
> Subject: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
> 
> Currently, linux kernel is basically not preventing userspace-userspace
> spectrev2 attack, because:
> 
> - IBPB is basically unused (issued only for tasks that marked themselves
>   explicitly non-dumpable, which is absolutely negligible minority of all
>   software out there), therefore cross-process branch buffer posioning
>   using spectrev2 is possible
> 
> - STIBP is completely unused, therefore cross-process branch buffer
>   poisoning using spectrev2 between processess running on two HT siblings
>   thread s is possible
> 
> This patchset changes IBPB semantics, so that it's now applied whenever
> context-switching between processess that can't use ptrace() to achieve
> the same. This admittedly comes with extra overhad on a context switch;
> systems that don't care about could disable the mitigation using
> nospectre_v2 boot option.
> The IBPB implementaion is heavily based on original patches by Tim Chen.
> 
> In addition to that, we unconditionally turn STIBP on so that HT siblings
> always have separate branch buffers.
> 
> We've been carrying IBPB implementation with the same semantics in our
> (SUSE) trees since january disclosure; STIBP was more or less ignored up
> to today.
> 
> v1->v2:
>         include IBPB changes
> v2->v3:
>         fix IBPB 'who can trace who' semantics
>         wire up STIBP flipping to SMT hotplug
> v3->v4:
> 	dropped ___ptrace_may_access(), as it's not needed
> 	fixed deadlock with LSM/audit/selinux (Andrea Arcangeli)
> 	statically patch out the ptrace check if !IBPB
> 
> v4->v5:
> 	fix MSR writing logic (Thomas Gleixner, Josh Poimboeuf)
> 
> v5->v6:
> 	propagate X86_FEATURE_RSB_CTXSW setting to sysfs
> 	propagate STIBP setting to sysfs (Thomas Gleixner)
> 	simplify arch_smt_update() (Thomas Gleixner)
> 
> Jiri Kosina (3):
>       x86/speculation: apply IBPB more strictly to avoid cross-process data leak
>       x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation
>       x86/speculation: Propagate information about RSB filling mitigation to sysfs
> 
>  arch/x86/kernel/cpu/bugs.c | 60
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  arch/x86/mm/tlb.c          | 31 ++++++++++++++++++++-----------
>  include/linux/ptrace.h     |  4 ++++
>  kernel/cpu.c               | 11 ++++++++++-
>  kernel/ptrace.c            | 12 ++++++++----
>  5 files changed, 96 insertions(+), 22 deletions(-)
> 
> --
> Jiri Kosina
> SUSE Labs

The locking issue with SELinux has a simple fix as below.
The other LSMs don't manifest this issue. With the change to
SELinux the call to security_ptrace_access_check() can and
should be made unconditionally.

Patch is attached, whitespace damaged (known problem) patch:

SELinux: Handle audit locking for PTRACE_MODE_IBPB

The SELinux audit code locking cannot be used from the
task switching code, which is where PTRACE_MODE_IBPB comes
from. As this is a system check, not a user action, audit
is not needed, and would generate noise. Use the unaudited
check for this case.

Signed-off-by: Casey Schaufler <casey.schaufler@intel.com>
---
 kernel/ptrace.c          | 4 +---
 security/selinux/hooks.c | 5 +++++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 5c5e7cb597cd..202a4d9c2af7 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
               !ptrace_has_cap(mm->user_ns, mode))))
            return -EPERM;

-       if (!(mode & PTRACE_MODE_NOACCESS_CHK))
-               return security_ptrace_access_check(task, mode);
-       return 0;
+       return security_ptrace_access_check(task, mode);
 }

 bool ptrace_may_access(struct task_struct *task, unsigned int mode)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 161a4f29f860..30d21142e9fe 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
 {
        u32 sid = current_sid();
        u32 csid = task_sid(child);
+       struct av_decision avd;

+       if (mode == PTRACE_MODE_IBPB)
+               return avc_has_perm_noaudit(&selinux_state, sid, csid,
+                                           SECCLASS_PROCESS, PROCESS__PTRACE,
+                                           0, &avd);
        if (mode & PTRACE_MODE_READ)
                return avc_has_perm(&selinux_state,
                                    sid, csid, SECCLASS_FILE, FILE__READ, NULL);


[-- Attachment #2: casey-jiri-v6.patch --]
[-- Type: application/octet-stream, Size: 1634 bytes --]

SELinux: Handle audit locking for PTRACE_MODE_IBPB

The SELinux audit code locking cannot be used from the
task switching code, which is where PTRACE_MODE_IBPB comes
from. As this is a system check, not a user action, audit
is not needed, and would generate noise. Use the unaudited
check for this case.

Signed-off-by: Casey Schaufler <casey.schaufler@intel.com>
---
 kernel/ptrace.c          | 4 +---
 security/selinux/hooks.c | 5 +++++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 5c5e7cb597cd..202a4d9c2af7 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 	       !ptrace_has_cap(mm->user_ns, mode))))
 	    return -EPERM;
 
-	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
-		return security_ptrace_access_check(task, mode);
-	return 0;
+	return security_ptrace_access_check(task, mode);
 }
 
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 161a4f29f860..30d21142e9fe 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
 {
 	u32 sid = current_sid();
 	u32 csid = task_sid(child);
+	struct av_decision avd;
 
+	if (mode == PTRACE_MODE_IBPB)
+		return avc_has_perm_noaudit(&selinux_state, sid, csid,
+					    SECCLASS_PROCESS, PROCESS__PTRACE,
+					    0, &avd);
 	if (mode & PTRACE_MODE_READ)
 		return avc_has_perm(&selinux_state,
 				    sid, csid, SECCLASS_FILE, FILE__READ, NULL);

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Peter Zijlstra]

On Mon, Sep 17, 2018 at 04:09:33PM +0000, Schaufler, Casey wrote:

> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 5c5e7cb597cd..202a4d9c2af7 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>                !ptrace_has_cap(mm->user_ns, mode))))
>             return -EPERM;
> 
> -       if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> -               return security_ptrace_access_check(task, mode);
> -       return 0;
> +       return security_ptrace_access_check(task, mode);
>  }
> 
>  bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 161a4f29f860..30d21142e9fe 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
>  {
>         u32 sid = current_sid();
>         u32 csid = task_sid(child);
> +       struct av_decision avd;
> 
> +       if (mode == PTRACE_MODE_IBPB)
> +               return avc_has_perm_noaudit(&selinux_state, sid, csid,
> +                                           SECCLASS_PROCESS, PROCESS__PTRACE,
> +                                           0, &avd);
>         if (mode & PTRACE_MODE_READ)
>                 return avc_has_perm(&selinux_state,
>                                     sid, csid, SECCLASS_FILE, FILE__READ, NULL);
> 

As far as I can tell, this still has:

	avc_has_perm_noaudit()
	  security_compute_av()
	    read_lock(&state->ss->policy_rwlock);
	  avc_insert()
	    spin_lock_irqsave();
	  avc_denied()
	    avc_update_node()
	      spin_lock_irqsave();

under the scheduler's raw_spinlock_t, which are invalid lock nestings.

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Jiri Kosina]

On Wed, 19 Sep 2018, Peter Zijlstra wrote:

> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 5c5e7cb597cd..202a4d9c2af7 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> >                !ptrace_has_cap(mm->user_ns, mode))))
> >             return -EPERM;
> > 
> > -       if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> > -               return security_ptrace_access_check(task, mode);
> > -       return 0;
> > +       return security_ptrace_access_check(task, mode);
> >  }
> > 
> >  bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 161a4f29f860..30d21142e9fe 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
> >  {
> >         u32 sid = current_sid();
> >         u32 csid = task_sid(child);
> > +       struct av_decision avd;
> > 
> > +       if (mode == PTRACE_MODE_IBPB)
> > +               return avc_has_perm_noaudit(&selinux_state, sid, csid,
> > +                                           SECCLASS_PROCESS, PROCESS__PTRACE,
> > +                                           0, &avd);
> >         if (mode & PTRACE_MODE_READ)
> >                 return avc_has_perm(&selinux_state,
> >                                     sid, csid, SECCLASS_FILE, FILE__READ, NULL);
> > 
> 
> As far as I can tell, this still has:
> 
> 	avc_has_perm_noaudit()
> 	  security_compute_av()
> 	    read_lock(&state->ss->policy_rwlock);
> 	  avc_insert()
> 	    spin_lock_irqsave();
> 	  avc_denied()
> 	    avc_update_node()
> 	      spin_lock_irqsave();
> 
> under the scheduler's raw_spinlock_t, which are invalid lock nestings.

Agreed. Therefore, if the current form (v6) of the patches is merged, the 
check before security_ptrace_access_check() should stay.

Once all the LSM callbacks are potentially audited, it could then go in 
second phase.

Is there anything else blocking v6 being merged? (and then Tim's set on 
top I guess, once the details are sorted out there).

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Thomas Gleixner]

On Sat, 22 Sep 2018, Jiri Kosina wrote:
> On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> > As far as I can tell, this still has:
> > 
> > 	avc_has_perm_noaudit()
> > 	  security_compute_av()
> > 	    read_lock(&state->ss->policy_rwlock);
> > 	  avc_insert()
> > 	    spin_lock_irqsave();
> > 	  avc_denied()
> > 	    avc_update_node()
> > 	      spin_lock_irqsave();
> > 
> > under the scheduler's raw_spinlock_t, which are invalid lock nestings.
> 
> Agreed. Therefore, if the current form (v6) of the patches is merged, the 
> check before security_ptrace_access_check() should stay.
> 
> Once all the LSM callbacks are potentially audited, it could then go in 
> second phase.
> 
> Is there anything else blocking v6 being merged? (and then Tim's set on 
> top I guess, once the details are sorted out there).

I was waiting for this to resolve. Now, looking at the outcome of this, I
think, that calling into security hooks needs very special care from that
context.

So we better split that up right away instead of adding this magic
PTRACE_MODE_NOACCESS_CHK bit. This split up will be needed for adding the
special LSM speculation control hook anyway because adding yet more magic
bits and conditionals makes this code completely undigestable. It's
convoluted enough already.

Something along the compiled but completely untested patch below should do
the trick. If that is what we want this needs to be split up of course.

Thanks,

	tglx

8<--------------------------
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
+#include <linux/ptrace.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
 	}
 }
 
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+	/*
+	 * Check if the current (previous) task has access to the memory
+	 * of the @tsk (next) task. If access is denied, make sure to
+	 * issue a IBPB to stop user->user Spectre-v2 attacks.
+	 *
+	 * Note: __ptrace_may_access() returns 0 or -ERRNO.
+	 */
+	return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+		ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
 void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			struct task_struct *tsk)
 {
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
 		 * one process from doing Spectre-v2 attacks on another.
 		 *
 		 * As an optimization, flush indirect branches only when
-		 * switching into processes that disable dumping. This
-		 * protects high value processes like gpg, without having
-		 * too high performance overhead. IBPB is *expensive*!
-		 *
-		 * This will not flush branches when switching into kernel
-		 * threads. It will also not flush if we switch to idle
-		 * thread and back to the same process. It will flush if we
-		 * switch to a different non-dumpable process.
+		 * switching into a processes that can't be ptrace by the
+		 * current one (as in such case, attacker has much more
+		 * convenient way how to tamper with the next process than
+		 * branch buffer poisoning).
 		 */
-		if (tsk && tsk->mm &&
-		    tsk->mm->context.ctx_id != last_ctx_id &&
-		    get_dumpable(tsk->mm) != SUID_DUMP_USER)
+		if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+				ibpb_needed(tsk, last_ctx_id))
 			indirect_branch_prediction_barrier();
 
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,16 @@ extern void exit_ptrace(struct task_stru
 #define PTRACE_MODE_READ	0x01
 #define PTRACE_MODE_ATTACH	0x02
 #define PTRACE_MODE_NOAUDIT	0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS	0x08
+#define PTRACE_MODE_REALCREDS	0x10
+#define PTRACE_MODE_IBPB	0x20
 
 /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
 #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
 #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MODE_IBPB)
 
 /**
  * ptrace_may_access - check whether the caller is permitted to access
@@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
  * process_vm_writev or ptrace (and should use the real credentials).
  */
 extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode);
 
 static inline int ptrace_reparented(struct task_struct *child)
 {
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -267,14 +267,8 @@ static int ptrace_has_cap(struct user_na
 		return has_ns_capability(current, ns, CAP_SYS_PTRACE);
 }
 
-/* Returns 0 on success, -errno on denial. */
-static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
+static int __ptrace_may_access_basic(struct task_struct *task, unsigned int mode)
 {
-	const struct cred *cred = current_cred(), *tcred;
-	struct mm_struct *mm;
-	kuid_t caller_uid;
-	kgid_t caller_gid;
-
 	if (!(mode & PTRACE_MODE_FSCREDS) == !(mode & PTRACE_MODE_REALCREDS)) {
 		WARN(1, "denying ptrace access check without PTRACE_MODE_*CREDS\n");
 		return -EPERM;
@@ -292,7 +286,16 @@ static int __ptrace_may_access(struct ta
 	/* Don't let security modules deny introspection */
 	if (same_thread_group(task, current))
 		return 0;
-	rcu_read_lock();
+	/* No final decision yet */
+	return 1;
+}
+
+static int __ptrace_may_access_cred(const struct cred *tcred, unsigned int mode)
+{
+	const struct cred *cred = current_cred();
+	kuid_t caller_uid;
+	kgid_t caller_gid;
+
 	if (mode & PTRACE_MODE_FSCREDS) {
 		caller_uid = cred->fsuid;
 		caller_gid = cred->fsgid;
@@ -308,25 +311,61 @@ static int __ptrace_may_access(struct ta
 		caller_uid = cred->uid;
 		caller_gid = cred->gid;
 	}
-	tcred = __task_cred(task);
 	if (uid_eq(caller_uid, tcred->euid) &&
 	    uid_eq(caller_uid, tcred->suid) &&
 	    uid_eq(caller_uid, tcred->uid)  &&
 	    gid_eq(caller_gid, tcred->egid) &&
 	    gid_eq(caller_gid, tcred->sgid) &&
 	    gid_eq(caller_gid, tcred->gid))
-		goto ok;
-	if (ptrace_has_cap(tcred->user_ns, mode))
-		goto ok;
+		return 0;
+	return 1;
+}
+
+bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
+{
+	struct mm_struct *mm;
+	int res;
+
+	res = __ptrace_may_access_basic(task, mode);
+	if (res <= 0)
+		return !res;
+
+	rcu_read_lock();
+	res = __ptrace_may_access_cred(__task_cred(task), mode);
 	rcu_read_unlock();
-	return -EPERM;
-ok:
+	if (res)
+		return false;
+
+	mm = task->mm;
+	if (mm && get_dumpable(mm) != SUID_DUMP_USER)
+		return false;
+	return true;
+}
+
+/* Returns 0 on success, -errno on denial. */
+static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
+{
+	const struct cred *tcred;
+	struct mm_struct *mm;
+	int res;
+
+	res = __ptrace_may_access_basic(task, mode);
+	if (res <= 0)
+		return res;
+
+	rcu_read_lock();
+	tcred = __task_cred(task);
+	res = __ptrace_may_access_cred(tcred, mode);
+	if (res > 0)
+		res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
 	rcu_read_unlock();
+	if (res < 0)
+		return res;
+
 	mm = task->mm;
-	if (mm &&
-	    ((get_dumpable(mm) != SUID_DUMP_USER) &&
-	     !ptrace_has_cap(mm->user_ns, mode)))
-	    return -EPERM;
+	if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
+		   !ptrace_has_cap(mm->user_ns, mode)))
+		return -EPERM;
 
 	return security_ptrace_access_check(task, mode);
 }
@@ -334,6 +373,7 @@ static int __ptrace_may_access(struct ta
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
 	int err;
+
 	task_lock(task);
 	err = __ptrace_may_access(task, mode);
 	task_unlock(task);

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Peter Zijlstra]

On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> @@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
>   * process_vm_writev or ptrace (and should use the real credentials).
>   */
>  extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
> +extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode);

I like that..

>  static inline int ptrace_reparented(struct task_struct *child)
>  {
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c

> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> +	struct mm_struct *mm;
> +	int res;
> +
> +	res = __ptrace_may_access_basic(task, mode);
> +	if (res <= 0)
> +		return !res;
> +
> +	rcu_read_lock();
> +	res = __ptrace_may_access_cred(__task_cred(task), mode);
>  	rcu_read_unlock();
> +	if (res)
> +		return false;
> +
> +	mm = task->mm;
> +	if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> +		return false;
> +	return true;
> +}
> +
> +/* Returns 0 on success, -errno on denial. */
> +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +{
> +	const struct cred *tcred;
> +	struct mm_struct *mm;
> +	int res;
> +
> +	res = __ptrace_may_access_basic(task, mode);
> +	if (res <= 0)
> +		return res;
> +
> +	rcu_read_lock();
> +	tcred = __task_cred(task);
> +	res = __ptrace_may_access_cred(tcred, mode);
> +	if (res > 0)
> +		res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
>  	rcu_read_unlock();
> +	if (res < 0)
> +		return res;
> +
>  	mm = task->mm;
> +	if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> +		   !ptrace_has_cap(mm->user_ns, mode)))
> +		return -EPERM;
>  
>  	return security_ptrace_access_check(task, mode);
>  }

This has some unfortunate duplication.

Lets go with it for now, but I'll see if I can do something about that
later.

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Thomas Gleixner]

On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> > +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> > +{
> > +	struct mm_struct *mm;
> > +	int res;
> > +
> > +	res = __ptrace_may_access_basic(task, mode);
> > +	if (res <= 0)
> > +		return !res;
> > +
> > +	rcu_read_lock();
> > +	res = __ptrace_may_access_cred(__task_cred(task), mode);
> >  	rcu_read_unlock();
> > +	if (res)
> > +		return false;
> > +
> > +	mm = task->mm;
> > +	if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> > +		return false;
> > +	return true;
> > +}
> > +
> > +/* Returns 0 on success, -errno on denial. */
> > +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> > +{
> > +	const struct cred *tcred;
> > +	struct mm_struct *mm;
> > +	int res;
> > +
> > +	res = __ptrace_may_access_basic(task, mode);
> > +	if (res <= 0)
> > +		return res;
> > +
> > +	rcu_read_lock();
> > +	tcred = __task_cred(task);
> > +	res = __ptrace_may_access_cred(tcred, mode);
> > +	if (res > 0)
> > +		res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
> >  	rcu_read_unlock();
> > +	if (res < 0)
> > +		return res;
> > +
> >  	mm = task->mm;
> > +	if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> > +		   !ptrace_has_cap(mm->user_ns, mode)))
> > +		return -EPERM;
> >  
> >  	return security_ptrace_access_check(task, mode);
> >  }
> 
> This has some unfortunate duplication.
> 
> Lets go with it for now, but I'll see if I can do something about that
> later.

Yes, I know. I tried to make the duplication smaller, but all attempts
ended up being a convoluted mess. I'll try again after applying more
coffee.

Thanks,

	tglx

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Thomas Gleixner]

On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > This has some unfortunate duplication.
> > 
> > Lets go with it for now, but I'll see if I can do something about that
> > later.
> 
> Yes, I know. I tried to make the duplication smaller, but all attempts
> ended up being a convoluted mess. I'll try again after applying more
> coffee.

Lunch and coffee indeed made brain work better. The simple solution was way
too obvious.

Thanks,

	tglx

8<--------------------
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
+#include <linux/ptrace.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
 	}
 }
 
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+	/*
+	 * Check if the current (previous) task has access to the memory
+	 * of the @tsk (next) task. If access is denied, make sure to
+	 * issue a IBPB to stop user->user Spectre-v2 attacks.
+	 *
+	 * Note: __ptrace_may_access() returns 0 or -ERRNO.
+	 */
+	return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+		ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
 void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			struct task_struct *tsk)
 {
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
 		 * one process from doing Spectre-v2 attacks on another.
 		 *
 		 * As an optimization, flush indirect branches only when
-		 * switching into processes that disable dumping. This
-		 * protects high value processes like gpg, without having
-		 * too high performance overhead. IBPB is *expensive*!
-		 *
-		 * This will not flush branches when switching into kernel
-		 * threads. It will also not flush if we switch to idle
-		 * thread and back to the same process. It will flush if we
-		 * switch to a different non-dumpable process.
+		 * switching into a processes that can't be ptrace by the
+		 * current one (as in such case, attacker has much more
+		 * convenient way how to tamper with the next process than
+		 * branch buffer poisoning).
 		 */
-		if (tsk && tsk->mm &&
-		    tsk->mm->context.ctx_id != last_ctx_id &&
-		    get_dumpable(tsk->mm) != SUID_DUMP_USER)
+		if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+				ibpb_needed(tsk, last_ctx_id))
 			indirect_branch_prediction_barrier();
 
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,17 @@ extern void exit_ptrace(struct task_stru
 #define PTRACE_MODE_READ	0x01
 #define PTRACE_MODE_ATTACH	0x02
 #define PTRACE_MODE_NOAUDIT	0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS	0x08
+#define PTRACE_MODE_REALCREDS	0x10
+#define PTRACE_MODE_SCHED	0x20
+#define PTRACE_MODE_IBPB	0x40
 
 /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
 #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
 #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MOD_IBPB)
 
 /**
  * ptrace_may_access - check whether the caller is permitted to access
@@ -87,6 +90,20 @@ extern void exit_ptrace(struct task_stru
  */
 extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
 
+/**
+ * ptrace_may_access - check whether the caller is permitted to access
+ * a target task.
+ * @task: target task
+ * @mode: selects type of access and caller credentials
+ *
+ * Returns true on success, false on denial.
+ *
+ * Similar to ptrace_may_access(). Only to be called from context switch
+ * code. Does not call into audit and the regular LSM hooks due to locking
+ * constraints.
+ */
+extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode);
+
 static inline int ptrace_reparented(struct task_struct *child)
 {
 	return !same_thread_group(child->real_parent, child->parent);
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
 
 static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
 {
+	if (mode & PTRACE_MODE_SCHED)
+		return false;
+
 	if (mode & PTRACE_MODE_NOAUDIT)
 		return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
 	else
@@ -328,9 +331,16 @@ static int __ptrace_may_access(struct ta
 	     !ptrace_has_cap(mm->user_ns, mode)))
 	    return -EPERM;
 
+	if (mode & PTRACE_MODE_SCHED)
+		return 0;
 	return security_ptrace_access_check(task, mode);
 }
 
+bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
+{
+	return __ptrace_may_access(task, mode | PTRACE_MODE_SCHED);
+}
+
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
 	int err;

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Peter Zijlstra]

On Sat, Sep 22, 2018 at 03:30:07PM +0200, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> > On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > > This has some unfortunate duplication.
> > > 
> > > Lets go with it for now, but I'll see if I can do something about that
> > > later.
> > 
> > Yes, I know. I tried to make the duplication smaller, but all attempts
> > ended up being a convoluted mess. I'll try again after applying more
> > coffee.
> 
> Lunch and coffee indeed made brain work better. The simple solution was way
> too obvious.
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
>  
>  static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
>  {
> +	if (mode & PTRACE_MODE_SCHED)
> +		return false;
> +
>  	if (mode & PTRACE_MODE_NOAUDIT)
>  		return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
>  	else
> @@ -328,9 +331,16 @@ static int __ptrace_may_access(struct ta
>  	     !ptrace_has_cap(mm->user_ns, mode)))
>  	    return -EPERM;
>  
> +	if (mode & PTRACE_MODE_SCHED)
> +		return 0;
>  	return security_ptrace_access_check(task, mode);
>  }
>  
> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> +	return __ptrace_may_access(task, mode | PTRACE_MODE_SCHED);
> +}

Ha!, much nicer. Thanks!

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Jiri Kosina]

On Sat, 22 Sep 2018, Thomas Gleixner wrote:

> Lunch and coffee indeed made brain work better. The simple solution was way
> too obvious.

Ah, cool, I like it a lot.

Do you want me to fold this into v7, or are you on it already?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Thomas Gleixner]

On Mon, 24 Sep 2018, Jiri Kosina wrote:

> On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> 
> > Lunch and coffee indeed made brain work better. The simple solution was way
> > too obvious.
> 
> Ah, cool, I like it a lot.
> 
> Do you want me to fold this into v7, or are you on it already?

Please do so. I'm traveling/conferencing and only spotty online.

Thanks,

	tglx

Re: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Pavel Machek]

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

Hi!

> In order to minimize the performance impact (for usecases that do require
> spectrev2 protection), issue the barrier only in cases when switching between
> processess where the victim can't be ptraced by the potential attacker (as in
> such cases, the attacker doesn't have to bother with branch buffers
> at all).

Testing if attacker can ptrace victim is very good approximation, and
certainly better than "dumpable" check, but it is still not correct.

Imagine JIT running evil code (flash, javascript). JIT will prevent
evil code from doing ptrace() (or maybe there is syscall filter in
effect or something like that), but if evil code can poison branch
buffers and do timings, security problem stays.

Do we need prctl(I_DONT_RUN_EVIL_CODE)?

Or maybe we should just do barrier unconditionally for now?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to avoid cross-process data leak

[Jiri Kosina]

On Sun, 21 Oct 2018, Pavel Machek wrote:

> Imagine JIT running evil code (flash, javascript). JIT will prevent evil 
> code from doing ptrace() (or maybe there is syscall filter in effect or 
> something like that), but if evil code can poison branch buffers and do 
> timings, security problem stays.

JITs sort of remove the traditional unix security domain boundary between 
mutually (un)trusted code (processess and threads), that's a more general 
problem, yes.

> Do we need prctl(I_DONT_RUN_EVIL_CODE)?

That's basically the level of fine-graining Tim's followup patchset 
(that's currently being discussed) is eventually going to achieve.

Thanks,

-- 
Jiri Kosina
SUSE Labs