[Qemu-devel] [PATCH] target-i386: fix addr16 prefix

[Qemu-devel] [PATCH] target-i386: fix addr16 prefix

[Paolo Bonzini]

While ADDSEG will only be false in 16-bit mode for LEA, it can be
false even in other cases when 16-bit addresses are obtained via
the 67h prefix in 32-bit mode.  In this case, gen_lea_v_seg forgets
to add a nonzero FS or GS base if CS/DS/ES/SS are all zero.  This
case is pretty rare but happens when booting Windows 95/98, and
this patch fixes it.

The bug is visible since commit d6a291498, but it was introduced
together with gen_lea_v_seg and it probably could be reproduced
with a "addr16 gs movsb" instruction as early as in commit
ca2f29f555805d07fb0b9ebfbbfc4e3656530977.

Cc: rth@twiddle.net
Reported-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/translate.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index aaac3c2..b11dfbd 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -466,15 +466,15 @@ static void gen_lea_v_seg(DisasContext *s, TCGMemOp aflag, TCGv a0,
         break;
     case MO_16:
         /* 16 bit address */
-        if (ovr_seg < 0) {
-            ovr_seg = def_seg;
-        }
         tcg_gen_ext16u_tl(cpu_A0, a0);
-        /* ADDSEG will only be false in 16-bit mode for LEA.  */
-        if (!s->addseg) {
-            return;
-        }
         a0 = cpu_A0;
+        if (ovr_seg < 0) {
+            if (s->addseg) {
+                ovr_seg = def_seg;
+            } else {
+                return;
+            }
+        }
         break;
     default:
         tcg_abort();
-- 
2.5.0

Re: [Qemu-devel] [PATCH] target-i386: fix addr16 prefix

[Hervé Poussineau]

Le 02/03/2016 16:04, Paolo Bonzini a écrit :
> While ADDSEG will only be false in 16-bit mode for LEA, it can be
> false even in other cases when 16-bit addresses are obtained via
> the 67h prefix in 32-bit mode.  In this case, gen_lea_v_seg forgets
> to add a nonzero FS or GS base if CS/DS/ES/SS are all zero.  This
> case is pretty rare but happens when booting Windows 95/98, and
> this patch fixes it.
>
> The bug is visible since commit d6a291498, but it was introduced
> together with gen_lea_v_seg and it probably could be reproduced
> with a "addr16 gs movsb" instruction as early as in commit
> ca2f29f555805d07fb0b9ebfbbfc4e3656530977.
>
> Cc: rth@twiddle.net
> Reported-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Tested-by: Hervé Poussineau <hpoussin@reactos.org>

Re: [Qemu-devel] [PATCH] target-i386: fix addr16 prefix

[Richard Henderson]

On 03/02/2016 07:04 AM, Paolo Bonzini wrote:
> While ADDSEG will only be false in 16-bit mode for LEA, it can be
> false even in other cases when 16-bit addresses are obtained via
> the 67h prefix in 32-bit mode.  In this case, gen_lea_v_seg forgets
> to add a nonzero FS or GS base if CS/DS/ES/SS are all zero.  This
> case is pretty rare but happens when booting Windows 95/98, and
> this patch fixes it.
> 
> The bug is visible since commit d6a291498, but it was introduced
> together with gen_lea_v_seg and it probably could be reproduced
> with a "addr16 gs movsb" instruction as early as in commit
> ca2f29f555805d07fb0b9ebfbbfc4e3656530977.
> 
> Cc: rth@twiddle.net
> Reported-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-i386/translate.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>

It doesn't even seem to be uncommon inside the win98 kernel, once you start
looking for that addr16 gs pattern.

Thanks,


r~