linux-kernel - Re: [PATCH v4 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.1809081058410.1402@nanos.tec.linutronix.de>
Date:   Sat, 8 Sep 2018 11:05:07 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Jiri Kosina <jikos@...nel.org>
cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        "Schaufler, Casey" <casey.schaufler@...el.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v4 2/2] x86/speculation: Enable cross-hyperthread spectre
 v2 STIBP mitigation

On Sat, 8 Sep 2018, Thomas Gleixner wrote:
> On Fri, 7 Sep 2018, Jiri Kosina wrote:
> > So I will go through the whole codepath again, but I fear your suggestion 
> > would not work -- see the check for cpu_smt_control in stibp_needed(). We 
> > need to see the old (or new, depending on the direction of the transition) 
> > value of cpu_smt_contol, which will break if we move 
> > arch_smt_enable_errata() (and thus the check).
> 
> That's bogus. The arch_smt_control(enable) function does not need to look
> at the SMT control state at all. The caller hands the not yet populated new
> state in and that's enough to make the decision whether to set or clear the
> bit in x86_spec_ctrl_base.

And thinking more about it, the function does not even need an argument.

smt_disable()

	res = shut_down_siblings();
	if (!res) {
		state = disabled;
		arch_smt_update();
	}

smt_enable()

	state = enabled;
	arch_smt_update();
	bringup_siblings();

So the update function has the correct state in both cases and then does:

	if (!cpu_is_trainwreck() || !cpu_has(stibp))
       		return;

	lock();
	ctrl = spec_ctrl_base;
	if (smt_disabled())
		ctrl &= ~STIPB;
	else
		ctrl |= STIPB;
	if (ctrl != spec_ctrl_base) {
		spec_ctrl_base = ctrl;
		on_each_cpu(write_spec_msr);
	}
	unlock();

See?

Thanks,

	tglx