Skip to content

Commit be8dc80

Browse files
committed
target/i386: Reject ModRM.reg == 7 early when decoding 0xFE/0xFF (group 4 and 5) instructions
According to both table A-6 in Volume 2 of Intel 64 and IA-32 Architectures Software Developer's Manual and table A-6 in Volume 3 of AMD64 Architecture Programmer's Manual, the opcode for ModRM.reg == 7 is reserved for both group 4 (0xFE) and 5 (0xFF) instructions. Although Unicorn already rejects ModRM.reg == 7 early with group 4 (0xFE) instructions, that is not the case with group 5 (0xFF) instructions. The latter case is eventually handled in the `default:` block of `switch(op)`, but by the time that code is reached, some micro-ops are already generated. These may include a memory load micro-op. As a result, an invalid memory access error may be produced instead of an expected invalid instruction error. Fix this bug by rejecting group 4 and 5 instructions with ModRM.reg == 7 before generating any micro-operations. Signed-off-by: Andrey Polivoda <apolivodaa433@gmail.com>
1 parent 3a8311a commit be8dc80

2 files changed

Lines changed: 19 additions & 1 deletion

File tree

qemu/target/i386/translate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5493,7 +5493,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
54935493
mod = (modrm >> 6) & 3;
54945494
rm = (modrm & 7) | REX_B(s);
54955495
op = (modrm >> 3) & 7;
5496-
if (op >= 2 && b == 0xfe) {
5496+
if (op == 7 || (op >= 2 && b == 0xfe)) {
54975497
goto unknown_op;
54985498
}
54995499
if (CODE64(s)) {

tests/unit/test_x86.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2618,6 +2618,23 @@ static void test_x86_aas_flags(void)
26182618
OK(uc_close(uc));
26192619
}
26202620

2621+
static void test_x86_group_5_modrm_reg_7(void)
2622+
{
2623+
uc_engine *uc;
2624+
char code[] = {
2625+
0xff, (7 << 3), 0x01, 0x02, 0x03, 0x04
2626+
};
2627+
2628+
uc_common_setup(&uc, UC_ARCH_X86, UC_MODE_64, code, sizeof(code));
2629+
2630+
uint64_t rax = code_start + code_len + 0x100;
2631+
OK(uc_reg_write(uc, UC_X86_REG_RAX, &rax));
2632+
uc_assert_err(UC_ERR_INSN_INVALID,
2633+
uc_emu_start(uc, code_start, code_start + sizeof(code), 0, 0));
2634+
2635+
OK(uc_close(uc));
2636+
}
2637+
26212638
TEST_LIST = {
26222639
{"test_x86_in", test_x86_in},
26232640
{"test_x86_out", test_x86_out},
@@ -2695,4 +2712,5 @@ TEST_LIST = {
26952712
{"test_x86_mem_hooks_pc_guarantee", test_x86_mem_hooks_pc_guarantee},
26962713
{"test_x86_aaa_flags", test_x86_aaa_flags},
26972714
{"test_x86_aas_flags", test_x86_aas_flags},
2715+
{"test_x86_group_5_modrm_reg_7", test_x86_group_5_modrm_reg_7},
26982716
{NULL, NULL}};

0 commit comments

Comments
 (0)