x86/speculation: Prevent stale SPEC_CTRL msr content

The seccomp speculation control operates on all tasks of a process, but
only the current task of a process can update the MSR immediately. For the
other threads the update is deferred to the next context switch.

This creates the following situation with Process A and B:

Process A task 2 and Process B task 1 are pinned on CPU1. Process A task 2
does not have the speculation control TIF bit set. Process B task 1 has the
speculation control TIF bit set.

CPU0					CPU1
					MSR bit is set
					ProcB.T1 schedules out
					ProcA.T2 schedules in
					MSR bit is cleared
ProcA.T1
  seccomp_update()
  set TIF bit on ProcA.T2
					ProcB.T1 schedules in
					MSR is not updated  <-- FAIL

This happens because the context switch code tries to avoid the MSR update
if the speculation control TIF bits of the incoming and the outgoing task
are the same. In the worst case ProcB.T1 and ProcA.T2 are the only tasks
scheduling back and forth on CPU1, which keeps the MSR stale forever.

In theory this could be remedied by IPIs, but chasing the remote task which
could be migrated is complex and full of races.

The straight forward solution is to avoid the asychronous update of the TIF
bit and defer it to the next context switch. The speculation control state
is stored in task_struct::atomic_flags by the prctl and seccomp updates
already.

Add a new TIF_SPEC_FORCE_UPDATE bit and set this after updating the
atomic_flags. Check the bit on context switch and force a synchronous
update of the speculation control if set. Use the same mechanism for
updating the current task.

Reported-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Casey Schaufler <casey.schaufler@intel.com>
Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Jon Masters <jcm@redhat.com>
Cc: Waiman Long <longman9394@gmail.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Dave Stewart <david.c.stewart@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/alpine.DEB.2.21.1811272247140.1875@nanos.tec.linutronix.de
CVE-2017-5715

(cherry picked from commit 6d991ba509ebcfcc908e009d1db51972a4f7a064)
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Juerg Haefliger <juergh@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/arch/x86/include/asm/spec-ctrl.h b/arch/x86/include/asm/spec-ctrl.h
index 27b0bce3933bac5bdf43d8592fc74251a3f61be4..5393babc05989ebc0cbcbbb21251f2c241e3df04 100644 (file)
--- a/arch/x86/include/asm/spec-ctrl.h
+++ b/arch/x86/include/asm/spec-ctrl.h
@@ -83,10 +83,6 @@ static inline void speculative_store_bypass_ht_init(void) { }
 #endif
 
 extern void speculation_ctrl_update(unsigned long tif);
-
-static inline void speculation_ctrl_update_current(void)
-{
-       speculation_ctrl_update(current_thread_info()->flags);
-}
+extern void speculation_ctrl_update_current(void);
 
 #endif

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 7b30338ca55b5c0157b5e0cea16aa3f468987703..bf9175d8784421ad797450ccbc28559e3f09b954 100644 (file)
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -86,6 +86,7 @@ struct thread_info {
 #define TIF_SYSCALL_AUDIT      7       /* syscall auditing active */
 #define TIF_SECCOMP            8       /* secure computing */
 #define TIF_SPEC_IB            9       /* Indirect branch speculation mitigation */
+#define TIF_SPEC_FORCE_UPDATE  10      /* Force speculation MSR update in context switch */
 #define TIF_USER_RETURN_NOTIFY 11      /* notify kernel of userspace return */
 #define TIF_UPROBE             12      /* breakpointed or singlestepping */
 #define TIF_PATCH_PENDING      13      /* pending live patching update */
@@ -114,6 +115,7 @@ struct thread_info {
 #define _TIF_SYSCALL_AUDIT     (1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP           (1 << TIF_SECCOMP)
 #define _TIF_SPEC_IB           (1 << TIF_SPEC_IB)
+#define _TIF_SPEC_FORCE_UPDATE (1 << TIF_SPEC_FORCE_UPDATE)
 #define _TIF_USER_RETURN_NOTIFY        (1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE            (1 << TIF_UPROBE)
 #define _TIF_PATCH_PENDING     (1 << TIF_PATCH_PENDING)
@@ -151,7 +153,7 @@ struct thread_info {
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW_BASE                                           \
        (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|         \
-        _TIF_SSBD)
+        _TIF_SSBD | _TIF_SPEC_FORCE_UPDATE)
 
 /*
  * Avoid calls to __switch_to_xtra() on UP as STIBP is not evaluated.

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 448392c0b8f275959333bc01ed0ab164bb2dd235..68cc8ed4a7a7a7f95dc2f4161bce2bfdd463ce71 100644 (file)
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -699,14 +699,10 @@ static void ssb_select_mitigation(void)
 #undef pr_fmt
 #define pr_fmt(fmt)     "Speculation prctl: " fmt
 
-static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
+static void task_update_spec_tif(struct task_struct *tsk)
 {
-       bool update;
-
-       if (on)
-               update = !test_and_set_tsk_thread_flag(tsk, tifbit);
-       else
-               update = test_and_clear_tsk_thread_flag(tsk, tifbit);
+       /* Force the update of the real TIF bits */
+       set_tsk_thread_flag(tsk, TIF_SPEC_FORCE_UPDATE);
 
        /*
         * Immediately update the speculation control MSRs for the current
@@ -716,7 +712,7 @@ static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
         * This can only happen for SECCOMP mitigation. For PRCTL it's
         * always the current task.
         */
-       if (tsk == current && update)
+       if (tsk == current)
                speculation_ctrl_update_current();
 }
 
@@ -732,16 +728,16 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
                if (task_spec_ssb_force_disable(task))
                        return -EPERM;
                task_clear_spec_ssb_disable(task);
-               task_update_spec_tif(task, TIF_SSBD, false);
+               task_update_spec_tif(task);
                break;
        case PR_SPEC_DISABLE:
                task_set_spec_ssb_disable(task);
-               task_update_spec_tif(task, TIF_SSBD, true);
+               task_update_spec_tif(task);
                break;
        case PR_SPEC_FORCE_DISABLE:
                task_set_spec_ssb_disable(task);
                task_set_spec_ssb_force_disable(task);
-               task_update_spec_tif(task, TIF_SSBD, true);
+               task_update_spec_tif(task);
                break;
        default:
                return -ERANGE;

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 38e2dc160c9e4c3c2b15128ba41adcf5be4015cc..533337a845b86c5ff785c1de233aba3b79d5fc53 100644 (file)
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -446,6 +446,18 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,
                wrmsrl(MSR_IA32_SPEC_CTRL, msr);
 }
 
+static unsigned long speculation_ctrl_update_tif(struct task_struct *tsk)
+{
+       if (test_and_clear_tsk_thread_flag(tsk, TIF_SPEC_FORCE_UPDATE)) {
+               if (task_spec_ssb_disable(tsk))
+                       set_tsk_thread_flag(tsk, TIF_SSBD);
+               else
+                       clear_tsk_thread_flag(tsk, TIF_SSBD);
+       }
+       /* Return the updated threadinfo flags*/
+       return task_thread_info(tsk)->flags;
+}
+
 void speculation_ctrl_update(unsigned long tif)
 {
        /* Forced update. Make sure all relevant TIF flags are different */
@@ -454,6 +466,14 @@ void speculation_ctrl_update(unsigned long tif)
        preempt_enable();
 }
 
+/* Called from seccomp/prctl update */
+void speculation_ctrl_update_current(void)
+{
+       preempt_disable();
+       speculation_ctrl_update(speculation_ctrl_update_tif(current));
+       preempt_enable();
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
 {
        struct thread_struct *prev, *next;
@@ -485,7 +505,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
        if ((tifp ^ tifn) & _TIF_NOCPUID)
                set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
 
-       __speculation_ctrl_update(tifp, tifn);
+       if (likely(!((tifp | tifn) & _TIF_SPEC_FORCE_UPDATE))) {
+               __speculation_ctrl_update(tifp, tifn);
+       } else {
+               speculation_ctrl_update_tif(prev_p);
+               tifn = speculation_ctrl_update_tif(next_p);
+
+               /* Enforce MSR update to ensure consistent state */
+               __speculation_ctrl_update(~tifn, tifn);
+       }
 }
 
 /*