[Qemu-devel] [PATCH] target/i386: Generate #UD when applying LOCK to a register destination

Richard Henderson posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181113193510.24862-1-richard.henderson@linaro.org
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
target/i386/translate.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
[Qemu-devel] [PATCH] target/i386: Generate #UD when applying LOCK to a register destination
Posted by Richard Henderson 7 years, 2 months ago
Fixes a TCG crash due to attempting the atomic operation without
having set up the address first.  This does not attempt to fix
all of the other missing checks for LOCK.

Fixes: a7cee522f35
Fixes: https://bugs.launchpad.net/qemu/+bug/1803160
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/translate.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index f8bc7680af..0dd5fbe45c 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -1268,10 +1268,30 @@ static void gen_helper_fp_arith_STN_ST0(int op, int opreg)
     }
 }
 
+static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
+{
+    gen_update_cc_op(s);
+    gen_jmp_im(s, cur_eip);
+    gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno));
+    s->base.is_jmp = DISAS_NORETURN;
+}
+
+/* Generate #UD for the current instruction.  The assumption here is that
+   the instruction is known, but it isn't allowed in the current cpu mode.  */
+static void gen_illegal_opcode(DisasContext *s)
+{
+    gen_exception(s, EXCP06_ILLOP, s->pc_start - s->cs_base);
+}
+
 /* if d == OR_TMP0, it means memory operand (address in A0) */
 static void gen_op(DisasContext *s1, int op, TCGMemOp ot, int d)
 {
     if (d != OR_TMP0) {
+        if (s1->prefix & PREFIX_LOCK) {
+            /* Lock prefix when destination is not memory.  */
+            gen_illegal_opcode(s1);
+            return;
+        }
         gen_op_mov_v_reg(s1, ot, s1->T0, d);
     } else if (!(s1->prefix & PREFIX_LOCK)) {
         gen_op_ld_v(s1, ot, s1->T0, s1->A0);
@@ -2469,21 +2489,6 @@ static void gen_leave(DisasContext *s)
     gen_op_mov_reg_v(s, a_ot, R_ESP, s->T1);
 }
 
-static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
-{
-    gen_update_cc_op(s);
-    gen_jmp_im(s, cur_eip);
-    gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno));
-    s->base.is_jmp = DISAS_NORETURN;
-}
-
-/* Generate #UD for the current instruction.  The assumption here is that
-   the instruction is known, but it isn't allowed in the current cpu mode.  */
-static void gen_illegal_opcode(DisasContext *s)
-{
-    gen_exception(s, EXCP06_ILLOP, s->pc_start - s->cs_base);
-}
-
 /* Similarly, except that the assumption here is that we don't decode
    the instruction at all -- either a missing opcode, an unimplemented
    feature, or just a bogus instruction stream.  */
-- 
2.17.2


Re: [Qemu-devel] [PATCH] target/i386: Generate #UD when applying LOCK to a register destination
Posted by Philippe Mathieu-Daudé 7 years, 2 months ago
On Tue, Nov 13, 2018 at 8:36 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Fixes a TCG crash due to attempting the atomic operation without
> having set up the address first.  This does not attempt to fix
> all of the other missing checks for LOCK.
>
> Fixes: a7cee522f35
> Fixes: https://bugs.launchpad.net/qemu/+bug/1803160
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/i386/translate.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index f8bc7680af..0dd5fbe45c 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -1268,10 +1268,30 @@ static void gen_helper_fp_arith_STN_ST0(int op, int opreg)
>      }
>  }
>
> +static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
> +{
> +    gen_update_cc_op(s);
> +    gen_jmp_im(s, cur_eip);
> +    gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno));
> +    s->base.is_jmp = DISAS_NORETURN;
> +}
> +
> +/* Generate #UD for the current instruction.  The assumption here is that
> +   the instruction is known, but it isn't allowed in the current cpu mode.  */
> +static void gen_illegal_opcode(DisasContext *s)
> +{
> +    gen_exception(s, EXCP06_ILLOP, s->pc_start - s->cs_base);
> +}
> +
>  /* if d == OR_TMP0, it means memory operand (address in A0) */
>  static void gen_op(DisasContext *s1, int op, TCGMemOp ot, int d)
>  {
>      if (d != OR_TMP0) {
> +        if (s1->prefix & PREFIX_LOCK) {
> +            /* Lock prefix when destination is not memory.  */
> +            gen_illegal_opcode(s1);
> +            return;
> +        }
>          gen_op_mov_v_reg(s1, ot, s1->T0, d);
>      } else if (!(s1->prefix & PREFIX_LOCK)) {
>          gen_op_ld_v(s1, ot, s1->T0, s1->A0);
> @@ -2469,21 +2489,6 @@ static void gen_leave(DisasContext *s)
>      gen_op_mov_reg_v(s, a_ot, R_ESP, s->T1);
>  }
>
> -static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
> -{
> -    gen_update_cc_op(s);
> -    gen_jmp_im(s, cur_eip);
> -    gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno));
> -    s->base.is_jmp = DISAS_NORETURN;
> -}
> -
> -/* Generate #UD for the current instruction.  The assumption here is that
> -   the instruction is known, but it isn't allowed in the current cpu mode.  */
> -static void gen_illegal_opcode(DisasContext *s)
> -{
> -    gen_exception(s, EXCP06_ILLOP, s->pc_start - s->cs_base);
> -}
> -
>  /* Similarly, except that the assumption here is that we don't decode
>     the instruction at all -- either a missing opcode, an unimplemented
>     feature, or just a bogus instruction stream.  */
> --
> 2.17.2
>