Re: [Patch v4 17/18] x86/speculation: Update SPEC_CTRL MSRs of remote CPUs

[Tim Chen <tim.c.chen@linux.intel.com>]

Thomas,

> 
>>> 2) Add _TIF_UPDATE_SPEC_CTRL to the SYSCALL_EXIT_WORK_FLAGS and handle it
>>>    in the slow work path.
>>
>> There can be tasks that don't do any syscalls, and it seems like we can
>> have MSRs getting out of sync?
> 
> Setting the TIF flag directly in a remote task is wrong. It needs to be
> handled when the _TIF_UPDATE_SPEC_CTRL is evaluated, i.e. the information
> needs to be stored process wide e.g. in task->mm.
> 
> But yes, if the remote task runs in user space forever, it won't
> help. Though the point is that dumpable is usually set when the process
> starts, so it's probably mostly a theoretical issue.
> 

I took a crack to implement what you suggested to update
remote task's flag and remote SPEC_CTRL MSR on the syscall exit slow path.

This looks reasobale?

Tim


------------

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3b2490b..614594a 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -216,7 +216,7 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 
 #define SYSCALL_EXIT_WORK_FLAGS				\
 	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT |	\
-	 _TIF_SINGLESTEP | _TIF_SYSCALL_TRACEPOINT)
+	 _TIF_SINGLESTEP | _TIF_SYSCALL_TRACEPOINT | _TIF_UPDATE_SPEC_CTRL)
 
 static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
 {
@@ -227,6 +227,8 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
 	if (cached_flags & _TIF_SYSCALL_TRACEPOINT)
 		trace_sys_exit(regs, regs->ax);
 
+	if (cached_flags & _TIF_UPDATE_SPEC_CTRL)
+		spec_ctrl_do_pending_update();
 	/*
 	 * If TIF_SYSCALL_EMU is set, we only get here because of
 	 * TIF_SINGLESTEP (i.e. this is PTRACE_SYSEMU_SINGLESTEP).
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index c59a6c4..f124597 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -276,6 +276,8 @@ static inline void indirect_branch_prediction_barrier(void)
 	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
 }
 
+void spec_ctrl_do_pending_update(void);
+
 /* The Intel SPEC CTRL MSR base value cache */
 extern u64 x86_spec_ctrl_base;
 
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 4f6a7a9..b78db59 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -97,6 +97,7 @@ struct thread_info {
 #define TIF_USER_RETURN_NOTIFY	14	/* Notify kernel of userspace return */
 #define TIF_PATCH_PENDING	15	/* Pending live patching update */
 #define TIF_FSCHECK		16	/* Check FS is USER_DS on return */
+#define TIF_UPDATE_SPEC_CTRL	17	/* Pending update of speculation control */
 
 /* Task status */
 #define TIF_UPROBE		18	/* Breakpointed or singlestepping */
@@ -131,6 +132,7 @@ struct thread_info {
 #define _TIF_USER_RETURN_NOTIFY	(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_PATCH_PENDING	(1 << TIF_PATCH_PENDING)
 #define _TIF_FSCHECK		(1 << TIF_FSCHECK)
+#define _TIF_UPDATE_SPEC_CTRL	(1 << TIF_UPDATE_SPEC_CTRL)
 
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
 #define _TIF_MEMDIE		(1 << TIF_MEMDIE)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4c15c86..d82d3f8 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -14,6 +14,8 @@
 #include <linux/module.h>
 #include <linux/nospec.h>
 #include <linux/prctl.h>
+#include <linux/sched/coredump.h>
+#include <linux/sched/signal.h>
 
 #include <asm/spec-ctrl.h>
 #include <asm/cmdline.h>
@@ -770,6 +772,69 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
 	return 0;
 }
 
+static void set_task_stibp(struct task_struct *tsk, bool stibp_on)
+{
+	bool update = false;
+
+	if (stibp_on)
+		update = !test_and_set_tsk_thread_flag(tsk, TIF_STIBP);
+	else
+		update = test_and_clear_tsk_thread_flag(tsk, TIF_STIBP);
+
+	if (tsk == current && update)
+		speculation_ctrl_update_current();
+}
+
+void spec_ctrl_do_pending_update(void)
+{
+	if (!static_branch_unlikely(&spectre_v2_app_lite))
+		return;
+
+	if (!current->mm)
+		return;
+
+	if (get_dumpable(current->mm) != SUID_DUMP_USER)
+		set_tsk_thread_flag(current, TIF_STIBP);
+	else
+		clear_tsk_thread_flag(current, TIF_STIBP);
+
+	clear_tsk_thread_flag(current, TIF_UPDATE_SPEC_CTRL);
+	speculation_ctrl_update_current();
+}
+ 
+int arch_update_spec_ctrl_restriction(struct task_struct *task)
+{
+	unsigned long flags;
+	struct task_struct *t;
+	bool stibp_on = false;
+
+	if (!static_branch_unlikely(&spectre_v2_app_lite))
+		return 0;
+
+	if (!task->mm)
+		return -EINVAL;
+
+	if (!lock_task_sighand(task, &flags))
+		return -ESRCH;
+
+	if (get_dumpable(task->mm) != SUID_DUMP_USER)
+		stibp_on = true;
+
+	for_each_thread(task, t) {
+		if (task_cpu(task) == smp_processor_id())
+			set_task_stibp(task, stibp_on);
+		else if (test_tsk_thread_flag(task, TIF_STIBP) != stibp_on) 
+			set_tsk_thread_flag(task, TIF_UPDATE_SPEC_CTRL);
+	}
+
+	unlock_task_sighand(task, &flags);
+	return 0;
+}
+
 int arch_prctl_spec_ctrl_set(struct task_struct *task, unsigned long which,
 			     unsigned long ctrl)
 {