On 05/10/2010 06:51 PM, Mohammed Gamal wrote:
- Correct unusable flag check on SS, DS, ES, FS, GS, and LDTR
- Add rflags checks
- Report failed instruction on emulation failure


Please post as separate patches.

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 777e00d..968384b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2122,7 +2122,7 @@ static bool stack_segment_valid(struct kvm_vcpu *vcpu)
        ss_rpl = ss.selector&  SELECTOR_RPL_MASK;

        if (ss.unusable)
-               return true;
+               return false;

Where does it say that ss must be usable?

        if (ss.type != 3&&  ss.type != 7)
                return false;
        if (!ss.s)
@@ -2144,7 +2144,7 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, int 
seg)
        rpl = var.selector&  SELECTOR_RPL_MASK;

        if (var.unusable)
-               return true;
+               return false;
        if (!var.s)
                return false;
        if (!var.present)

Ditto.

@@ -2185,7 +2185,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
        vmx_get_segment(vcpu,&ldtr, VCPU_SREG_LDTR);

        if (ldtr.unusable)
-               return true;
+               return false;
        if (ldtr.selector&  SELECTOR_TI_MASK)       /* TI = 1 */
                return false;
        if (ldtr.type != 2)

Ditto.

@@ -2207,6 +2207,27 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
                 (ss.selector&  SELECTOR_RPL_MASK));
  }

+static bool rflags_valid(struct kvm_vcpu *vcpu)
+{
+       unsigned long rflags;
+       u32 entry_intr_info;
+
+       rflags = vmcs_readl(GUEST_RFLAGS);
+       entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+#ifdef CONFIG_X86_64
+       if (is_long_mode(vcpu))
+               if (rflags&  X86_EFLAGS_VM)
+                       return false;   
+#endif

This is architecturally illegal, not just for vmx entries. The check should be made when emulating setting rflags and in KVM_SET_REGS.

These checks should assume the state is architecturally legal, and only check if they are legal for vmx entries.

+       if ((entry_intr_info&  INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_EXT_INTR
+               &&  (entry_intr_info&  INTR_INFO_VALID_MASK)) {
+               if (!(rflags&  X86_EFLAGS_IF))
+                       return false;
+       }
+

Ditto.

@@ -3559,6 +3581,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
                }

                if (err != EMULATE_DONE) {
+                       kvm_report_emulation_failure(vcpu, "invalid guest state 
handler");
                        vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
                        vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
                        vcpu->run->internal.ndata = 0;

Uneeded, userspace can report it.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to