From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59990) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkdFA-00059A-GP for qemu-devel@nongnu.org; Wed, 14 May 2014 13:45:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkdF3-0006CX-K9 for qemu-devel@nongnu.org; Wed, 14 May 2014 13:45:28 -0400 Received: from mail-qg0-x234.google.com ([2607:f8b0:400d:c04::234]:48273) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkdF3-0006Aq-EM for qemu-devel@nongnu.org; Wed, 14 May 2014 13:45:21 -0400 Received: by mail-qg0-f52.google.com with SMTP id a108so3446254qge.11 for ; Wed, 14 May 2014 10:45:20 -0700 (PDT) Sender: Richard Henderson Message-ID: <5373ABAC.90707@twiddle.net> Date: Wed, 14 May 2014 10:45:16 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1400015573-7919-1-git-send-email-alexander.zuepke@hs-rm.de> In-Reply-To: <1400015573-7919-1-git-send-email-alexander.zuepke@hs-rm.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] x86: enforce DPL checking on task gate switches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Zuepke , qemu-devel@nongnu.org On 05/13/2014 02:12 PM, Alex Zuepke wrote: > x86 software emulation (non-KVM mode) does not check privilege levels on task gate switches. > An "int $8" in user mode panics any OS kernel by a forbidden direct call into the double fault handler. > > This testcase crashes a Linux kernel with a double fault panic: > > $ cat test.c > int main(void) > { > __asm__ volatile ("int $8"); > } > $ gcc test.c > $ ./a.out > panic ... > > Signed-off-by: Alex Zuepke > --- > target-i386/seg_helper.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c > index 8c3f92c..d7f3d49 100644 > --- a/target-i386/seg_helper.c > +++ b/target-i386/seg_helper.c > @@ -582,12 +582,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int, > e2 = cpu_ldl_kernel(env, ptr + 4); > /* check gate type */ > type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; > + dpl = (e2 >> DESC_DPL_SHIFT) & 3; > + cpl = env->hflags & HF_CPL_MASK; > switch (type) { > case 5: /* task gate */ > /* must do that check here to return the correct error code */ > if (!(e2 & DESC_P_MASK)) { > raise_exception_err(env, EXCP0B_NOSEG, intno * 8 + 2); > } > + /* check privilege if software int */ > + if (is_int && (dpl < cpl)) { > + raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); > + } > switch_tss(env, intno * 8, e1, e2, SWITCH_TSS_CALL, old_eip); > if (has_error_code) { > int type; > @@ -620,8 +626,6 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int, > raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); > break; > } > - dpl = (e2 >> DESC_DPL_SHIFT) & 3; > - cpl = env->hflags & HF_CPL_MASK; > /* check privilege if software int */ > if (is_int && dpl < cpl) { > raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2); > Please refer to the (rather large) pseudo code for the "int" instruction: # IF ...selected IDT descriptor is not an interrupt-, trap-, or task-gate type # THEN #GP(error_code(vector_number,1,EXT)); FI; # (* idt operand to error_code set because vector is used *) This would be that switch statement that we currently have, with the default case raising this exception. I'll note that we appear to be ignoring EXT? I.e. this should be intno * 8 + 2 + !is_int. # IF software interrupt (* Generated by INT n, INT3, or INTO *) # THEN # IF gate DPL < CPL (* PE = 1, DPL < CPL, software interrupt *) # THEN #GP(error_code(vector_number,1,0)); FI; # (* idt operand to error_code set because vector is used *) # (* ext operand to error_code is 0 because INT n ... *) # FI; This is the check that you're moving. But... # IF gate not present # THEN #NP(error_code(vector_number,1,EXT)); FI; # (* idt operand to error_code set because vector is used *) ... the #NP check should come after the #GP check. I'll note the missed EXT as part of the error_code here as well. So it would seem that moving the entire "check privilege" block above the switch statement will do the right thing, since the non-task-gate entries in the switch either do nothing or raise another identical #GP. r~