Skip to content
This repository was archived by the owner on Jan 28, 2023. It is now read-only.

Commit 98c8126

Browse files
wcwangwcwang
authored
Merge pull request #135 from kryptoslogic/fix-bitop
Fixed out-of-bounds behavior for btx mN,rN instructions
2 parents ff53025 + 9a3e342 commit 98c8126

File tree

2 files changed

+134
-16
lines changed

2 files changed

+134
-16
lines changed

‎core/emulate.c‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
#define INSN_NOFLAGS ((uint64_t)1 << 6)
4848
/* Instruction has two memory operands */
4949
#define INSN_TWOMEM ((uint64_t)1 << 7)
50+
/* Instruction takes bit test operands */
51+
#define INSN_BITOP ((uint64_t)1 << 8)
5052
/* String instruction */
5153
#define INSN_STRING (INSN_REP|INSN_REPX)
5254

@@ -246,19 +248,19 @@ static const struct em_opcode_t opcode_table_0F[256] = {
246248
X16(N), X16(N),
247249
/* 0xA0 - 0xAF */
248250
X3(N),
249-
F(em_bt, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM),
251+
F(em_bt, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM | INSN_BITOP),
250252
X7(N),
251-
F(em_bts, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM),
253+
F(em_bts, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM | INSN_BITOP),
252254
X4(N),
253255
/* 0xB0 - 0xBF */
254256
X3(N),
255-
F(em_btr, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM),
257+
F(em_btr, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM | INSN_BITOP),
256258
X2(N),
257259
I(em_movzx, op_modrm_reg, op_modrm_rm8, op_none, INSN_MODRM | INSN_MOV),
258260
I(em_movzx, op_modrm_reg, op_modrm_rm16, op_none, INSN_MODRM | INSN_MOV),
259261
X2(N),
260-
G(opcode_group8, op_modrm_rm, op_simm8, op_none, 0),
261-
F(em_btc, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM),
262+
G(opcode_group8, op_modrm_rm, op_simm8, op_none, INSN_BITOP),
263+
F(em_btc, op_modrm_rm, op_modrm_reg, op_none, INSN_MODRM | INSN_BITOP),
262264
X2(N),
263265
I(em_movsx, op_modrm_reg, op_modrm_rm8, op_none, INSN_MODRM | INSN_MOV),
264266
I(em_movsx, op_modrm_reg, op_modrm_rm16, op_none, INSN_MODRM | INSN_MOV),
@@ -382,6 +384,41 @@ static em_status_t segmented_write(struct em_context_t *ctxt,
382384
return ctxt->ops->write_memory(ctxt->vcpu, la, data, size, flags);
383385
}
384386

387+
static em_status_t operand_read_bitop(struct em_context_t *ctxt)
388+
{
389+
em_status_t rc;
390+
struct em_operand_t *base = &ctxt->dst;
391+
struct em_operand_t *offs = &ctxt->src1;
392+
int64_t offset;
393+
394+
if (base->flags & OP_READ_PENDING) {
395+
rc = ctxt->ops->read_memory_post(ctxt->vcpu, &base->value, base->size);
396+
return rc;
397+
}
398+
399+
offset = READ_GPR(offs->reg.index, offs->size);
400+
switch (offs->size) {
401+
case 2:
402+
offset = (int16_t)offset;
403+
break;
404+
case 4:
405+
offset = (int32_t)offset;
406+
break;
407+
case 8:
408+
offset = (int64_t)offset;
409+
break;
410+
default:
411+
return EM_ERROR;
412+
}
413+
base->mem.ea += (offset >> 3);
414+
offs->value = (offset & 0x7);
415+
rc = segmented_read(ctxt, &base->mem, &base->value, base->size);
416+
if (rc != EM_CONTINUE) {
417+
base->flags |= OP_READ_PENDING;
418+
}
419+
return rc;
420+
}
421+
385422
static em_status_t operand_read(struct em_context_t *ctxt,
386423
struct em_operand_t *op)
387424
{
@@ -1072,17 +1109,24 @@ em_status_t EMCALL em_emulate_insn(struct em_context_t *ctxt)
10721109
if (!(opcode->flags & INSN_NOFLAGS)) {
10731110
ctxt->rflags = ctxt->ops->read_rflags(ctxt->vcpu);
10741111
}
1075-
if (!(opcode->flags & INSN_MOV)) {
1076-
rc = operand_read(ctxt, &ctxt->dst);
1112+
if (opcode->flags & INSN_BITOP &&
1113+
ctxt->dst.type == OP_MEM && ctxt->src1.type == OP_REG) {
1114+
rc = operand_read_bitop(ctxt);
1115+
if (rc != EM_CONTINUE)
1116+
goto exit;
1117+
} else {
1118+
if (!(opcode->flags & INSN_MOV)) {
1119+
rc = operand_read(ctxt, &ctxt->dst);
1120+
if (rc != EM_CONTINUE)
1121+
goto exit;
1122+
}
1123+
rc = operand_read(ctxt, &ctxt->src1);
1124+
if (rc != EM_CONTINUE)
1125+
goto exit;
1126+
rc = operand_read(ctxt, &ctxt->src2);
10771127
if (rc != EM_CONTINUE)
10781128
goto exit;
10791129
}
1080-
rc = operand_read(ctxt, &ctxt->src1);
1081-
if (rc != EM_CONTINUE)
1082-
goto exit;
1083-
rc = operand_read(ctxt, &ctxt->src2);
1084-
if (rc != EM_CONTINUE)
1085-
goto exit;
10861130

10871131
// Emulate instruction
10881132
if (opcode->flags & INSN_FASTOP) {

‎tests/test_emulator.cpp‎

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ void test_advance_rip(void* obj, uint64_t len) {
9797
em_status_t test_read_memory(void* obj, uint64_t ea, uint64_t* value,
9898
uint32_t size, uint32_t flags) {
9999
test_cpu_t* vcpu = reinterpret_cast<test_cpu_t*>(obj);
100-
ea &= 0xFF;
101100
if (ea + size >= 0x100) {
102101
return EM_ERROR;
103102
}
@@ -123,7 +122,6 @@ em_status_t test_read_memory(void* obj, uint64_t ea, uint64_t* value,
123122
em_status_t test_write_memory(void* obj, uint64_t ea, uint64_t* value,
124123
uint32_t size, uint32_t flags) {
125124
test_cpu_t* vcpu = reinterpret_cast<test_cpu_t*>(obj);
126-
ea &= 0xFF;
127125
if (ea + size > 0x100) {
128126
return EM_ERROR;
129127
}
@@ -495,10 +493,10 @@ class EmulatorTest : public testing::Test {
495493
if (N == 64 && sizeof(void*) < 8) {
496494
return;
497495
}
496+
// Only bit-test variants that implicitly use bit offset modulo width.
498497
test_insn_rN_rM<N>(insn_name, tests);
499498
test_insn_rN_iM<N,8>(insn_name, tests);
500499
test_insn_mN_iM<N,8>(insn_name, tests);
501-
test_insn_mN_rM<N>(insn_name, tests);
502500
}
503501

504502
template <int N>
@@ -629,6 +627,24 @@ TEST_F(EmulatorTest, insn_bt) {
629627
{ 0x80000000'00000000ULL, 0xFF, 0,
630628
0x80000000'00000000ULL, RFLAGS_CF },
631629
});
630+
631+
// Variant `bt mN,rN`: Modifies the EA based on the bit offset.
632+
test_cpu_t vcpu_original;
633+
test_cpu_t vcpu_expected;
634+
vcpu_original = {};
635+
vcpu_original.gpr[REG_RCX] = 0ULL;
636+
(uint64_t&)vcpu_original.mem[0x00] = 0x00020000'00000000;
637+
(uint64_t&)vcpu_original.mem[0x08] = 0x00000000'00000000;
638+
(uint64_t&)vcpu_original.mem[0x10] = 0xFFFFFFFF'FFFFFFFE;
639+
640+
vcpu_original.gpr[REG_RAX] = -15LL;
641+
vcpu_expected = vcpu_original;
642+
vcpu_expected.flags |= RFLAGS_CF;
643+
run("bt [ecx + 0x08], eax", vcpu_original, vcpu_expected);
644+
vcpu_original.gpr[REG_RAX] = +64LL;
645+
vcpu_expected = vcpu_original;
646+
vcpu_expected.flags &= ~RFLAGS_CF;
647+
run("bt [rcx + 0x08], rax", vcpu_original, vcpu_expected);
632648
}
633649

634650
TEST_F(EmulatorTest, insn_btc) {
@@ -650,6 +666,26 @@ TEST_F(EmulatorTest, insn_btc) {
650666
{ 0x80000000'00000000ULL, 0xFF, 0,
651667
0x00000000'00000000ULL, RFLAGS_CF },
652668
});
669+
670+
// Variant `btc mN,rN`: Modifies the EA based on the bit offset.
671+
test_cpu_t vcpu_original;
672+
test_cpu_t vcpu_expected;
673+
vcpu_original = {};
674+
vcpu_original.gpr[REG_RCX] = 0ULL;
675+
(uint64_t&)vcpu_original.mem[0x00] = 0x00020000'00000000;
676+
(uint64_t&)vcpu_original.mem[0x08] = 0x00000000'00000000;
677+
(uint64_t&)vcpu_original.mem[0x10] = 0xFFFFFFFF'FFFFFFFE;
678+
679+
vcpu_original.gpr[REG_RAX] = -15LL;
680+
vcpu_expected = vcpu_original;
681+
vcpu_expected.flags |= RFLAGS_CF;
682+
(uint64_t&)vcpu_expected.mem[0x00] = 0x00000000'00000000;
683+
run("btc [ecx + 0x08], eax", vcpu_original, vcpu_expected);
684+
vcpu_original.gpr[REG_RAX] = +64LL;
685+
vcpu_expected = vcpu_original;
686+
vcpu_expected.flags &= ~RFLAGS_CF;
687+
(uint64_t&)vcpu_expected.mem[0x10] = 0xFFFFFFFF'FFFFFFFF;
688+
run("btc [rcx + 0x08], rax", vcpu_original, vcpu_expected);
653689
}
654690

655691
TEST_F(EmulatorTest, insn_btr) {
@@ -671,6 +707,25 @@ TEST_F(EmulatorTest, insn_btr) {
671707
{ 0x80000000'00000000ULL, 0xFF, 0,
672708
0x00000000'00000000ULL, RFLAGS_CF },
673709
});
710+
711+
// Variant `btr mN,rN`: Modifies the EA based on the bit offset.
712+
test_cpu_t vcpu_original;
713+
test_cpu_t vcpu_expected;
714+
vcpu_original = {};
715+
vcpu_original.gpr[REG_RCX] = 0ULL;
716+
(uint64_t&)vcpu_original.mem[0x00] = 0x00020000'00000000;
717+
(uint64_t&)vcpu_original.mem[0x08] = 0x00000000'00000000;
718+
(uint64_t&)vcpu_original.mem[0x10] = 0xFFFFFFFF'FFFFFFFE;
719+
720+
vcpu_original.gpr[REG_RAX] = -15LL;
721+
vcpu_expected = vcpu_original;
722+
vcpu_expected.flags |= RFLAGS_CF;
723+
(uint64_t&)vcpu_expected.mem[0x00] = 0x00000000'00000000;
724+
run("btr [ecx + 0x08], eax", vcpu_original, vcpu_expected);
725+
vcpu_original.gpr[REG_RAX] = +64LL;
726+
vcpu_expected = vcpu_original;
727+
vcpu_expected.flags &= ~RFLAGS_CF;
728+
run("btr [rcx + 0x08], rax", vcpu_original, vcpu_expected);
674729
}
675730

676731
TEST_F(EmulatorTest, insn_bts) {
@@ -692,6 +747,25 @@ TEST_F(EmulatorTest, insn_bts) {
692747
{ 0x80000000'00000000ULL, 0xFF, 0,
693748
0x80000000'00000000ULL, RFLAGS_CF },
694749
});
750+
751+
// Variant `bts mN,rN`: Modifies the EA based on the bit offset.
752+
test_cpu_t vcpu_original;
753+
test_cpu_t vcpu_expected;
754+
vcpu_original = {};
755+
vcpu_original.gpr[REG_RCX] = 0ULL;
756+
(uint64_t&)vcpu_original.mem[0x00] = 0x00020000'00000000;
757+
(uint64_t&)vcpu_original.mem[0x08] = 0x00000000'00000000;
758+
(uint64_t&)vcpu_original.mem[0x10] = 0xFFFFFFFF'FFFFFFFE;
759+
760+
vcpu_original.gpr[REG_RAX] = -15LL;
761+
vcpu_expected = vcpu_original;
762+
vcpu_expected.flags |= RFLAGS_CF;
763+
run("bts [ecx + 0x08], eax", vcpu_original, vcpu_expected);
764+
vcpu_original.gpr[REG_RAX] = +64LL;
765+
vcpu_expected = vcpu_original;
766+
vcpu_expected.flags &= ~RFLAGS_CF;
767+
(uint64_t&)vcpu_expected.mem[0x10] = 0xFFFFFFFF'FFFFFFFF;
768+
run("bts [rcx + 0x08], rax", vcpu_original, vcpu_expected);
695769
}
696770

697771
TEST_F(EmulatorTest, insn_movs) {

0 commit comments

Comments
 (0)