From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v2 1/3] KVM: x86 emulator: Fix task switch privilege checks Date: Mon, 30 Jan 2012 12:39:38 +0200 Message-ID: <4F26736A.9050800@redhat.com> References: <1327692216-20450-1-git-send-email-kwolf@redhat.com> <1327692216-20450-2-git-send-email-kwolf@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, gleb@redhat.com, joerg.roedel@amd.com, yoshikawa.takuya@oss.ntt.co.jp, mtosatti@redhat.com To: Kevin Wolf Return-path: Received: from mx1.redhat.com ([209.132.183.28]:16454 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751202Ab2A3Kjt (ORCPT ); Mon, 30 Jan 2012 05:39:49 -0500 In-Reply-To: <1327692216-20450-2-git-send-email-kwolf@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/27/2012 09:23 PM, Kevin Wolf wrote: > 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. > > > @@ -2372,12 +2389,32 @@ 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. Software interrupt: Check against DPL of the task gate > + * 2. Exception/IRQ/iret: No check is performed > + * 3. jmp/call: Check agains DPL of the TSS > + */ > + dpl = -1; > + if (reason == TASK_SWITCH_GATE) { > + if (idt_index != -1) { > + struct kvm_desc_struct task_gate_desc; > + > + ret = read_interrupt_descriptor(ctxt, idt_index, > + &task_gate_desc); > + if (ret != X86EMUL_CONTINUE) > + return ret; > + > + dpl = task_gate_desc.dpl; > + } > + } else if (reason != TASK_SWITCH_IRET) { > + dpl = next_tss_desc.dpl; > } > > + if (dpl != -1 && ((tss_selector & 3) > dpl || ops->cpl(ctxt) > dpl)) > + return emulate_gp(ctxt, 0); Should be #GP(selector), no? The TASK-GATE: branch of the CALL instruction documentation lists many other conditions which we don't check, so I'll accept this, otherwise we'll never make any progress. -- error compiling committee.c: too many arguments to function