linux-kernel - Re: [PATCH] KVM: VMX: do not disable interception for MSR_IA32_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: <Yof6ZE0IjSAL+iO8@google.com>
Date:   Fri, 20 May 2022 20:30:28 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Jon Kohler <jon@...anix.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Waiman Long <longman@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: VMX: do not disable interception for
 MSR_IA32_SPEC_CTRL on eIBRS

On Fri, May 20, 2022, Jon Kohler wrote:
> 
> 
> > On May 20, 2022, at 4:06 PM, Sean Christopherson <seanjc@...gle.com> wrote:
> > 
> > On Fri, May 20, 2022, Jon Kohler wrote:
> >> 
> >>> On May 18, 2022, at 10:23 AM, Jon Kohler <jon@...anix.com> wrote:
> >>> 
> >>>> On May 17, 2022, at 9:42 PM, Sean Christopherson <seanjc@...gle.com> wrote:
> >>>>> +		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) && data == BIT(0)) {
> >>>> 
> >>>> Use SPEC_CTRL_IBRS instead of open coding "BIT(0)", then a chunk of the comment
> >>>> goes away.
> >>>> 
> >>>>> +			vmx->spec_ctrl = data;
> >>>>> +			break;
> >>>>> +		}
> >>>> 
> >>>> There's no need for a separate if statement.  And the boot_cpu_has() check can
> >>>> be dropped, kvm_spec_ctrl_test_value() has already verified the bit is writable
> >>>> (unless you're worried about bit 0 being used for something else?)
> >> 
> >> I was (and am) worried about misbehaving guests on pre-eIBRS systems spamming IBRS
> >> MSR, which we wouldn’t be able to see today. Intel’s guidance for eIBRS has long been
> >> set it once and be done with it, so any eIBRS aware guest should behave nicely with that.
> >> That limits the blast radius a bit here.
> > 
> > Then check the guest capabilities, not the host flag.
> > 
> > 	if (data == SPEC_CTRL_IBRS &&
> > 	    (vcpu->arch.arch_capabilities & ARCH_CAP_IBRS_ALL))
> 
> So I originally did that in my first internal patch; however, the code you wrote is
> effectively the code I wrote, because cpu_set_bug_bits() already does that exact
> same thing when it sets up X86_FEATURE_IBRS_ENHANCED. 
> 
> Is the boot cpu check more expensive than checking the vCPU perhaps? Otherwise,
> checking X86_FEATURE_IBRS_ENHANCED seemed like it might be easier
> understand for future onlookers, as thats what the rest of the kernel keys off of
> when checking for eIBRS (e.g. in bugs.c etc). 

Cost is irrelevant, checking X86_FEATURE_IBRS_ENHANCED is simply wrong.  Just
because eIBRS is supported in the host doesn't mean it's advertised to the guest,
e.g. an older VM could have been created without eIBRS and then migrated to a host
that does support eIBRS.  Now you have a guest that thinks it needs to constantly
toggle IBRS (I assume that's the pre-eIBRS behavior), but by looking at the _host_
value KVM would assume it's a one-time write and not disable interception.