Re: [PATCH 2/2] i386: Add notify VM exit support

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Mar 10, 2022 at 05:53:05PM +0800, Chenyi Qiang wrote:
> 
> 
> On 3/10/2022 5:17 PM, Daniel P. Berrangé wrote:
> > On Thu, Mar 10, 2022 at 05:02:05PM +0800, Chenyi Qiang wrote:
> > > There are cases that malicious virtual machine can cause CPU stuck (due
> > > to event windows don't open up), e.g., infinite loop in microcode when
> > > nested #AC (CVE-2015-5307). No event window means no event (NMI, SMI and
> > > IRQ) can be delivered. It leads the CPU to be unavailable to host or
> > > other VMs. Notify VM exit is introduced to mitigate such kind of
> > > attacks, which will generate a VM exit if no event window occurs in VM
> > > non-root mode for a specified amount of time (notify window).
> > > 
> > > A new KVM capability KVM_CAP_X86_NOTIFY_VMEXIT is exposed to user space
> > > so that the user can query the capability and set the expected notify
> > > window when creating VMs.
> > > 
> > > If notify VM exit happens with VM_INVALID_CONTEXT, hypervisor should
> > > exit to user space with the exit reason KVM_EXIT_NOTIFY to inform the
> > > fatal case. Then user space can inject a SHUTDOWN event to the target
> > > vcpu. This is implemented by defining a new bit in flags field of
> > > kvm_vcpu_event in KVM_SET_VCPU_EVENTS ioctl.
> > > 
> > > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> > > ---
> > >   hw/i386/x86.c         | 24 ++++++++++++++++++
> > >   include/hw/i386/x86.h |  3 +++
> > >   target/i386/kvm/kvm.c | 58 ++++++++++++++++++++++++++++---------------
> > >   3 files changed, 65 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > > index b84840a1bb..25e6c50b1e 100644
> > > --- a/hw/i386/x86.c
> > > +++ b/hw/i386/x86.c
> > > @@ -1309,6 +1309,23 @@ static void machine_set_sgx_epc(Object *obj, Visitor *v, const char *name,
> > >       qapi_free_SgxEPCList(list);
> > >   }
> > > +static void x86_machine_get_notify_window(Object *obj, Visitor *v,
> > > +                                const char *name, void *opaque, Error **errp)
> > > +{
> > > +    X86MachineState *x86ms = X86_MACHINE(obj);
> > > +    int32_t notify_window = x86ms->notify_window;
> > > +
> > > +    visit_type_int32(v, name, &notify_window, errp);
> > > +}
> > > +
> > > +static void x86_machine_set_notify_window(Object *obj, Visitor *v,
> > > +                               const char *name, void *opaque, Error **errp)
> > > +{
> > > +    X86MachineState *x86ms = X86_MACHINE(obj);
> > > +
> > > +    visit_type_int32(v, name, &x86ms->notify_window, errp);
> > > +}
> > > +
> > >   static void x86_machine_initfn(Object *obj)
> > >   {
> > >       X86MachineState *x86ms = X86_MACHINE(obj);
> > > @@ -1319,6 +1336,7 @@ static void x86_machine_initfn(Object *obj)
> > >       x86ms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
> > >       x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
> > >       x86ms->bus_lock_ratelimit = 0;
> > > +    x86ms->notify_window = -1;
> > >   }
> > 
> > IIUC from the kernel patch, this negative value leaves the protection
> > disabled, and thus the host remains vulnerable to the CVE. I would
> > expect this ought to set a suitable default value to fix the flaw.
> > 
> 
> Hum, I missed some explanation in commit message.
> We had some discussion about the default behavior of this feature. There are
> some concerns. e.g.
> There's a possibility, however small, that a notify VM exit happens
> with VM_CONTEXT_INVALID set in exit qualification, which means VM
> context is corrupted. To avoid the false positive and a well-behaved
> guest gets killed, we decide to make this feature opt-in.

That is unfortunate. It is not going to be much benefit to add this
feature, if users are discouraged from using it because of the danger
of it killing non-malicious guests :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|