| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 6 Nov 2018 16:18:17 -0800
From: Tim Chen <tim.c.chen@...ux.intel.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Jiri Kosina <jikos@...nel.org>,
Tom Lendacky <thomas.lendacky@....com>,
Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Andrea Arcangeli <aarcange@...hat.com>,
David Woodhouse <dwmw@...zon.co.uk>,
Andi Kleen <ak@...ux.intel.com>,
Dave Hansen <dave.hansen@...el.com>,
Casey Schaufler <casey.schaufler@...el.com>,
Asit Mallick <asit.k.mallick@...el.com>,
Arjan van de Ven <arjan@...ux.intel.com>,
Jon Masters <jcm@...hat.com>,
Waiman Long <longman9394@...il.com>,
LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
Kees Cook <keescook@...omium.org>
Subject: Re: [Patch v4 17/18] x86/speculation: Update SPEC_CTRL MSRs of remote
CPUs
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)
{
Powered by blists - more mailing lists