Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 10 May 2022 15:50:31 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Jon Kohler <jon@nutanix.com>
Cc:     Borislav Petkov <bp@alien8.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Balbir Singh <sblbir@amazon.com>,
        Kim Phillips <kim.phillips@amd.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Kees Cook <keescook@chromium.org>,
        Waiman Long <longman@redhat.com>
Subject: Re: [PATCH v3] x86/speculation, KVM: only IBPB for
 switch_mm_always_ibpb on vCPU load
Message-ID: <YnqJx/5hos0lKqI9@google.com>
References: <YmxKqpWFvdUv+GwJ@google.com>
 <YmxRnwSUBIkOIjLA@zn.tnic>
 <Ymxf2Jnmz5y4CHFN@google.com>
 <YmxlHBsxcIy8uYaB@zn.tnic>
 <YmxzdAbzJkvjXSAU@google.com>
 <Ym0GcKhPZxkcMCYp@zn.tnic>
 <4E46337F-79CB-4ADA-B8C0-009E7500EDF8@nutanix.com>
 <Ym1fGZIs6K7T6h3n@zn.tnic>
 <Ynp6ZoQUwtlWPI0Z@google.com>
 <520D7CBE-55FA-4EB9-BC41-9E8D695334D1@nutanix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <520D7CBE-55FA-4EB9-BC41-9E8D695334D1@nutanix.com>
Precedence: bulk

On Tue, May 10, 2022, Jon Kohler wrote:
> 
> > On May 10, 2022, at 10:44 AM, Sean Christopherson <seanjc@google.com> wrote:
> > 
> > On Sat, Apr 30, 2022, Borislav Petkov wrote:
> >> But I'm likely missing a virt aspect here so I'd let Sean explain what
> >> the rules are...
> > 
> > I don't think you're missing anything.  I think the original 15d45071523d ("KVM/x86:
> > Add IBPB support") was simply wrong.
> > 
> > As I see it:
> > 
> >  1. If the vCPUs belong to the same VM, they are in the same security domain and
> >     do not need an IPBP.
> > 
> >  2. If the vCPUs belong to different VMs, and each VM is in its own mm_struct,
> >     defer to switch_mm_irqs_off() to handle IBPB as an mm switch is guaranteed to
> >     occur prior to loading a vCPU belonging to a different VMs.
> > 
> >  3. If the vCPUs belong to different VMs, but multiple VMs share an mm_struct,
> >     then the security benefits of an IBPB when switching vCPUs are dubious, at best.
> > 
> > If we only consider #1 and #2, then KVM doesn't need an IBPB, period.
> > 
> > #3 is the only one that's a grey area.  I have no objection to omitting IBPB entirely
> > even in that case, because none of us can identify any tangible value in doing so.
> 
> Thanks, Sean. Our messages crossed in flight, I sent a reply to your earlier message
> just a bit ago. This is super helpful to frame this up.
> 
> What would you think framing the patch like this:
> 
>     x86/speculation, KVM: remove IBPB on vCPU load
> 
>     Remove IBPB that is done on KVM vCPU load, as the guest-to-guest
>     attack surface is already covered by switch_mm_irqs_off() ->
>     cond_mitigation().
> 
>     The original 15d45071523d ("KVM/x86: Add IBPB support") was simply wrong in
>     its guest-to-guest design intention. There are three scenarios at play
>     here:
> 
>     1. If the vCPUs belong to the same VM, they are in the same security 
>     domain and do not need an IPBP.
>     2. If the vCPUs belong to different VMs, and each VM is in its own mm_struct,
>     switch_mm_irqs_off() will handle IBPB as an mm switch is guaranteed to
>     occur prior to loading a vCPU belonging to a different VMs.
>     3. If the vCPUs belong to different VMs, but multiple VMs share an mm_struct,
>     then the security benefits of an IBPB when switching vCPUs are dubious, 
>     at best.
> 
>     Issuing IBPB from KVM vCPU load would only cover #3, but there are no

Just to hedge, there are no _known_ use cases.

>     real world tangible use cases for such a configuration.

and I would further qualify this with:

      but there are no known real world, tangible use cases for running multiple
      VMs belonging to different security domains in a shared address space.

Running multiple VMs in a single address space is plausible and sane, _if_ they
are all in the same security domain or security is not a concern.  That way the
statement isn't invalidated if someone pops up with a use case for running multiple
VMs but has no security story.

Other than that, LGTM.

>     If multiple VMs
>     are sharing an mm_structs, prediction attacks are the least of their
>     security worries.
> 
>     Fixes: 15d45071523d ("KVM/x86: Add IBPB support")
>     (Reviewedby/signed of by people here)
>     (Code change simply whacks IBPB in KVM vmx/svm and thats it)
> 
> 
>