From 1a3abaa81a93d5630cb69255c5acd2367b6b602c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Wed, 3 Mar 2021 19:31:48 -0500 Subject: [PATCH] target/i386: Check privilege level for protected mode 'int N' task gate When the 'int N' instruction is executed in protected mode, the pseudocode in the architecture manual specifies that we need to check: * vector number within IDT limits * selected IDT descriptor is a valid type (interrupt, trap or task gate) * if this was a software interrupt then gate DPL < CPL The way we had structured the code meant that the privilege check for software interrupts ended up not in the code path taken for task gate handling, because all of the task gate handling code was in the 'case 5' of the switch which was checking "is this descriptor a valid type". Move the task gate handling code out of that switch (so that it is now purely doing the "valid type?" check) and below the software interrupt privilege check. The effect of this missing check was that in a guest userspace binary executing 'int 8' would cause a guest kernel panic rather than the userspace binary being handed a SEGV. This is essentially the same bug fixed in VirtualBox in 2012: https://www.halfdog.net/Security/2012/VirtualBoxSoftwareInterrupt0x8GuestCrash/ Note that for QEMU this is not a security issue because it is only present when using TCG. Backports 3df1a3d070575419859cbbab1083fafa7ec2669a --- qemu/target/i386/seg_helper.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/qemu/target/i386/seg_helper.c b/qemu/target/i386/seg_helper.c index 87a42a9b..7835afa9 100644 --- a/qemu/target/i386/seg_helper.c +++ b/qemu/target/i386/seg_helper.c @@ -633,6 +633,24 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int, type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; switch (type) { case 5: /* task gate */ + case 6: /* 286 interrupt gate */ + case 7: /* 286 trap gate */ + case 14: /* 386 interrupt gate */ + case 15: /* 386 trap gate */ + break; + default: + 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); + } + + if (type == 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); @@ -660,21 +678,10 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int, SET_ESP(esp, mask); } return; - case 6: /* 286 interrupt gate */ - case 7: /* 286 trap gate */ - case 14: /* 386 interrupt gate */ - case 15: /* 386 trap gate */ - break; - default: - 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); } + + /* Otherwise, trap or interrupt gate */ + /* check valid bit */ if (!(e2 & DESC_P_MASK)) { raise_exception_err(env, EXCP0B_NOSEG, intno * 8 + 2);