Skip to content

Commit 6d991ba

Browse files
committed
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 <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Andy Lutomirski <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Jiri Kosina <[email protected]> Cc: Tom Lendacky <[email protected]> Cc: Josh Poimboeuf <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: David Woodhouse <[email protected]> Cc: Tim Chen <[email protected]> Cc: Andi Kleen <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Casey Schaufler <[email protected]> Cc: Asit Mallick <[email protected]> Cc: Arjan van de Ven <[email protected]> Cc: Jon Masters <[email protected]> Cc: Waiman Long <[email protected]> Cc: Greg KH <[email protected]> Cc: Dave Stewart <[email protected]> Cc: Kees Cook <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected]
1 parent e6da8bb commit 6d991ba

File tree

4 files changed

+40
-18
lines changed

4 files changed

+40
-18
lines changed

arch/x86/include/asm/spec-ctrl.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ static inline void speculative_store_bypass_ht_init(void) { }
8383
#endif
8484

8585
extern void speculation_ctrl_update(unsigned long tif);
86-
87-
static inline void speculation_ctrl_update_current(void)
88-
{
89-
speculation_ctrl_update(current_thread_info()->flags);
90-
}
86+
extern void speculation_ctrl_update_current(void);
9187

9288
#endif

arch/x86/include/asm/thread_info.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ struct thread_info {
8484
#define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */
8585
#define TIF_SECCOMP 8 /* secure computing */
8686
#define TIF_SPEC_IB 9 /* Indirect branch speculation mitigation */
87+
#define TIF_SPEC_FORCE_UPDATE 10 /* Force speculation MSR update in context switch */
8788
#define TIF_USER_RETURN_NOTIFY 11 /* notify kernel of userspace return */
8889
#define TIF_UPROBE 12 /* breakpointed or singlestepping */
8990
#define TIF_PATCH_PENDING 13 /* pending live patching update */
@@ -112,6 +113,7 @@ struct thread_info {
112113
#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
113114
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
114115
#define _TIF_SPEC_IB (1 << TIF_SPEC_IB)
116+
#define _TIF_SPEC_FORCE_UPDATE (1 << TIF_SPEC_FORCE_UPDATE)
115117
#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY)
116118
#define _TIF_UPROBE (1 << TIF_UPROBE)
117119
#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -149,7 +151,7 @@ struct thread_info {
149151
/* flags to check in __switch_to() */
150152
#define _TIF_WORK_CTXSW_BASE \
151153
(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \
152-
_TIF_SSBD)
154+
_TIF_SSBD | _TIF_SPEC_FORCE_UPDATE)
153155

154156
/*
155157
* Avoid calls to __switch_to_xtra() on UP as STIBP is not evaluated.

arch/x86/kernel/cpu/bugs.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -702,14 +702,10 @@ static void ssb_select_mitigation(void)
702702
#undef pr_fmt
703703
#define pr_fmt(fmt) "Speculation prctl: " fmt
704704

705-
static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
705+
static void task_update_spec_tif(struct task_struct *tsk)
706706
{
707-
bool update;
708-
709-
if (on)
710-
update = !test_and_set_tsk_thread_flag(tsk, tifbit);
711-
else
712-
update = test_and_clear_tsk_thread_flag(tsk, tifbit);
707+
/* Force the update of the real TIF bits */
708+
set_tsk_thread_flag(tsk, TIF_SPEC_FORCE_UPDATE);
713709

714710
/*
715711
* Immediately update the speculation control MSRs for the current
@@ -719,7 +715,7 @@ static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
719715
* This can only happen for SECCOMP mitigation. For PRCTL it's
720716
* always the current task.
721717
*/
722-
if (tsk == current && update)
718+
if (tsk == current)
723719
speculation_ctrl_update_current();
724720
}
725721

@@ -735,16 +731,16 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
735731
if (task_spec_ssb_force_disable(task))
736732
return -EPERM;
737733
task_clear_spec_ssb_disable(task);
738-
task_update_spec_tif(task, TIF_SSBD, false);
734+
task_update_spec_tif(task);
739735
break;
740736
case PR_SPEC_DISABLE:
741737
task_set_spec_ssb_disable(task);
742-
task_update_spec_tif(task, TIF_SSBD, true);
738+
task_update_spec_tif(task);
743739
break;
744740
case PR_SPEC_FORCE_DISABLE:
745741
task_set_spec_ssb_disable(task);
746742
task_set_spec_ssb_force_disable(task);
747-
task_update_spec_tif(task, TIF_SSBD, true);
743+
task_update_spec_tif(task);
748744
break;
749745
default:
750746
return -ERANGE;

arch/x86/kernel/process.c

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,18 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,
443443
wrmsrl(MSR_IA32_SPEC_CTRL, msr);
444444
}
445445

446+
static unsigned long speculation_ctrl_update_tif(struct task_struct *tsk)
447+
{
448+
if (test_and_clear_tsk_thread_flag(tsk, TIF_SPEC_FORCE_UPDATE)) {
449+
if (task_spec_ssb_disable(tsk))
450+
set_tsk_thread_flag(tsk, TIF_SSBD);
451+
else
452+
clear_tsk_thread_flag(tsk, TIF_SSBD);
453+
}
454+
/* Return the updated threadinfo flags*/
455+
return task_thread_info(tsk)->flags;
456+
}
457+
446458
void speculation_ctrl_update(unsigned long tif)
447459
{
448460
/* Forced update. Make sure all relevant TIF flags are different */
@@ -451,6 +463,14 @@ void speculation_ctrl_update(unsigned long tif)
451463
preempt_enable();
452464
}
453465

466+
/* Called from seccomp/prctl update */
467+
void speculation_ctrl_update_current(void)
468+
{
469+
preempt_disable();
470+
speculation_ctrl_update(speculation_ctrl_update_tif(current));
471+
preempt_enable();
472+
}
473+
454474
void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
455475
{
456476
struct thread_struct *prev, *next;
@@ -482,7 +502,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
482502
if ((tifp ^ tifn) & _TIF_NOCPUID)
483503
set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
484504

485-
__speculation_ctrl_update(tifp, tifn);
505+
if (likely(!((tifp | tifn) & _TIF_SPEC_FORCE_UPDATE))) {
506+
__speculation_ctrl_update(tifp, tifn);
507+
} else {
508+
speculation_ctrl_update_tif(prev_p);
509+
tifn = speculation_ctrl_update_tif(next_p);
510+
511+
/* Enforce MSR update to ensure consistent state */
512+
__speculation_ctrl_update(~tifn, tifn);
513+
}
486514
}
487515

488516
/*

0 commit comments

Comments
 (0)