[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <99FC4B6EFCEFD44486C35F4C281DC6732148DD1C@ORSMSX110.amr.corp.intel.com>
Date: Tue, 30 Oct 2018 22:02:44 +0000
From: "Schaufler, Casey" <casey.schaufler@...el.com>
To: Tim Chen <tim.c.chen@...ux.intel.com>,
Jiri Kosina <jikos@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>
CC: 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>,
"Hansen, Dave" <dave.hansen@...el.com>,
"Mallick, Asit K" <asit.k.mallick@...el.com>,
Arjan van de Ven <arjan@...ux.intel.com>,
Jon Masters <jcm@...hat.com>,
Waiman Long <longman9394@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>,
"Schaufler, Casey" <casey.schaufler@...el.com>
Subject: RE: [Patch v4 16/18] x86/speculation: Enable STIBP to protect
security sensitive tasks
> -----Original Message-----
> From: Tim Chen [mailto:tim.c.chen@...ux.intel.com]
> Sent: Tuesday, October 30, 2018 2:35 PM
> To: Schaufler, Casey <casey.schaufler@...el.com>; Jiri Kosina
> <jikos@...nel.org>; Thomas Gleixner <tglx@...utronix.de>
> Cc: 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>;
> Hansen, Dave <dave.hansen@...el.com>; Mallick, Asit K
> <asit.k.mallick@...el.com>; Arjan van de Ven <arjan@...ux.intel.com>; Jon
> Masters <jcm@...hat.com>; Waiman Long <longman9394@...il.com>;
> linux-kernel@...r.kernel.org; x86@...nel.org
> Subject: Re: [Patch v4 16/18] x86/speculation: Enable STIBP to protect security
> sensitive tasks
>
> On 10/30/2018 02:07 PM, Schaufler, Casey wrote:
> >> -----Original Message-----
> >> From: Tim Chen [mailto:tim.c.chen@...ux.intel.com]
> >> Sent: Tuesday, October 30, 2018 11:49 AM
> >> To: Jiri Kosina <jikos@...nel.org>; Thomas Gleixner <tglx@...utronix.de>
> >> Cc: Tim Chen <tim.c.chen@...ux.intel.com>; 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>; Hansen, Dave
> >> <dave.hansen@...el.com>; Schaufler, Casey <casey.schaufler@...el.com>;
> >> Mallick, Asit K <asit.k.mallick@...el.com>; Arjan van de Ven
> >> <arjan@...ux.intel.com>; Jon Masters <jcm@...hat.com>; Waiman Long
> >> <longman9394@...il.com>; linux-kernel@...r.kernel.org; x86@...nel.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.chen@...ux.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?
Better. On the other hand, adding function call layers just in case leads
to cascades of functions that do nothing but call other functions, and that
makes code hard to understand. I would leave generalization for the 2nd
person who wants to add mitigations.
>
> 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
> >
Powered by blists - more mailing lists