KVM: x86 emulator: Fix task switch privilege checks
authorKevin Wolf <kwolf@redhat.com>
Wed, 8 Feb 2012 13:34:38 +0000 (14:34 +0100)
committerAvi Kivity <avi@redhat.com>
Thu, 8 Mar 2012 12:10:26 +0000 (14:10 +0200)
Currently, all task switches check privileges against the DPL of the
TSS. This is only correct for jmp/call to a TSS. If a task gate is used,
the DPL of this take gate is used for the check instead. Exceptions,
external interrupts and iret shouldn't perform any check.

[avi: kill kvm-kmod remnants]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
arch/x86/include/asm/kvm_emulate.h
arch/x86/include/asm/kvm_host.h
arch/x86/kvm/emulate.c
arch/x86/kvm/svm.c
arch/x86/kvm/vmx.c
arch/x86/kvm/x86.c

index 7b9cfc4878afc374dacb4852312be4fcba35384e..df437b68f42b693da00e115b93937bd54d695feb 100644 (file)
@@ -388,7 +388,7 @@ bool x86_page_table_writing_insn(struct x86_emulate_ctxt *ctxt);
 #define EMULATION_INTERCEPTED 2
 int x86_emulate_insn(struct x86_emulate_ctxt *ctxt);
 int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
-                        u16 tss_selector, int reason,
+                        u16 tss_selector, int idt_index, int reason,
                         bool has_error_code, u32 error_code);
 int emulate_int_real(struct x86_emulate_ctxt *ctxt, int irq);
 #endif /* _ASM_X86_KVM_X86_EMULATE_H */
index 74c9edf2bb18c9b1b4788a4006dc34d298ae67de..e216ba066e798cbd7be31892ac76afcc78055c0b 100644 (file)
@@ -768,8 +768,8 @@ int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu);
 void kvm_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
 int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector, int seg);
 
-int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int reason,
-                   bool has_error_code, u32 error_code);
+int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index,
+                   int reason, bool has_error_code, u32 error_code);
 
 int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);
 int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3);
index 71450aca3b86889abff6bb7a5230ec997ee75bef..fa310a48591ce2e2bc000fe63ba3a54219823a12 100644 (file)
@@ -1152,6 +1152,22 @@ static int pio_in_emulated(struct x86_emulate_ctxt *ctxt,
        return 1;
 }
 
+static int read_interrupt_descriptor(struct x86_emulate_ctxt *ctxt,
+                                    u16 index, struct desc_struct *desc)
+{
+       struct desc_ptr dt;
+       ulong addr;
+
+       ctxt->ops->get_idt(ctxt, &dt);
+
+       if (dt.size < index * 8 + 7)
+               return emulate_gp(ctxt, index << 3 | 0x2);
+
+       addr = dt.address + index * 8;
+       return ctxt->ops->read_std(ctxt, addr, desc, sizeof *desc,
+                                  &ctxt->exception);
+}
+
 static void get_descriptor_table_ptr(struct x86_emulate_ctxt *ctxt,
                                     u16 selector, struct desc_ptr *dt)
 {
@@ -2421,7 +2437,7 @@ static int task_switch_32(struct x86_emulate_ctxt *ctxt,
 }
 
 static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
-                                  u16 tss_selector, int reason,
+                                  u16 tss_selector, int idt_index, int reason,
                                   bool has_error_code, u32 error_code)
 {
        struct x86_emulate_ops *ops = ctxt->ops;
@@ -2443,12 +2459,35 @@ static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
 
        /* FIXME: check that next_tss_desc is tss */
 
-       if (reason != TASK_SWITCH_IRET) {
-               if ((tss_selector & 3) > next_tss_desc.dpl ||
-                   ops->cpl(ctxt) > next_tss_desc.dpl)
-                       return emulate_gp(ctxt, 0);
+       /*
+        * Check privileges. The three cases are task switch caused by...
+        *
+        * 1. jmp/call/int to task gate: Check against DPL of the task gate
+        * 2. Exception/IRQ/iret: No check is performed
+        * 3. jmp/call to TSS: Check agains DPL of the TSS
+        */
+       if (reason == TASK_SWITCH_GATE) {
+               if (idt_index != -1) {
+                       /* Software interrupts */
+                       struct desc_struct task_gate_desc;
+                       int dpl;
+
+                       ret = read_interrupt_descriptor(ctxt, idt_index,
+                                                       &task_gate_desc);
+                       if (ret != X86EMUL_CONTINUE)
+                               return ret;
+
+                       dpl = task_gate_desc.dpl;
+                       if ((tss_selector & 3) > dpl || ops->cpl(ctxt) > dpl)
+                               return emulate_gp(ctxt, (idt_index << 3) | 0x2);
+               }
+       } else if (reason != TASK_SWITCH_IRET) {
+               int dpl = next_tss_desc.dpl;
+               if ((tss_selector & 3) > dpl || ops->cpl(ctxt) > dpl)
+                       return emulate_gp(ctxt, tss_selector);
        }
 
+
        desc_limit = desc_limit_scaled(&next_tss_desc);
        if (!next_tss_desc.p ||
            ((desc_limit < 0x67 && (next_tss_desc.type & 8)) ||
@@ -2501,7 +2540,7 @@ static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
 }
 
 int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
-                        u16 tss_selector, int reason,
+                        u16 tss_selector, int idt_index, int reason,
                         bool has_error_code, u32 error_code)
 {
        int rc;
@@ -2509,7 +2548,7 @@ int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
        ctxt->_eip = ctxt->eip;
        ctxt->dst.type = OP_NONE;
 
-       rc = emulator_do_task_switch(ctxt, tss_selector, reason,
+       rc = emulator_do_task_switch(ctxt, tss_selector, idt_index, reason,
                                     has_error_code, error_code);
 
        if (rc == X86EMUL_CONTINUE)
index 0b7690ee20bdafa722fb160b25e214290d11915c..95cdeaf9c71810b912a4d36c77ce352a044f21ed 100644 (file)
@@ -2799,7 +2799,10 @@ static int task_switch_interception(struct vcpu_svm *svm)
             (int_vec == OF_VECTOR || int_vec == BP_VECTOR)))
                skip_emulated_instruction(&svm->vcpu);
 
-       if (kvm_task_switch(&svm->vcpu, tss_selector, reason,
+       if (int_type != SVM_EXITINTINFO_TYPE_SOFT)
+               int_vec = -1;
+
+       if (kvm_task_switch(&svm->vcpu, tss_selector, int_vec, reason,
                                has_error_code, error_code) == EMULATE_FAIL) {
                svm->vcpu.run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
                svm->vcpu.run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
index d2bd719925a62b9c8c37b2c99574f88f77804b11..124a0952a04049be18eae6a74542587f49ec4b29 100644 (file)
@@ -4658,9 +4658,10 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
        bool has_error_code = false;
        u32 error_code = 0;
        u16 tss_selector;
-       int reason, type, idt_v;
+       int reason, type, idt_v, idt_index;
 
        idt_v = (vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK);
+       idt_index = (vmx->idt_vectoring_info & VECTORING_INFO_VECTOR_MASK);
        type = (vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK);
 
        exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
@@ -4698,8 +4699,9 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
                       type != INTR_TYPE_NMI_INTR))
                skip_emulated_instruction(vcpu);
 
-       if (kvm_task_switch(vcpu, tss_selector, reason,
-                               has_error_code, error_code) == EMULATE_FAIL) {
+       if (kvm_task_switch(vcpu, tss_selector,
+                           type == INTR_TYPE_SOFT_INTR ? idt_index : -1, reason,
+                           has_error_code, error_code) == EMULATE_FAIL) {
                vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
                vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
                vcpu->run->internal.ndata = 0;
index ca74c1dadf3afeda4a455c5396e987291d8a8c00..490a1b1a255f5de7cc7bbfd02e3b5d3c9a629af9 100644 (file)
@@ -5655,15 +5655,15 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
        return 0;
 }
 
-int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int reason,
-                   bool has_error_code, u32 error_code)
+int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index,
+                   int reason, bool has_error_code, u32 error_code)
 {
        struct x86_emulate_ctxt *ctxt = &vcpu->arch.emulate_ctxt;
        int ret;
 
        init_emulate_ctxt(vcpu);
 
-       ret = emulator_task_switch(ctxt, tss_selector, reason,
+       ret = emulator_task_switch(ctxt, tss_selector, idt_index, reason,
                                   has_error_code, error_code);
 
        if (ret)