From 225d3873250ec53cac280bdc6f5612716f6bcd00 Mon Sep 17 00:00:00 2001 From: lazymio Date: Wed, 12 May 2021 00:10:45 +0800 Subject: [PATCH] Fix wrong sync after UC_ERR_[READ, WRITE, FETCH]_[UNMAPPED, PROT] (#1368) * Fix wrong sync after UC_ERR_[READ, WRITE, FETCH]_[UNMAPPED, PROT] Note that: 1. We only guarantee the pc (and other internal states) is correct if and only of `uc_emu_start` returns without any error (or errors have been handled in callbacks.). 2. If memory read/write error isn't handled by hooks, the state is undefined and the pc is probably wrong if no hook is installed. This fixes #1323. * Rename variables * Add note in unicorn.h * Refine test_i386_invalid_mem_read_in_tb --- include/unicorn/unicorn.h | 4 +++ qemu/cpu-exec.c | 36 ++++++++++++++------- samples/sample_x86.c | 68 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/include/unicorn/unicorn.h b/include/unicorn/unicorn.h index de3c5ba9..a87daf2f 100644 --- a/include/unicorn/unicorn.h +++ b/include/unicorn/unicorn.h @@ -533,6 +533,10 @@ uc_err uc_mem_read(uc_engine *uc, uint64_t address, void *bytes, size_t size); we will emulate the code in infinite time, until the code is finished. @count: the number of instructions to be emulated. When this value is 0, we will emulate all the code available, until the code is finished. + + NOTE: The internal states of the engine is guranteed to be correct if and only + if uc_emu_start returns without any errors or errors have been handled in + the callbacks. @return UC_ERR_OK on success, or other value on failure (refer to uc_err enum for detailed error). diff --git a/qemu/cpu-exec.c b/qemu/cpu-exec.c index b4aa4bf0..b9559645 100644 --- a/qemu/cpu-exec.c +++ b/qemu/cpu-exec.c @@ -327,17 +327,31 @@ static tcg_target_ulong cpu_tb_exec(CPUState *cpu, uint8_t *tb_ptr) * or timer mode is in effect, since these already fix the PC. */ if (!HOOK_EXISTS(env->uc, UC_HOOK_CODE) && !env->uc->timeout) { - if (cc->synchronize_from_tb) { - // avoid sync twice when helper_uc_tracecode() already did this. - if (env->uc->emu_counter <= env->uc->emu_count && - !env->uc->stop_request && !env->uc->quit_request) - cc->synchronize_from_tb(cpu, tb); - } else { - assert(cc->set_pc); - // avoid sync twice when helper_uc_tracecode() already did this. - if (env->uc->emu_counter <= env->uc->emu_count && - !env->uc->stop_request && !env->uc->quit_request) - cc->set_pc(cpu, tb->pc); + // We should sync pc for R/W error. + switch (env->invalid_error) { + case UC_ERR_WRITE_PROT: + case UC_ERR_READ_PROT: + case UC_ERR_FETCH_PROT: + case UC_ERR_WRITE_UNMAPPED: + case UC_ERR_READ_UNMAPPED: + case UC_ERR_FETCH_UNMAPPED: + case UC_ERR_WRITE_UNALIGNED: + case UC_ERR_READ_UNALIGNED: + case UC_ERR_FETCH_UNALIGNED: + break; + default: + if (cc->synchronize_from_tb) { + // avoid sync twice when helper_uc_tracecode() already did this. + if (env->uc->emu_counter <= env->uc->emu_count && + !env->uc->stop_request && !env->uc->quit_request) + cc->synchronize_from_tb(cpu, tb); + } else { + assert(cc->set_pc); + // avoid sync twice when helper_uc_tracecode() already did this. + if (env->uc->emu_counter <= env->uc->emu_count && + !env->uc->stop_request && !env->uc->quit_request) + cc->set_pc(cpu, tb->pc); + } } } } diff --git a/samples/sample_x86.c b/samples/sample_x86.c index 1b7ed33f..31714180 100644 --- a/samples/sample_x86.c +++ b/samples/sample_x86.c @@ -15,6 +15,7 @@ #define X86_CODE32_LOOP "\x41\x4a\xeb\xfe" // INC ecx; DEC edx; JMP self-loop #define X86_CODE32_MEM_WRITE "\x89\x0D\xAA\xAA\xAA\xAA\x41\x4a" // mov [0xaaaaaaaa], ecx; INC ecx; DEC edx #define X86_CODE32_MEM_READ "\x8B\x0D\xAA\xAA\xAA\xAA\x41\x4a" // mov ecx,[0xaaaaaaaa]; INC ecx; DEC edx +#define X86_CODE32_MEM_READ_IN_TB "\x40\x8b\x1d\x00\x00\x10\x00\x42" // inc eax; mov ebx, [0x100000]; inc edx #define X86_CODE32_JMP_INVALID "\xe9\xe9\xee\xee\xee\x41\x4a" // JMP outside; INC ecx; DEC edx #define X86_CODE32_INOUT "\x41\xE4\x3F\x4a\xE6\x46\x43" // INC ecx; IN AL, 0x3f; DEC edx; OUT 0x46, AL; INC ebx @@ -81,6 +82,14 @@ static bool hook_mem_invalid(uc_engine *uc, uc_mem_type type, } } +// dummy callback +static bool hook_mem_invalid_dummy(uc_engine *uc, uc_mem_type type, + uint64_t address, int size, int64_t value, void *user_data) +{ + // Stop emulation. + return false; +} + static void hook_mem64(uc_engine *uc, uc_mem_type type, uint64_t address, int size, int64_t value, void *user_data) { @@ -1026,6 +1035,63 @@ static void test_x86_16(void) uc_close(uc); } +static void test_i386_invalid_mem_read_in_tb(void) +{ + uc_engine *uc; + uc_err err; + uc_hook trace1; + + int r_eax = 0x1234; // EAX register + int r_edx = 0x7890; // EDX register + int r_eip = 0; + + printf("===================================\n"); + printf("Emulate i386 code that read invalid memory in the middle of a TB\n"); + + // Initialize emulator in X86-32bit mode + err = uc_open(UC_ARCH_X86, UC_MODE_32, &uc); + if (err) { + printf("Failed on uc_open() with error returned: %u\n", err); + return; + } + + // map 2MB memory for this emulation + uc_mem_map(uc, ADDRESS, 2 * 1024 * 1024, UC_PROT_ALL); + + // write machine code to be emulated to memory + if (uc_mem_write(uc, ADDRESS, X86_CODE32_MEM_READ_IN_TB, sizeof(X86_CODE32_MEM_READ_IN_TB) - 1)) { + printf("Failed to write emulation code to memory, quit!\n"); + return; + } + + // initialize machine registers + uc_reg_write(uc, UC_X86_REG_EAX, &r_eax); + uc_reg_write(uc, UC_X86_REG_EDX, &r_edx); + + // Add a dummy callback. + uc_hook_add(uc, &trace1, UC_HOOK_MEM_READ, hook_mem_invalid_dummy, NULL, 1, 0); + + // Let it crash by design. + err = uc_emu_start(uc, ADDRESS, ADDRESS + sizeof(X86_CODE32_MEM_READ_IN_TB) - 1, 0, 0); + if (err) { + printf("uc_emu_start() failed BY DESIGN with error returned %u: %s\n", + err, uc_strerror(err)); + } + + printf(">>> Emulation done. Below is the CPU context\n"); + + uc_reg_read(uc, UC_X86_REG_EIP, &r_eip); + printf(">>> EIP = 0x%x\n", r_eip); + + if (r_eip != ADDRESS + 1) { + printf(">>> ERROR: Wrong PC 0x%x when reading unmapped memory in the middle of TB!\n", r_eip); + } else { + printf(">>> The PC is correct after reading unmapped memory in the middle of TB.\n"); + } + + uc_close(uc); +} + int main(int argc, char **argv, char **envp) { if (argc == 2) { @@ -1066,7 +1132,7 @@ int main(int argc, char **argv, char **envp) //test_i386_invalid_c6c7(); test_x86_64(); test_x86_64_syscall(); - + test_i386_invalid_mem_read_in_tb(); } return 0;