Re: [Qemu-devel] [PATCH] target-i386: Fix lcall to call gate in IA-32e mode
On Sun, Aug 12, 2018 at 6:17 AM Paolo Bonzini wrote: > On 12/08/2018 05:07, Andrew Oates via Qemu-devel wrote: > > Currently call gates are always treated as 32-bit gates. In IA-32e mode > > (either compatibility or 64-bit submode), system segment descriptors are > > always 64-bit. Treating them as 32-bit has the expected unfortunate > > effect: only the lower 32 bits of the offset are loaded, the stack > > pointer is truncated, a bad new stack pointer is loaded from the TSS (if > > switching privilege levels), etc. > > > > This change adds support for 64-bit call gate to the lcall instruction. > > Additionally, there should be a check for non-canonical stack pointers, > > but I've omitted that since there doesn't seem to be checks for > > non-canonical addresses in this code elsewhere. > > Thanks. It's also missing a check for task gates in 64-bit mode and, > since we are at it, we should also implement ljmp to call gates. > Something like this (untested): > Yeah, I was going to do ljmp in a followup. I'll go ahead and add it to this patch. > > diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c > index d3b31f3c1b..0b2efe2cae 100644 > --- a/target/i386/seg_helper.c > +++ b/target/i386/seg_helper.c > @@ -1645,6 +1645,13 @@ void helper_ljmp_protected(CPUX86State *env, int > new_cs, target_ulong new_eip, > rpl = new_cs & 3; > cpl = env->hflags & HF_CPL_MASK; > type = (e2 >> DESC_TYPE_SHIFT) & 0xf; > +#ifdef TARGET_X86_64 > +if (env->efer & MSR_EFER_LMA) { > +if (type != 12) { > +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, > GETPC()); > +} > +} > +#endif > switch (type) { > case 1: /* 286 TSS */ > case 9: /* 386 TSS */ > @@ -1666,6 +1673,21 @@ void helper_ljmp_protected(CPUX86State *env, int > new_cs, target_ulong new_eip, > new_eip = (e1 & 0x); > if (type == 12) { > new_eip |= (e2 & 0x); > +#ifdef TARGET_X86_64 > +if (env->efer & MSR_EFER_LMA) { > +/* load the upper 8 bytes of the 64-bit call gate */ > +if (load_segment_ra(env, , , new_cs + 8, > GETPC())) { > +raise_exception_err_ra(env, EXCP0D_GPF, new_cs & > 0xfffc, > + GETPC()); > +} > +type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; > +if (type != 0) { > +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & > 0xfffc, > + GETPC()); > +} > +new_eip |= ((target_ulong)e1) << 32; > +} > +#endif > } > if (load_segment_ra(env, , , gate_cs, GETPC()) != 0) { > raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, > GETPC()); > @@ -1812,6 +1834,13 @@ void helper_lcall_protected(CPUX86State *env, int > new_cs, target_ulong new_eip, > type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; > dpl = (e2 >> DESC_DPL_SHIFT) & 3; > rpl = new_cs & 3; > +#ifdef TARGET_X86_64 > +if (env->efer & MSR_EFER_LMA) { > +if (type != 12) { > +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, > GETPC()); > +} > +} > +#endif > switch (type) { > case 1: /* available 286 TSS */ > case 9: /* available 386 TSS */ > > > Out of curiosity, what OS is using 64-bit call gates? :) > As far as I know, no important ones :) Just something I've been hacking around with in my spare time. The rational thing to do would have been to just stop using 64-bit call gates, but I figured it would be more fun to fix it for everyone. > > Paolo > > > I've left the raise_exception_err_ra lines unwapped at 80 columns to > > match the style in the rest of the file. > > > > Signed-off-by: Andrew Oates > > --- > > target/i386/seg_helper.c | 145 --- > > 1 file changed, 106 insertions(+), 39 deletions(-) > >
Re: [Qemu-devel] [PATCH] target-i386: Fix lcall to call gate in IA-32e mode
On 12/08/2018 05:07, Andrew Oates via Qemu-devel wrote: > Currently call gates are always treated as 32-bit gates. In IA-32e mode > (either compatibility or 64-bit submode), system segment descriptors are > always 64-bit. Treating them as 32-bit has the expected unfortunate > effect: only the lower 32 bits of the offset are loaded, the stack > pointer is truncated, a bad new stack pointer is loaded from the TSS (if > switching privilege levels), etc. > > This change adds support for 64-bit call gate to the lcall instruction. > Additionally, there should be a check for non-canonical stack pointers, > but I've omitted that since there doesn't seem to be checks for > non-canonical addresses in this code elsewhere. Thanks. It's also missing a check for task gates in 64-bit mode and, since we are at it, we should also implement ljmp to call gates. Something like this (untested): diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c index d3b31f3c1b..0b2efe2cae 100644 --- a/target/i386/seg_helper.c +++ b/target/i386/seg_helper.c @@ -1645,6 +1645,13 @@ void helper_ljmp_protected(CPUX86State *env, int new_cs, target_ulong new_eip, rpl = new_cs & 3; cpl = env->hflags & HF_CPL_MASK; type = (e2 >> DESC_TYPE_SHIFT) & 0xf; +#ifdef TARGET_X86_64 +if (env->efer & MSR_EFER_LMA) { +if (type != 12) { +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, GETPC()); +} +} +#endif switch (type) { case 1: /* 286 TSS */ case 9: /* 386 TSS */ @@ -1666,6 +1673,21 @@ void helper_ljmp_protected(CPUX86State *env, int new_cs, target_ulong new_eip, new_eip = (e1 & 0x); if (type == 12) { new_eip |= (e2 & 0x); +#ifdef TARGET_X86_64 +if (env->efer & MSR_EFER_LMA) { +/* load the upper 8 bytes of the 64-bit call gate */ +if (load_segment_ra(env, , , new_cs + 8, GETPC())) { +raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, + GETPC()); +} +type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; +if (type != 0) { +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, + GETPC()); +} +new_eip |= ((target_ulong)e1) << 32; +} +#endif } if (load_segment_ra(env, , , gate_cs, GETPC()) != 0) { raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, GETPC()); @@ -1812,6 +1834,13 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; dpl = (e2 >> DESC_DPL_SHIFT) & 3; rpl = new_cs & 3; +#ifdef TARGET_X86_64 +if (env->efer & MSR_EFER_LMA) { +if (type != 12) { +raise_exception_err_ra(env, EXCP0D_GPF, gate_cs & 0xfffc, GETPC()); +} +} +#endif switch (type) { case 1: /* available 286 TSS */ case 9: /* available 386 TSS */ Out of curiosity, what OS is using 64-bit call gates? :) Paolo > I've left the raise_exception_err_ra lines unwapped at 80 columns to > match the style in the rest of the file. > > Signed-off-by: Andrew Oates > --- > target/i386/seg_helper.c | 145 --- > 1 file changed, 106 insertions(+), 39 deletions(-)
[Qemu-devel] [PATCH] target-i386: Fix lcall to call gate in IA-32e mode
Currently call gates are always treated as 32-bit gates. In IA-32e mode (either compatibility or 64-bit submode), system segment descriptors are always 64-bit. Treating them as 32-bit has the expected unfortunate effect: only the lower 32 bits of the offset are loaded, the stack pointer is truncated, a bad new stack pointer is loaded from the TSS (if switching privilege levels), etc. This change adds support for 64-bit call gate to the lcall instruction. Additionally, there should be a check for non-canonical stack pointers, but I've omitted that since there doesn't seem to be checks for non-canonical addresses in this code elsewhere. I've left the raise_exception_err_ra lines unwapped at 80 columns to match the style in the rest of the file. Signed-off-by: Andrew Oates --- target/i386/seg_helper.c | 145 --- 1 file changed, 106 insertions(+), 39 deletions(-) diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c index 00301a0c04..d3b31f3c1b 100644 --- a/target/i386/seg_helper.c +++ b/target/i386/seg_helper.c @@ -518,6 +518,11 @@ static void switch_tss(CPUX86State *env, int tss_selector, static inline unsigned int get_sp_mask(unsigned int e2) { +#ifdef TARGET_X86_64 +if (e2 & DESC_L_MASK) { +return 0; +} else +#endif if (e2 & DESC_B_MASK) { return 0x; } else { @@ -1724,12 +1729,12 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, int shift, target_ulong next_eip) { int new_stack, i; -uint32_t e1, e2, cpl, dpl, rpl, selector, offset, param_count; -uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, sp, type, ss_dpl, sp_mask; +uint32_t e1, e2, cpl, dpl, rpl, selector, param_count; +uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, type, ss_dpl, sp_mask; uint32_t val, limit, old_sp_mask; -target_ulong ssp, old_ssp; +target_ulong ssp, old_ssp, offset, sp; -LOG_PCALL("lcall %04x:%08x s=%d\n", new_cs, (uint32_t)new_eip, shift); +LOG_PCALL("lcall %04x:" TARGET_FMT_lx " s=%d\n", new_cs, new_eip, shift); LOG_PCALL_STATE(CPU(x86_env_get_cpu(env))); if ((new_cs & 0xfffc) == 0) { raise_exception_err_ra(env, EXCP0D_GPF, 0, GETPC()); @@ -1833,8 +1838,23 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, raise_exception_err_ra(env, EXCP0B_NOSEG, new_cs & 0xfffc, GETPC()); } selector = e1 >> 16; -offset = (e2 & 0x) | (e1 & 0x); param_count = e2 & 0x1f; +offset = (e2 & 0x) | (e1 & 0x); +#ifdef TARGET_X86_64 +if (env->efer & MSR_EFER_LMA) { +/* load the upper 8 bytes of the 64-bit call gate */ +if (load_segment_ra(env, , , new_cs + 8, GETPC())) { +raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, + GETPC()); +} +type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; +if (type != 0) { +raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, + GETPC()); +} +offset |= ((target_ulong)e1) << 32; +} +#endif if ((selector & 0xfffc) == 0) { raise_exception_err_ra(env, EXCP0D_GPF, 0, GETPC()); } @@ -1849,46 +1869,80 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, if (dpl > cpl) { raise_exception_err_ra(env, EXCP0D_GPF, selector & 0xfffc, GETPC()); } +#ifdef TARGET_X86_64 +if (env->efer & MSR_EFER_LMA) { +if (!(e2 & DESC_L_MASK)) { +raise_exception_err_ra(env, EXCP0D_GPF, selector & 0xfffc, GETPC()); +} +if (e2 & DESC_B_MASK) { +raise_exception_err_ra(env, EXCP0D_GPF, selector & 0xfffc, GETPC()); +} +shift++; +} +#endif if (!(e2 & DESC_P_MASK)) { raise_exception_err_ra(env, EXCP0B_NOSEG, selector & 0xfffc, GETPC()); } if (!(e2 & DESC_C_MASK) && dpl < cpl) { /* to inner privilege */ -get_ss_esp_from_tss(env, , , dpl, GETPC()); -LOG_PCALL("new ss:esp=%04x:%08x param_count=%d env->regs[R_ESP]=" - TARGET_FMT_lx "\n", ss, sp, param_count, - env->regs[R_ESP]); -if ((ss & 0xfffc) == 0) { -raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC()); -} -if ((ss & 3) != dpl) { -raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC()); -} -if (load_segment_ra(env, _e1, _e2, ss, GETPC()) != 0) { -raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC()); -} -ss_dpl = (ss_e2 >> DESC_DPL_SHIFT) & 3; -if (ss_dpl != dpl) { -