]> xenbits.xensource.com Git - xen.git/commit
x86/vvmx: Disallow the use of VT-x instructions when nested virt is disabled
authorAndrew Cooper <andrew.cooper3@citrix.com>
Wed, 10 Oct 2018 09:17:15 +0000 (09:17 +0000)
committerAndrew Cooper <andrew.cooper3@citrix.com>
Wed, 24 Oct 2018 21:15:11 +0000 (22:15 +0100)
commitf294d80e8e43d4cdcc6d4d94b1e9c9b1aadf67d8
treee6b5077b8b3fd5d54161b11a3660624d6c71c6ba
parent782ca9b94f77026875dd98d6288fc1f8dcc7ce19
x86/vvmx: Disallow the use of VT-x instructions when nested virt is disabled

c/s ac6a4500b "vvmx: set vmxon_region_pa of vcpu out of VMX operation to an
invalid address" was a real bugfix as described, but has a very subtle bug
which results in all VT-x instructions being usable by a guest.

The toolstack constructs a guest by issuing:

  XEN_DOMCTL_createdomain
  XEN_DOMCTL_max_vcpus

and optionally later, HVMOP_set_param to enable nested virt.

As a result, the call to nvmx_vcpu_initialise() in hvm_vcpu_initialise()
(which is what makes the above patch look correct during review) is actually
dead code.  In practice, nvmx_vcpu_initialise() first gets called when nested
virt is enabled, which is typically never.

As a result, the zeroed memory of struct vcpu causes nvmx_vcpu_in_vmx() to
return true before nested virt is enabled for the guest.

Fixing the order of initialisation is a work in progress for other reasons,
but not viable for security backports.

A compounding factor is that the vmexit handlers for all instructions, other
than VMXON, pass 0 into vmx_inst_check_privilege()'s vmxop_check parameter,
which skips the CR4.VMXE check.  (This is one of many reasons why nested virt
isn't a supported feature yet.)

However, the overall result is that when nested virt is not enabled by the
toolstack (i.e. the default configuration for all production guests), the VT-x
instructions (other than VMXON) are actually usable, and Xen very quickly
falls over the fact that the nvmx structure is uninitialised.

In order to fail safe in the supported case, re-implement all the VT-x
instruction handling using a single function with a common prologue, covering
all the checks which should cause #UD or #GP faults.  This deliberately
doesn't use any state from the nvmx structure, in case there are other lurking
issues.

This is XSA-278

Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Sergey Dyasli <sergey.dyasli@citrix.com>
xen/arch/x86/hvm/vmx/vmx.c
xen/arch/x86/hvm/vmx/vvmx.c
xen/include/asm-x86/hvm/vmx/vvmx.h