From 97b10da1334c4a8da17db40fc60e5d1884657d1f Mon Sep 17 00:00:00 2001 From: Andrew Dutcher Date: Thu, 14 Jul 2016 09:40:45 -0700 Subject: [PATCH 1/2] Undo the disaster that was the patch to unicorn github issue #266 and fix it correctly. makes normal self-modifying code work. --- include/uc_priv.h | 1 + qemu/target-arm/translate-a64.c | 3 +++ qemu/target-arm/translate.c | 3 +++ qemu/target-i386/translate.c | 8 +++++--- qemu/target-m68k/translate.c | 3 +++ qemu/target-mips/translate.c | 3 +++ qemu/target-sparc/translate.c | 1 + qemu/translate-all.c | 8 ++++---- tests/regress/invalid_read_in_cpu_tb_exec.c | 2 +- 9 files changed, 24 insertions(+), 8 deletions(-) diff --git a/include/uc_priv.h b/include/uc_priv.h index a0879676..ecd74b26 100644 --- a/include/uc_priv.h +++ b/include/uc_priv.h @@ -240,6 +240,7 @@ struct uc_struct { int thumb; // thumb mode for ARM // full TCG cache leads to middle-block break in the last translation? bool block_full; + int size_arg; // what tcg arg slot do we need to update with the size of the block? MemoryRegion **mapped_blocks; uint32_t mapped_block_count; uint32_t mapped_block_cache_index; diff --git a/qemu/target-arm/translate-a64.c b/qemu/target-arm/translate-a64.c index e869da01..11790a32 100644 --- a/qemu/target-arm/translate-a64.c +++ b/qemu/target-arm/translate-a64.c @@ -11115,7 +11115,10 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu, if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { // save block address to see if we need to patch block size later env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); + } else { + env->uc->size_arg = -1; } gen_tb_start(tcg_ctx); diff --git a/qemu/target-arm/translate.c b/qemu/target-arm/translate.c index d097c236..2ac5f47d 100644 --- a/qemu/target-arm/translate.c +++ b/qemu/target-arm/translate.c @@ -11233,7 +11233,10 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu, if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { // save block address to see if we need to patch block size later env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); + } else { + env->uc->size_arg = -1; } gen_tb_start(tcg_ctx); diff --git a/qemu/target-i386/translate.c b/qemu/target-i386/translate.c index e176c417..771b294b 100644 --- a/qemu/target-i386/translate.c +++ b/qemu/target-i386/translate.c @@ -8388,15 +8388,17 @@ static inline void gen_intermediate_code_internal(uint8_t *gen_opc_cc_op, dc->is_jmp = DISAS_NEXT; lj = -1; max_insns = tb->cflags & CF_COUNT_MASK; - if (max_insns <= 1) + if (max_insns == 0) max_insns = CF_COUNT_MASK; // Unicorn: trace this block on request - // Only hook this block if it is not broken from previous translation due to - // full translation cache + // Only hook this block if the previous block was not truncated due to space if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); + } else { + env->uc->size_arg = -1; } gen_tb_start(tcg_ctx); diff --git a/qemu/target-m68k/translate.c b/qemu/target-m68k/translate.c index bf45a66a..f50ef2b4 100644 --- a/qemu/target-m68k/translate.c +++ b/qemu/target-m68k/translate.c @@ -3109,7 +3109,10 @@ gen_intermediate_code_internal(M68kCPU *cpu, TranslationBlock *tb, if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { // save block address to see if we need to patch block size later env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); + } else { + env->uc->size_arg = -1; } gen_tb_start(tcg_ctx); diff --git a/qemu/target-mips/translate.c b/qemu/target-mips/translate.c index ea9aa453..cd6255b8 100644 --- a/qemu/target-mips/translate.c +++ b/qemu/target-mips/translate.c @@ -19217,7 +19217,10 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { // save block address to see if we need to patch block size later env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); + } else { + env->uc->size_arg = -1; } gen_tb_start(tcg_ctx); diff --git a/qemu/target-sparc/translate.c b/qemu/target-sparc/translate.c index 77dc4cb8..f1e1c801 100644 --- a/qemu/target-sparc/translate.c +++ b/qemu/target-sparc/translate.c @@ -5421,6 +5421,7 @@ static inline void gen_intermediate_code_internal(SPARCCPU *cpu, if (!env->uc->block_full && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, pc_start)) { // save block address to see if we need to patch block size later env->uc->block_addr = pc_start; + env->uc->size_arg = tcg_ctx->gen_opparam_buf - tcg_ctx->gen_opparam_ptr + 1; gen_uc_tracecode(tcg_ctx, 0xf8f8f8f8, UC_HOOK_BLOCK_IDX, env->uc, pc_start); } diff --git a/qemu/translate-all.c b/qemu/translate-all.c index a476c0f7..c9787ca1 100644 --- a/qemu/translate-all.c +++ b/qemu/translate-all.c @@ -179,12 +179,12 @@ static int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int *gen_code_s gen_intermediate_code(env, tb); - // Unicorn: when tracing block, patch 1st operand for block size - if (env->uc->block_addr == tb->pc && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, tb->pc)) { + // Unicorn: when tracing block, patch block size operand for callback + if (env->uc->size_arg != -1 && HOOK_EXISTS_BOUNDED(env->uc, UC_HOOK_BLOCK, tb->pc)) { if (env->uc->block_full) // block size is unknown - *(s->gen_opparam_buf + 1) = 0; + *(s->gen_opparam_buf + env->uc->size_arg) = 0; else - *(s->gen_opparam_buf + 1) = tb->size; + *(s->gen_opparam_buf + env->uc->size_arg) = tb->size; } /* generate machine code */ diff --git a/tests/regress/invalid_read_in_cpu_tb_exec.c b/tests/regress/invalid_read_in_cpu_tb_exec.c index d618c31a..7654c07e 100644 --- a/tests/regress/invalid_read_in_cpu_tb_exec.c +++ b/tests/regress/invalid_read_in_cpu_tb_exec.c @@ -1,7 +1,7 @@ #include static void hook_block(uc_engine *uc, uint64_t address, uint32_t size, void *user_data) { - printf("hook_block(…)\n"); + printf("hook_block(%p, %lx, %d, %p)\n", uc, address, size, user_data); } /* From 42949408cae76c1d54c496648b6eb9a6f4a259cd Mon Sep 17 00:00:00 2001 From: Andrew Dutcher Date: Sun, 14 Aug 2016 23:21:38 -0700 Subject: [PATCH 2/2] Added test for real self-modifying code --- tests/regress/x86_self_modifying.elf | Bin 0 -> 720 bytes tests/regress/x86_self_modifying.py | 37 +++++++++++++++++++ tests/regress/x86_self_modifying.s | 51 +++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100755 tests/regress/x86_self_modifying.elf create mode 100755 tests/regress/x86_self_modifying.py create mode 100644 tests/regress/x86_self_modifying.s diff --git a/tests/regress/x86_self_modifying.elf b/tests/regress/x86_self_modifying.elf new file mode 100755 index 0000000000000000000000000000000000000000..c8613d04016c346d8b43fb2efb52026409cb1b6f GIT binary patch literal 720 zcma)4OG^S#6h7*VDS_FpXi-6nAe02%_rNHKpg+(s=1d8`aBh&R3^y4F{eb8f)V`oa zAhip^rMZd(Nh)eri|jkW><-Eh0!UM-arjCLz?X950{x};s{q1HA58H!5PED^ zFZqo7Lh*})ckNj59VsKN&&qNWQQG?17!Fm7Z|>r^D#m-wW%Ue`+7C+@a2KUY>xoPM z3h%{H-mAL0B-cOLV_ z0nYR5wmey4&oqM>vW%pur84n^nNOs5C}pI!vw6~znM_u*gq{;U-$=(v(_)q-qefg8 zI#JNdr-U8@id+|Wr6-Jifn)zI6R6~Sb3=JIZZ@G(