Il mar 29 set 2026, 04:22 Andrey Polivoda <[email protected]> ha
scritto:
> Hi Paolo,
>
> In its current form, this patch makes VROUNDSS (and possibly other
> instructions) use 256-bit operand size when VEX.L=1
>
Thanks for the test case. I will take a look. The ordering of
validate_vex() and decode_op_*() is tricky, I should fix it once and for
all...
Paolo
This happens because the operand sizes are set in `decode_op_size()`,
> which is called in `decode_insn()`
>
> `decode_op_size()` reaches the code below (irrelevant parts of pwndbg
> UI are omitted):
> ```
> pwndbg> set $decode=&decode
> pwndbg> watch $decode->op[0].ot
> Hardware watchpoint 3: $decode->op[0].ot
> pwndbg> c
> Continuing.
>
> Thread 1 hit Hardware watchpoint 3: $decode->op[0].ot
>
> Old value = MO_8
> New value = MO_256
> decode_op_size (s=0x7b1a3af02020, e=0x7b1a3ae08e20, size=X86_SIZE_x,
> ot=0x7b1a3ae08e44) at ../target/i386/tcg/decode-new.c.inc:2289
> 2289 return true;
> => 0x000055e7af160d5a <decode_op_size+4186>: c6 45 ff 01
> mov BYTE PTR [rbp-0x1],0x1
> 0x000055e7af160d5e <decode_op_size+4190>: e9 91 04 00 00
> jmp 0x55e7af1611f4 <decode_op_size+5364>
> ...
> ─[ SOURCE (CODE) ]─
> In file: .../QEMU/target/i386/tcg/decode-new.c.inc:2289
> 2283 return true;
> 2284 }
> 2285 /* fall through */
> 2286 case X86_SIZE_ps: /* SSE/AVX packed single precision */
> 2287 case X86_SIZE_pd: /* SSE/AVX packed double precision */
> 2288 *ot = s->vex_l ? MO_256 : MO_128;
> ► 2289 return true;
> 2290
> 2291 case X86_SIZE_xh: /* SSE/AVX packed half register */
> 2292 *ot = s->vex_l ? MO_128 : MO_64;
> 2293 return true;
> 2294
> 2295 case X86_SIZE_d64: /* Default to 64-bit in 64-bit mode */
> 2296 *ot = CODE64(s) && s->dflag == MO_32 ? MO_64 : s->dflag;
> ...
> ─[ BACKTRACE ]─
> ► 0 0x55e7af160d5a decode_op_size+4186
> 1 0x55e7af0b6598 decode_insn+1288
> 2 0x55e7af0b1edd disas_insn+14861
> 3 0x55e7af0ac8a9 i386_tr_translate_insn+1385
> 4 0x55e7aed5a57d translator_loop+5261
> 5 0x55e7af0a99d4 x86_translate_code+500
> 6 0x55e7aed56cdc setjmp_gen_code+1052
> 7 0x55e7aed53f45 tb_gen_code+4933
> ...
> pwndbg> p s->vex_l
> $5 = 0x1
> pwndbg>
> ```
>
> Then `validate_vex()` is called, which sets the `s->vex_l` to 0. But
> by the time it is called, it is already too late:
> ```
> pwndbg> set $s=s
> pwndbg> watch $s->vex_l
> Hardware watchpoint 4: $s->vex_l
> pwndbg> c
> Continuing.
>
> Thread 1 hit Hardware watchpoint 4: $s->vex_l
>
> Old value = 0x1
> New value = 0x0
> validate_vex (s=0x7b1a3af02020, decode=0x7b1a3ae08e20) at
> ../target/i386/tcg/decode-new.c.inc:2889
> 2889 break;
> => 0x000055e7af0ba0ad <validate_vex+1645>: e9 69 0a 00 00
> jmp 0x55e7af0bab1b <validate_vex+4315>
> ...
> ─[ SOURCE (CODE) ]─
> In file: /home/user/Projects/QEMU/target/i386/tcg/decode-new.c.inc:2889
> 2883 switch (e->vex_special) {
> 2884 case X86_VEX_None:
> 2885 break;
> 2886
> 2887 case X86_VEX_LIG:
> 2888 s->vex_l = 0;
> ► 2889 break;
> 2890
> 2891 case X86_VEX_REPScalar:
> 2892 /*
> 2893 * Instructions which differ between 00/66 and F2/F3 in the
> 2894 * exception classification and the size of the memory
> operand.
> 2895 */
> 2896 assert(e->vex_class == 1 || e->vex_class == 2 ||
> e->vex_class == 4);
> ...
> ─[ BACKTRACE ]─
> ► 0 0x55e7af0ba0ad validate_vex+1645
> 1 0x55e7af0b2948 disas_insn+17528
> 2 0x55e7af0ac8a9 i386_tr_translate_insn+1385
> 3 0x55e7aed5a57d translator_loop+5261
> 4 0x55e7af0a99d4 x86_translate_code+500
> 5 0x55e7aed56cdc setjmp_gen_code+1052
> 6 0x55e7aed53f45 tb_gen_code+4933
> 7 0x55e7aed294da cpu_exec_loop+1738
> ...
> pwndbg> p *decode
> $6 = {
> e = {
> {
> gen = 0x55e7af15a3b0 <gen_VROUNDSS>,
> decode = 0x55e7af15a3b0 <gen_VROUNDSS>
> },
> op0 = X86_TYPE_V,
> s0 = X86_SIZE_x,
> op1 = X86_TYPE_H,
> s1 = X86_SIZE_x,
> op2 = X86_TYPE_W,
> s2 = X86_SIZE_ss,
> op3 = X86_TYPE_I,
> s3 = X86_SIZE_b,
> special = X86_SPECIAL_None,
> ext0 = X86_EXT_None,
> ext1 = X86_EXT_None,
> ext2 = X86_EXT_None,
> cpuid = X86_FEAT_SSE41,
> vex_class = 0x3,
> vex_special = X86_VEX_LIG,
> valid_prefix = 0x100,
> check = 0x0,
> intercept = 0x0,
> has_intercept = 0x0,
> is_decode = 0x0
> },
> op = {{
> n = 0x0,
> ot = MO_256,
> unit = X86_OP_SKIP,
> ext = X86_EXT_None,
> has_ea = 0x0,
> offset = 0x0,
> {
> imm = 0x0,
> v_ptr = 0x0
> }
> }, {
> n = 0x0,
> ot = MO_256,
> unit = X86_OP_SKIP,
> ext = X86_EXT_None,
> has_ea = 0x0,
> offset = 0x0,
> {
> imm = 0x0,
> v_ptr = 0x0
> }
> }, {
> n = 0x0,
> ot = MO_32,
> unit = X86_OP_SKIP,
> ext = X86_EXT_None,
> has_ea = 0x0,
> offset = 0x0,
> {
> imm = 0x0,
> v_ptr = 0x0
> }
> }},
> immediate = 0x0,
> mem = {
> def_seg = 0x0,
> base = 0x0,
> index = 0x0,
> scale = 0x0,
> disp = 0x0
> },
> cc_dst = 0x0,
> cc_src = 0x0,
> cc_src2 = 0x0,
> cc_op_dynamic = 0x0,
> cc_op = 0xff,
> b = 0xa
> }
> pwndbg> p s->pc_save
> $7 = 0x201194
> pwndbg> x/i s->pc_save
> 0x201194: vroundss xmm0,xmm0,xmm1,0x0
> pwndbg> x/6xb s->pc_save
> 0x201194: 0xc4 0xe3 0xfd 0x0a 0xc1 0x00
> pwndbg>
> ```
>
> Since the operand size is already set to `MO_256`, that size will
> eventually be used by the TCG frontend. This is seen in the TCG ops
> dump (annotated a little for better readability):
> ```
> $ .../QEMU/build_asan_ubsan_dbg/qemu-x86_64 -d in_asm,op
> --one-insn-per-tb ./vroundss_test
> ...
> # VEX.L = 0
> ----------------
> IN:
> 0x0020117a: c4 e3 f9 0a c1 00 vroundss $0, %xmm1, %xmm0, %xmm0
>
> OP:
> ld_i32 loc3,env,$0xffffffffffffffec
> brcond_i32 loc3,$0x0,lt,$L0
>
> ---- 000000000020117a 000000000000003c 0000000000000000
> add_i64 loc5,env,$0x3c0
> add_i64 loc7,env,$0x3c0
> add_i64 loc8,env,$0x400
> call roundss_xmm,$0x0,$0,env,loc5,loc7,loc8,$0x0
> # v128 ops present in VEX.L = 0 case!
> mov_vec v128,e8,tmp10,v128$0x0
> st_vec v128,e8,tmp10,env,$0x3d0
> mov_i64 rip,$0x201180
> exit_tb $0x0
> set_label $L0
> exit_tb $0x7b1a30a003c3
> ...
> # VEX.L = 1
> ----------------
> IN:
> 0x00201194: c4 e3 fd 0a c1 00 vroundss $0, %xmm1, %xmm0, %xmm0
>
> OP:
> ld_i32 loc3,env,$0xffffffffffffffec
> brcond_i32 loc3,$0x0,lt,$L0
>
> ---- 0000000000201194 000000000000003c 0000000000000000
> add_i64 loc5,env,$0x3c0
> add_i64 loc7,env,$0x3c0
> add_i64 loc8,env,$0x400
> call roundss_xmm,$0x0,$0,env,loc5,loc7,loc8,$0x0
> # No v128 ops in VEX.L = 1 case!
> mov_i64 rip,$0x20119a
> exit_tb $0x0
> set_label $L0
> exit_tb $0x7b1a30a00883
> ...
> ----------------
> IN:
> 0x002011bf: f4 hlt
>
> OP:
> ld_i32 loc3,env,$0xffffffffffffffec
> brcond_i32 loc3,$0x0,lt,$L0
>
> ---- 00000000002011bf 000000000000003c 0000000000000000
> mov_i64 rip,$0x2011bf
> call raise_exception,$0xa,$0,env,$0xd
> set_label $L0
> exit_tb $0x7b1a30a01003
>
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> ```
>
> Test binary source below:
> ```nasm
> bits 64
> default rel
>
> global _start
> _start:
> mov dword [rounding_mode], 0x40200000
>
> vmovups ymm0, [garbage]
> vmovups ymm1, [rounding_mode]
> ; VEX.L = 0
> db 0xc4, 0xe3, 0xf9, 0x0a, 0xc1, 0x00
> vmovups ymm3, ymm0
>
> vmovups ymm0, [garbage]
> vmovups ymm1, [rounding_mode]
> ; VEX.L = 1
> db 0xc4, 0xe3, 0xfd, 0x0a, 0xc1, 0x00
>
> vmovups [out_vex_l_0], ymm3
> vmovups [out_vex_l_1], ymm0
>
> vpxor ymm2, ymm0, ymm3
> vptest ymm2, ymm2
> jnz .bug_detected
>
> mov rax, 60
> xor rdi, rdi
> syscall
>
> .bug_detected:
> hlt
>
> section .data
> align 32
> garbage: times 32 db 0xAA
> rounding_mode: times 32 db 0xBB
>
> out_vex_l_0: times 32 db 0xff
> out_vex_l_1: times 32 db 0xff
> ```
> ```
> $ nasm -f elf64 vroundss_test.asm
> $ ld.lld -o vroundss_test vroundss_test.o
> ```
>
> And here are the results in the `out_vex_l_0` and `out_vex_l_1`:
> ```
> $ .../QEMU/build_asan_ubsan_dbg/qemu-x86_64 -d in_asm,op
> --one-insn-per-tb -g 1234 ./vroundss_test
> ```
> ```
> pwndbg> x/i $pc
> => 0x2011b3 <_start+83>: jne 0x2011bf <_start.bug_detected>
> pwndbg> info r eflags
> eflags 0x203 [ IOPL=0 IF CF ]
> pwndbg> x/32xb &out_vex_l_0
> 0x202200: 0x00 0x00 0x00 0x40 0xaa 0xaa 0xaa
> 0xaa
> 0x202208: 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa
> 0xaa
> 0x202210: 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00
> 0x202218: 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00
> pwndbg> x/32xb &out_vex_l_1
> 0x202220: 0x00 0x00 0x00 0x40 0xaa 0xaa 0xaa
> 0xaa
> 0x202228: 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa
> 0xaa
> 0x202230: 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa
> 0xaa
> 0x202238: 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa 0xaa
> 0xaa
> pwndbg>
> ```
>
> (Sorry for the late response, got burned out a little recently. Still,
> couldn't let this issue go into the upstream unnoticed)
>
>