linux-kernel - Re: [PATCH] KVM: VMX: remove LFENCE in vmx_spec_ctrl_restore

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: <20230601004202.63yulqs73kuh3ep6@treble>
Date:   Wed, 31 May 2023 17:42:02 -0700
From:   Josh Poimboeuf <jpoimboe@...nel.org>
To:     Jon Kohler <jon@...anix.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
        "kvm @ vger . kernel . org" <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Cooper <andrew.cooper3@...rix.com>
Subject: Re: [PATCH] KVM: VMX: remove LFENCE in vmx_spec_ctrl_restore_host()

On Wed, May 31, 2023 at 11:58:12PM +0000, Jon Kohler wrote:
> > The goal of this barrier_nospec() is to prevent speculative execution
> > from bypassing the SPEC_CTRL write (due to misprediction of the
> > conditional branch, Spectre v1 style).  Otherwise the next indirect
> > branch or unbalanced RET could be an attack target.
> > 
> > So any previous LFENCEs before that conditional branch won't help here.
> 
> Ah interesting. Ok, to be clear, thats a guest -> host attack, correct? And such
> an attack would not at all be thwarted by the first CALL retire + LFENCE that
> was added on commit 2b1299322016 ("x86/speculation: Add RSB VM Exit 
> protections”)?

Right.

> Sorry to be long winded, just wanting to triple check because
> the aforementioned commit was added slightly after the original one, and I 
> want to make extra sure that they aren’t solving the same thing.
> 
> If that is indeed the case, does that commit need to be revisited at all?
> 
> Or are we saying that this Intel vulnerability needs *two* LFENCE’s to keep
> the host secure?

The first LFENCE (FILL_RETURN_BUFFER) forces the CALL to retire so the
RSB stuff is guaranteed to take effect before the next unbalanced RET
can be speculatively executed.

The second LFENCE (vmx_spec_ctrl_restore_host) forces the conditional
branch to retire so the SPEC_CTRL write (potential IBRS/eIBRS
enablement) is guaranteed to take effect before the next indirect branch
and/or unbalanced RET can be speculatively executed.

So each LFENCE has a distinct purpose.  That said, there are no indirect
branches or unbalanced RETs between them.  So it should be fine to
combine them into a single LFENCE after both.

You could for example just remove the first LFENCE.  But only for that
usage site, i.e. not for other users of FILL_RETURN_BUFFER.

Or, remove them both and add an LFENCE in vmx_vmexit() right after the
call to vmx_spec_ctrl_restore_host().  That might be clearer.  Then it
could have a comment describing its dual purposes.

-- 
Josh