linux-kernel - Re: [tip:x86/pti] x86/speculation: Use IBRS if available before calling into firmware

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: <1518438439.6606.36.camel@infradead.org>
Date:   Mon, 12 Feb 2018 12:27:19 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Mingarelli <thomas.mingarelli@....com>
Cc:     hpa@...or.com, tglx@...utronix.de, torvalds@...ux-foundation.org,
        linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org,
        Dave Hansen <dave@...1.net>, Borislav Petkov <bp@...en8.de>,
        Arjan van de Ven <arjan@...radead.org>
Subject: Re: [tip:x86/pti] x86/speculation: Use IBRS if available before
 calling into firmware

On Mon, 2018-02-12 at 12:50 +0100, Peter Zijlstra wrote:
> On Mon, Feb 12, 2018 at 11:22:11AM +0100, Ingo Molnar wrote:
> > > +static inline void firmware_restrict_branch_speculation_start(void)
> > > +{
> > > +   alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,
> > > +                         X86_FEATURE_USE_IBRS_FW);
> > > +}
> > > +
> > > +static inline void firmware_restrict_branch_speculation_end(void)
> > > +{
> > > +   alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,
> > > +                         X86_FEATURE_USE_IBRS_FW);
> > 
> > BTW., there's a detail that only occurred to me today, this enabling/disabling 
> > sequence is not NMI safe, and it might be called from NMI context:
> 
> Wait, we're doing firmware from NMI? That sounds like a _REALLY_ bad
> idea.

And spin_lock_irqsave() too. Which is probably why I missed the fact
that this was being called in NMI context.

Yay for HP and their persistent attempts to "value subtract" in their
firmware offerings.

I'm tempted to drop that part of the patch and declare that if you're
using this driver, the potential for stray branch prediction when you
call into the firmware from the NMI handler is the *least* of your
problems.

I *will* go back over the other parts of the patch and audit them for
preempt safety though; there could potentially be a similar issue
there. I think I put them close enough to the actual firmware calls
that if we aren't already preempt-safe then we were screwed anyway, but
*maybe* there's merit in making the macros explicitly bump the preempt
count anyway.
Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (5213 bytes)