[PATCH 1/4] target/i386: decode-new: avoid out-of-bounds access to xmm_regs[-1]

Paolo Bonzini posted 4 patches 3 years, 3 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 1/4] target/i386: decode-new: avoid out-of-bounds access to xmm_regs[-1]
Posted by Paolo Bonzini 3 years, 3 months ago
If the destination is a memory register, op->n is -1.  Going through
tcg_gen_gvec_dup_imm path is both useless (the value has been stored
by the gen_* function already) and wrong because of the out-of-bounds
access.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/emit.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 27eca591a9..ebf299451d 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -296,7 +296,7 @@ static void gen_writeback(DisasContext *s, X86DecodedInsn *decode, int opn, TCGv
     case X86_OP_MMX:
         break;
     case X86_OP_SSE:
-        if ((s->prefix & PREFIX_VEX) && op->ot == MO_128) {
+        if (!op->has_ea && (s->prefix & PREFIX_VEX) && op->ot == MO_128) {
             tcg_gen_gvec_dup_imm(MO_64,
                                  offsetof(CPUX86State, xmm_regs[op->n].ZMM_X(1)),
                                  16, 16, 0);
-- 
2.37.3
Re: [PATCH 1/4] target/i386: decode-new: avoid out-of-bounds access to xmm_regs[-1]
Posted by Richard Henderson 3 years, 3 months ago
On 10/20/22 01:06, Paolo Bonzini wrote:
> If the destination is a memory register, op->n is -1.  Going through
> tcg_gen_gvec_dup_imm path is both useless (the value has been stored
> by the gen_* function already) and wrong because of the out-of-bounds
> access.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/emit.c.inc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Re: [PATCH 1/4] target/i386: decode-new: avoid out-of-bounds access to xmm_regs[-1]
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 19/10/22 17:06, Paolo Bonzini wrote:
> If the destination is a memory register, op->n is -1.  Going through
> tcg_gen_gvec_dup_imm path is both useless (the value has been stored
> by the gen_* function already) and wrong because of the out-of-bounds
> access.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/emit.c.inc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
> index 27eca591a9..ebf299451d 100644
> --- a/target/i386/tcg/emit.c.inc
> +++ b/target/i386/tcg/emit.c.inc
> @@ -296,7 +296,7 @@ static void gen_writeback(DisasContext *s, X86DecodedInsn *decode, int opn, TCGv
>       case X86_OP_MMX:
>           break;
>       case X86_OP_SSE:
> -        if ((s->prefix & PREFIX_VEX) && op->ot == MO_128) {
> +        if (!op->has_ea && (s->prefix & PREFIX_VEX) && op->ot == MO_128) {
>               tcg_gen_gvec_dup_imm(MO_64,
>                                    offsetof(CPUX86State, xmm_regs[op->n].ZMM_X(1)),
>                                    16, 16, 0);

Fixes: 20581aadec ("target/i386: validate VEX prefixes via the 
instructions' exception classes")

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>