Re: [PATCH] KVM: nVMX: Honor userspace MSR filter lists for nested VM-Enter/VM-Exit

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 23, 2024, Hou Wenlong wrote:
> On Tue, Jul 23, 2024 at 07:59:22AM +0800, Sean Christopherson wrote:
> > ---
> > 
> > I found this by inspection when backporting Hou's change to an internal kernel.
> > I don't love piggybacking Intel's "you can't touch these special MSRs" behavior,
> > but ignoring the userspace MSR filters is far worse IMO.  E.g. if userspace is
> > denying access to an MSR in order to reduce KVM's attack surface, letting L1
> > sneak in reads/writes through VM-Enter/VM-Exit completely circumvents the
> > filters.
> > 
> >  Documentation/virt/kvm/api.rst  | 19 ++++++++++++++++---
> >  arch/x86/include/asm/kvm_host.h |  2 ++
> >  arch/x86/kvm/vmx/nested.c       | 12 ++++++------
> >  arch/x86/kvm/x86.c              |  6 ++++--
> >  4 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 8e5dad80b337..e6b1e42186f3 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -4226,9 +4226,22 @@ filtering. In that mode, ``KVM_MSR_FILTER_DEFAULT_DENY`` is invalid and causes
> >  an error.
> >  
> >  .. warning::
> > -   MSR accesses as part of nested VM-Enter/VM-Exit are not filtered.
> > -   This includes both writes to individual VMCS fields and reads/writes
> > -   through the MSR lists pointed to by the VMCS.
> > +   MSR accesses that are side effects of instruction execution (emulated or
> > +   native) are not filtered as hardware does not honor MSR bitmaps outside of
> > +   RDMSR and WRMSR, and KVM mimics that behavior when emulating instructions
> > +   to avoid pointless divergence from hardware.  E.g. RDPID reads MSR_TSC_AUX,
> > +   SYSENTER reads the SYSENTER MSRs, etc.
> > +
> > +   MSRs that are loaded/stored via dedicated VMCS fields are not filtered as
> > +   part of VM-Enter/VM-Exit emulation.
> > +
> > +   MSRs that are loaded/store via VMX's load/store lists _are_ filtered as part
> > +   of VM-Enter/VM-Exit emulation.  If an MSR access is denied on VM-Enter, KVM
> > +   synthesizes a consistency check VM-Exit(EXIT_REASON_MSR_LOAD_FAIL).  If an
> > +   MSR access is denied on VM-Exit, KVM synthesizes a VM-Abort.  In short, KVM
> > +   extends Intel's architectural list of MSRs that cannot be loaded/saved via
> > +   the VM-Enter/VM-Exit MSR list.  It is platform owner's responsibility to
> > +   to communicate any such restrictions to their end users.
> >
> Do we also need to modify the statement before this warning?

Yeah, that's a good idea.

While you're here, did you have a use case that is/was affected by the current
VM-Enter/VM-Exit vs. MSR filtering behavior?

> Since the behaviour is different from RDMSR/WRMSR emulation case.
> 
> ```
> if an MSR access is denied by userspace the resulting KVM behavior depends on
> whether or not KVM_CAP_X86_USER_SPACE_MSR's KVM_MSR_EXIT_REASON_FILTER is
> enabled.  If KVM_MSR_EXIT_REASON_FILTER is enabled, KVM will exit to userspace
> on denied accesses, i.e. userspace effectively intercepts the MSR access.
> ```