lkml.org 
[lkml]   [2018]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Patch v4 16/18] x86/speculation: Enable STIBP to protect security sensitive tasks
From
Date
On 10/30/2018 02:07 PM, Schaufler, Casey wrote:
>> -----Original Message-----
>> From: Tim Chen [mailto:tim.c.chen@linux.intel.com]
>> Sent: Tuesday, October 30, 2018 11:49 AM
>> To: Jiri Kosina <jikos@kernel.org>; Thomas Gleixner <tglx@linutronix.de>
>> Cc: Tim Chen <tim.c.chen@linux.intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Ingo Molnar <mingo@redhat.com>; Peter
>> Zijlstra <peterz@infradead.org>; Josh Poimboeuf <jpoimboe@redhat.com>;
>> Andrea Arcangeli <aarcange@redhat.com>; David Woodhouse
>> <dwmw@amazon.co.uk>; Andi Kleen <ak@linux.intel.com>; Hansen, Dave
>> <dave.hansen@intel.com>; Schaufler, Casey <casey.schaufler@intel.com>;
>> Mallick, Asit K <asit.k.mallick@intel.com>; Arjan van de Ven
>> <arjan@linux.intel.com>; Jon Masters <jcm@redhat.com>; Waiman Long
>> <longman9394@gmail.com>; linux-kernel@vger.kernel.org; x86@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.chen@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
>

\
 
 \ /
  Last update: 2018-10-30 22:35    [W:0.268 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site