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:   Fri, 29 Apr 2022 23:23:32 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Borislav Petkov <bp@alien8.de>
Cc:     Jon Kohler <jon@nutanix.com>, 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: <YmxzdAbzJkvjXSAU@google.com>
References: <20220422162103.32736-1-jon@nutanix.com>
 <YmwZYEGtJn3qs0j4@zn.tnic>
 <645E4ED5-F6EE-4F8F-A990-81F19ED82BFA@nutanix.com>
 <Ymw9UZDpXym2vXJs@zn.tnic>
 <YmxKqpWFvdUv+GwJ@google.com>
 <YmxRnwSUBIkOIjLA@zn.tnic>
 <Ymxf2Jnmz5y4CHFN@google.com>
 <YmxlHBsxcIy8uYaB@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YmxlHBsxcIy8uYaB@zn.tnic>
Precedence: bulk

On Sat, Apr 30, 2022, Borislav Petkov wrote:
> On Fri, Apr 29, 2022 at 09:59:52PM +0000, Sean Christopherson wrote:
> > Correct, but KVM also doesn't do IBPB on VM-Exit (or VM-Entry),
> 
> Why doesn't it do that? Not needed?

The host kernel is protected via RETPOLINE and by flushing the RSB immediately
after VM-Exit.

> > nor does KVM do IBPB before exiting to userspace.
> 
> Same question.

I don't know definitively.  My guess is that IBPB is far too costly to do on every
exit, and so the onus was put on userspace to recompile with RETPOLINE.  What I
don't know is why it wasn't implemented as an opt-out feature.

> > The IBPB we want to whack is issued only when KVM is switching vCPUs.
> 
> Then please document it properly as I've already requested.

I'll write up the bits I have my head wrapped around.

> > Except that _none_ of that documentation explains why the hell KVM
> > does IBPB when switching betwen vCPUs.
> 
> Probably because the folks involved in those patches weren't the hell
> mainly virt people. Although I see a bunch of virt people on CC on that
> patch.
> 
> >   : But stepping back, why does KVM do its own IBPB in the first place? ?The goal is
> >   : to prevent one vCPU from attacking the next vCPU run on the same pCPU. ?But unless
> >   : userspace is running multiple VMs in the same process/mm_struct, switching vCPUs,
> >   : i.e. switching tasks, will also switch mm_structs and thus do IPBP via cond_mitigation.
> >   :
> >   : If userspace runs multiple VMs in the same process,
> 
> This keeps popping up. Who does that? Can I get a real-life example to
> such VM-based containers or what the hell that is, pls?

I don't know of any actual examples.  But, it's trivially easy to create multiple
VMs in a single process, and so proving the negative that no one runs multiple VMs
in a single address space is essentially impossible.

The container thing is just one scenario I can think of where userspace might
actually benefit from sharing an address space, e.g. it would allow backing the
image for large number of VMs with a single set of read-only VMAs.

> > enables cond_ipbp, _and_ sets
> >   : TIF_SPEC_IB, then it's being stupid and isn't getting full protection in any case,
> >   : e.g. if userspace is handling an exit-to-userspace condition for two vCPUs from
> >   : different VMs, then the kernel could switch between those two vCPUs' tasks without
> >   : bouncing through KVM and thus without doing KVM's IBPB.
> >   :
> >   : I can kinda see doing this for always_ibpb, e.g. if userspace is unaware of spectre
> >   : and is naively running multiple VMs in the same process.
> 
> So this needs a clearer definition: what protection are we even talking
> about when the address spaces of processes are shared? My na?ve
> thinking would be: none. They're sharing address space - branch pred.
> poisoning between the two is the least of their worries.

I truly have no idea, which is part of the reason I brought it up in the first
place.  I'd have happily just whacked KVM's IBPB entirely, but it seemed prudent
to preserve the existing behavior if someone went out of their way to enable
switch_mm_always_ibpb.

> So to cut to the chase: it sounds to me like you don't want to do IBPB
> at all on vCPU switch.

Yes, or do it iff switch_mm_always_ibpb is enabled to maintain "compability".

> And the process switch case is taken care of by switch_mm().

Yep.