linux-kernel - Re: [Patch v4 17/18] x86/speculation: Update SPEC

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ea646842-bd32-3615-bf38-27b568a2d81e@linux.intel.com>
Date:   Mon, 5 Nov 2018 14:02:59 -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>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [Patch v4 17/18] x86/speculation: Update SPEC_CTRL MSRs of remote
 CPUs


Thomas,

>> +	if (tifp & _TIF_UPDATE_SPEC_CTRL)
>> +		clear_tsk_thread_flag(prev_p, TIF_UPDATE_SPEC_CTRL);
>> +
>> +	if (tifn & _TIF_UPDATE_SPEC_CTRL) {
>> +		clear_tsk_thread_flag(next_p, TIF_UPDATE_SPEC_CTRL);
>> +		tifn &= ~_TIF_UPDATE_SPEC_CTRL;
>> +	}
> 
> I'm really unhappy about adding yet more conditionals into this code
> path. We really need to find some better solution for that.
> 
> There are basically two options:
> 
>  1) Restrict the PRCTL control so it is only possible to modify it at the
>     point where the application is still single threaded.
> 
>  2) Add _TIF_UPDATE_SPEC_CTRL to the SYSCALL_EXIT_WORK_FLAGS and handle it
>     in the slow work path.
> 
>     The KVM side can be handled in x86_virt_spec_ctrl().
> 

How about sending an IPI if a remote CPU needs to have its SPEC_CTRL MSR
updated?

Something like the following to replace this patch?

Tim

---

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index b8103991..7505925 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -772,9 +772,15 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
 	return 0;
 }
 
+static void spec_ctrl_update_func(void *info)
+{
+	speculation_ctrl_update(task_thread_info(current)->flags);
+}
+
 static void set_task_stibp(struct task_struct *tsk, bool stibp_on)
 {
 	bool update = false;
+	int cpu;
 
 	if (!static_branch_unlikely(&spectre_v2_app_lite))
 		return;
@@ -789,6 +795,12 @@ static void set_task_stibp(struct task_struct *tsk, bool stibp_on)
 
 	if (tsk == current)
 		speculation_ctrl_update_current();
+	else {
+		cpu = task_cpu(tsk);
+		if (cpu != smp_processor_id())
+			smp_call_function_single(cpu, spec_ctrl_update_func,
+						 NULL, false); 
+	}
 }
 
 void arch_set_security(struct task_struct *tsk, unsigned int value)