| Message ID | 20240710062920.73063-8-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] target/i386/tcg: Remove SEG_ADDL | expand |
On 7/9/24 23:29, Paolo Bonzini wrote: > This fixes a bug wherein i386/tcg assumed an interrupt return using > the CALL or JMP instructions were always going from kernel or user mode to > kernel mode, when using a call gate. This assumption is violated if > the call gate has a DPL that is greater than 0. > > In addition, the stack accesses should count as explicit, not implicit > ("kernel" in QEMU code), so that SMAP is not applied if DPL=3. > > Analyzed-by: Robert R. Henry<rrh.henry@gmail.com> > Resolves:https://gitlab.com/qemu-project/qemu/-/issues/249 > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > --- > target/i386/tcg/seg_helper.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
10.07.2024 09:29, Paolo Bonzini wrote: > This fixes a bug wherein i386/tcg assumed an interrupt return using > the CALL or JMP instructions were always going from kernel or user mode to > kernel mode, when using a call gate. This assumption is violated if > the call gate has a DPL that is greater than 0. > > In addition, the stack accesses should count as explicit, not implicit > ("kernel" in QEMU code), so that SMAP is not applied if DPL=3. > > Analyzed-by: Robert R. Henry <rrh.henry@gmail.com> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/249 > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> This sounds like qemu-stable material, is it not? It can be picked up for 9.1.x, but for 9.0 and before it needs a few other changes in this area, like v9.0.0-2238-g8053862af9 "target/i386/tcg: Compute MMU index once" and v9.0.0-2236-g059368bcf5 "target/i386/tcg: Reorg push/pop within seg_helper.c", or it needs a proper backport. What do you think? Thanks, /mjt
18.10.2024 19:02, Michael Tokarev wrote: > 10.07.2024 09:29, Paolo Bonzini wrote: >> This fixes a bug wherein i386/tcg assumed an interrupt return using >> the CALL or JMP instructions were always going from kernel or user mode to >> kernel mode, when using a call gate. This assumption is violated if >> the call gate has a DPL that is greater than 0. >> >> In addition, the stack accesses should count as explicit, not implicit >> ("kernel" in QEMU code), so that SMAP is not applied if DPL=3. >> >> Analyzed-by: Robert R. Henry <rrh.henry@gmail.com> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/249 >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > This sounds like qemu-stable material, is it not? > > It can be picked up for 9.1.x, but for 9.0 and before it needs a few > other changes in this area, like v9.0.0-2238-g8053862af9 "target/i386/tcg: > Compute MMU index once" and v9.0.0-2236-g059368bcf5 "target/i386/tcg: > Reorg push/pop within seg_helper.c", or it needs a proper backport. > > What do you think? A friendly ping/help? :) Or should I drop this from 9.1.x too? Thanks, /mjt
On Fri, Oct 25, 2024 at 5:26 PM Michael Tokarev <mjt@tls.msk.ru> wrote: > > It can be picked up for 9.1.x, but for 9.0 and before it needs a few > > other changes in this area, like v9.0.0-2238-g8053862af9 "target/i386/tcg: > > Compute MMU index once" and v9.0.0-2236-g059368bcf5 "target/i386/tcg: > > Reorg push/pop within seg_helper.c", or it needs a proper backport. > > > > What do you think? > > A friendly ping/help? :) Hi! No, this is totally ok for 9.1.x; it missed 9.1 but it was already submitted back then and it's okay to apply it there. On the other hand, Richard wrote some large cleanup patches to enable this relatively small patch. The bug has been there for many years, we can keep it in 9.0.x and earlier. Thanks, Paolo
25.10.2024 18:28, Paolo Bonzini wrote: > Hi! No, this is totally ok for 9.1.x; it missed 9.1 but it was already > submitted back then and it's okay to apply it there. > > On the other hand, Richard wrote some large cleanup patches to enable > this relatively small patch. The bug has been there for many years, we > can keep it in 9.0.x and earlier. Aha. That makes good sense. Thank you for the follow-up and for clearing my confusion :) /mjt
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index 07e3667639a..1430f477c43 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -678,7 +678,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int, sa.env = env; sa.ra = 0; - sa.mmu_index = cpu_mmu_index_kernel(env); + sa.mmu_index = x86_mmu_index_pl(env, dpl); if (type == 5) { /* task gate */ @@ -984,7 +984,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int, sa.env = env; sa.ra = 0; - sa.mmu_index = cpu_mmu_index_kernel(env); + sa.mmu_index = x86_mmu_index_pl(env, dpl); sa.sp_mask = -1; sa.ss_base = 0; if (dpl < cpl || ist != 0) { @@ -1119,7 +1119,7 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int, sa.sp = env->regs[R_ESP]; sa.sp_mask = 0xffff; sa.ss_base = env->segs[R_SS].base; - sa.mmu_index = cpu_mmu_index_kernel(env); + sa.mmu_index = x86_mmu_index_pl(env, 0); if (is_int) { old_eip = next_eip; @@ -1583,7 +1583,7 @@ void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip, sa.sp = env->regs[R_ESP]; sa.sp_mask = get_sp_mask(env->segs[R_SS].flags); sa.ss_base = env->segs[R_SS].base; - sa.mmu_index = cpu_mmu_index_kernel(env); + sa.mmu_index = x86_mmu_index_pl(env, 0); if (shift) { pushl(&sa, env->segs[R_CS].selector); @@ -1619,17 +1619,17 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC()); } cpl = env->hflags & HF_CPL_MASK; + dpl = (e2 >> DESC_DPL_SHIFT) & 3; LOG_PCALL("desc=%08x:%08x\n", e1, e2); sa.env = env; sa.ra = GETPC(); - sa.mmu_index = cpu_mmu_index_kernel(env); + sa.mmu_index = x86_mmu_index_pl(env, dpl); if (e2 & DESC_S_MASK) { if (!(e2 & DESC_CS_MASK)) { raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC()); } - dpl = (e2 >> DESC_DPL_SHIFT) & 3; if (e2 & DESC_C_MASK) { /* conforming code segment */ if (dpl > cpl) { @@ -1691,7 +1691,6 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip, } else { /* check gate type */ type = (e2 >> DESC_TYPE_SHIFT) & 0x1f; - dpl = (e2 >> DESC_DPL_SHIFT) & 3; rpl = new_cs & 3; #ifdef TARGET_X86_64
This fixes a bug wherein i386/tcg assumed an interrupt return using the CALL or JMP instructions were always going from kernel or user mode to kernel mode, when using a call gate. This assumption is violated if the call gate has a DPL that is greater than 0. In addition, the stack accesses should count as explicit, not implicit ("kernel" in QEMU code), so that SMAP is not applied if DPL=3. Analyzed-by: Robert R. Henry <rrh.henry@gmail.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/249 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/tcg/seg_helper.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)