[Bug 216033] KVM VMX nested virtualization: VMXON does not check guest CR0 against IA32_VMX_CR0_FIXED0

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=216033

--- Comment #3 from Sean Christopherson (seanjc@google.com) ---
On Fri, Sep 02, 2022, bugzilla-daemon@kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=216033
> 
> --- Comment #2 from Eric Li (ercli@ucdavis.edu) ---
> @Sean Christopherson Thanks for submitting the fix to this bug in
> https://lore.kernel.org/lkml/20220607213604.3346000-4-seanjc@google.com/ .
> However, I recently tested this fix and the behavior is not as expected.
> 
> According to Intel's SDM, VMXON may generate 2 types of exceptions:
> 
>     IF (register operand) or (CR0.PE = 0) or (CR4.VMXE = 0) or ...
>         THEN #UD;
>     ELSIF not in VMX operation
>         THEN
>             IF (CPL > 0) or (in A20M mode) or
>             (the values of CR0 and CR4 are not supported in VMX operation ...
>                 THEN #GP(0);
> 
> For example, when CR4 value is incorrect, different exceptions may be
> generated
> depending on which bit is incorrect. If CR4.VMXE = 0, #UD should be
> generated.
> Otherwise, #GP(0) should be generated. However, after the fix, #UD is always
> generated.

/facepalm

All that and I overlooked that the other CR0/CR4 checks take a #GP.

On the bright side, it does mean I can blame Jim at least a little bit for
commit
70f3aac964ae ("kvm: nVMX: Remove superfluous VMX instruction fault checks").

Untested, but this should do the trick.

---
 arch/x86/kvm/vmx/nested.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index ddd4367d4826..86ee2ab8a497 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -4936,25 +4936,32 @@ static int handle_vmxon(struct kvm_vcpu *vcpu)
                | FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX;

        /*
-        * Note, KVM cannot rely on hardware to perform the CR0/CR4 #UD checks
-        * that have higher priority than VM-Exit (see Intel SDM's pseudocode
-        * for VMXON), as KVM must load valid CR0/CR4 values into hardware
while
-        * running the guest, i.e. KVM needs to check the _guest_ values.
+        * Note, KVM cannot rely on hardware to perform the CR0.PE and CR4.VMXE
+        * #UD checks that have higher priority than VM-Exit (see Intel SDM's
+        * pseudocode for VMXON), as KVM must load valid CR0/CR4 values into
+        * hardware while running the guest, i.e. KVM needs to check the
_guest_
+        * values.
         *
         * Rely on hardware for the other two pre-VM-Exit checks, !VM86 and
         * !COMPATIBILITY modes.  KVM may run the guest in VM86 to emulate Real
         * Mode, but KVM will never take the guest out of those modes.
         */
+       if (!kvm_read_cr4_bits(vcpu, X86_CR4_VMXE) ||
+           !kvm_read_cr0_bits(vcpu, X86_CR0_PE)) {
+               kvm_queue_exception(vcpu, UD_VECTOR);
+               return 1;
+       }
+
+       /*
+        * All other checks that are lower priority than VM-Exit must be
+        * checked manually, including the other CR0/CR4 reserved bit checks.
+        */
        if (!nested_host_cr0_valid(vcpu, kvm_read_cr0(vcpu)) ||
            !nested_host_cr4_valid(vcpu, kvm_read_cr4(vcpu))) {
                kvm_queue_exception(vcpu, UD_VECTOR);
                return 1;
        }

-       /*
-        * CPL=0 and all other checks that are lower priority than VM-Exit must
-        * be checked manually.
-        */
        if (vmx_get_cpl(vcpu)) {
                kvm_inject_gp(vcpu, 0);
                return 1;

base-commit: 476d5fb78ea6438941559af4814a2795849cb8f0

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.