Skip to content

Commit

Permalink
KVM: x86 emulator: Fix task switch privilege checks
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kevin Wolf authored and Avi Kivity committed Mar 8, 2012
1 parent 9cc815e commit 7f3d35f
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 17 deletions.
2 changes: 1 addition & 1 deletion arch/x86/include/asm/kvm_emulate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
4 changes: 2 additions & 2 deletions arch/x86/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
53 changes: 46 additions & 7 deletions arch/x86/kvm/emulate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand All @@ -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)) ||
Expand Down Expand Up @@ -2501,15 +2540,15 @@ 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;

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)
Expand Down
5 changes: 4 additions & 1 deletion arch/x86/kvm/svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions arch/x86/kvm/vmx.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions arch/x86/kvm/x86.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7f3d35f

Please sign in to comment.