On 10/30/2018 02:07 PM, Schaufler, Casey wrote: >> -----Original Message----- >> From: Tim Chen [mailto:tim.c.c...@linux.intel.com] >> Sent: Tuesday, October 30, 2018 11:49 AM >> To: Jiri Kosina <ji...@kernel.org>; Thomas Gleixner <t...@linutronix.de> >> Cc: Tim Chen <tim.c.c...@linux.intel.com>; Tom Lendacky >> <thomas.lenda...@amd.com>; Ingo Molnar <mi...@redhat.com>; Peter >> Zijlstra <pet...@infradead.org>; Josh Poimboeuf <jpoim...@redhat.com>; >> Andrea Arcangeli <aarca...@redhat.com>; David Woodhouse >> <d...@amazon.co.uk>; Andi Kleen <a...@linux.intel.com>; Hansen, Dave >> <dave.han...@intel.com>; Schaufler, Casey <casey.schauf...@intel.com>; >> Mallick, Asit K <asit.k.mall...@intel.com>; Arjan van de Ven >> <ar...@linux.intel.com>; Jon Masters <j...@redhat.com>; Waiman Long >> <longman9...@gmail.com>; linux-kernel@vger.kernel.org; x...@kernel.org >> Subject: [Patch v4 16/18] x86/speculation: Enable STIBP to protect security >> sensitive tasks >> >> Enable STIBP defense on high security tasks. >> >> For normal tasks, STIBP is unused so they are not impacted by overhead >> from STIBP in lite protection mode. >> >> Signed-off-by: Tim Chen <tim.c.c...@linux.intel.com> >> --- >> arch/x86/kernel/cpu/bugs.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c >> index 54f4675..b402b96 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/security.h> >> >> #include <asm/spec-ctrl.h> >> #include <asm/cmdline.h> >> @@ -770,6 +772,37 @@ 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 (!static_branch_unlikely(&spectre_v2_app_lite)) >> + return; >> + >> + 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 (!update) >> + return; >> + >> + if (tsk == current) >> + speculation_ctrl_update_current(); >> +} >> + >> +void arch_set_security(struct task_struct *tsk, unsigned int value) > > In this context "security" isn't descriptive. arch_set_stibp_defenses() > would be better.
A more generic name decoupled from STIBP will be preferable. There can other kind of security defenses to be erected in the future. Perhaps arch_set_mitigation? Thanks. Tim > > Since "value" should only ever have one of two values, and those > map directly to "true" or "false" this should be a bool, making the > code trivial: > > void arch_set_stibp_defenses(struct task_struct *task, bool stibp) > { > set_task_stibp(task, stibp); > } > > Or perhaps arch_set_security() should go away, and the calling > code would call set_task_stibp() directly. Unless there is some compelling > reason for the abstractions. > >> +{ >> + if (value > SECURITY_HIGH) >> + return; >> + >> + /* Update STIBP defenses */ >> + if (value == SECURITY_HIGH) >> + set_task_stibp(tsk, true); >> + else >> + set_task_stibp(tsk, false); >> +} >> + >> int arch_prctl_spec_ctrl_set(struct task_struct *task, unsigned long which, >> unsigned long ctrl) >> { >> -- >> 2.9.4 >