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: <ZH4qBjLi0egsuC1D@google.com>
Date:   Mon, 5 Jun 2023 11:31:34 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
Cc:     Jon Kohler <jon@...anix.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
        Andrew Cooper <andrew.cooper3@...rix.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>,
        Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
        "kvm @ vger . kernel . org" <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: VMX: remove LFENCE in vmx_spec_ctrl_restore_host()

On Mon, Jun 05, 2023, Pawan Gupta wrote:
> On Mon, Jun 05, 2023 at 04:39:02PM +0000, Jon Kohler wrote:
> > >>> Yes.  Though in practice it might not make much of a difference.  With
> > >>> wrmsr+lfence, the lfence has nothing to do so it might be almost
> > >>> instantaneous anyway.
> > >>> 
> > >>> -- 
> > >>> Josh
> > >> 
> > >> Coming back to this, what if we hoisted call vmx_spec_ctrl_restore_host above
> > >> FILL_RETURN_BUFFER, and dropped this LFENCE as I did here?
> > >> 
> > >> That way, we wouldn’t have to mess with the internal LFENCE in nospec-branch.h,
> > >> and that would act as the “final line of defense” LFENCE.
> > >> 
> > >> Would that be acceptable? Or does FILL_RETURN_BUFFER *need* to occur
> > >> before any sort of calls no matter what?
> > > 
> > > If we go by Intel's statement that only unbalanced RETs are a concern,
> > > that *might* be ok as long as there's a nice comment above the
> > > FILL_RETURN_BUFFER usage site describing the two purposes for the
> > > LFENCE.
> 
> We would then need FILL_RETURN_BUFFER to unconditionally execute LFENCE
> to account for wrmsr branch misprediction. Currently LFENCE is not
> executed for !X86_BUG_EIBRS_PBRSB.
> 
> > > However, based on Andy's concerns, which I've discussed with him
> > > privately (but I'm not qualified to agree or disagree with), we may want
> > > to just convert vmx_spec_ctrl_restore_host() to asm.  Better safe than
> > > sorry.  My original implementation of that function was actually asm.  I
> > > can try to dig up that code.
> 
> Note:
> 
>   VMexit
>   CALL
>     RET
>   RET    <---- This is also a problem if the first call hasn't retired yet.
>   LFENCE
> 
> Converting vmx_spec_ctrl_restore_host() to ASM should be able to take care
> of this.

Is there an actual bug here, or are we just micro-optimizing something that may or
may not need additional optimization?  Unless there's a bug to be fixed, moving
code into ASM and increasing complexity doesn't seem worthwhile.