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

[Chenyi Qiang]

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.

> Regards,
> Daniel