diff --git a/qemu/target/arm/unicorn_aarch64.c b/qemu/target/arm/unicorn_aarch64.c index 987e1180dd..e2ecb1c85e 100644 --- a/qemu/target/arm/unicorn_aarch64.c +++ b/qemu/target/arm/unicorn_aarch64.c @@ -461,6 +461,11 @@ void uc_init(struct uc_struct *uc) uc->release = arm64_release; uc->cpus_init = arm64_cpus_init; uc->insn_hook_validate = arm64_insn_hook_validate; + // n.b. The following results in memcpy of pointers from CPUARMState to + // context buffers and vice versa. That is currently safe for AArch64 + // because none of the pointer fields of CPUARMState are allocated for any + // of the AArch64 CPUs, and none of reg_read/reg_write, when called on a + // context, accesses fields including and beyond cpu_watchpoint. uc->cpu_context_size = offsetof(CPUARMState, cpu_watchpoint); uc_common_init(uc); } diff --git a/qemu/target/arm/unicorn_arm.c b/qemu/target/arm/unicorn_arm.c index 7ecd0416d6..f0c502dade 100644 --- a/qemu/target/arm/unicorn_arm.c +++ b/qemu/target/arm/unicorn_arm.c @@ -630,9 +630,18 @@ static int arm_cpus_init(struct uc_struct *uc, const char *cpu_model) return 0; } +// The strategy is to copy the top part of CPUARMState that does not contain +// any pointers into the context buffer, and zero-fill the remainder. For all +// fields in the pointer range, data that needs to be saved to the context must +// be manually appended to the end of the buffer. We need to zero the pointer +// portion in the context buffer, because reg_read/reg_write are accessing some +// of these fields when accessing registers in a context. +#define CONTEXT_COPYSIZE offsetof(CPUARMState, pmsav7) +#define CONTEXT_ZEROSIZE (sizeof(CPUARMState) - CONTEXT_COPYSIZE) + static size_t uc_arm_context_size(struct uc_struct *uc) { - size_t ret = offsetof(CPUARMState, cpu_watchpoint); + size_t ret = CONTEXT_COPYSIZE + CONTEXT_ZEROSIZE; ARMCPU *cpu = (ARMCPU *)uc->cpu; CPUARMState *env = (CPUARMState *)&cpu->env; uint32_t nr; @@ -708,8 +717,10 @@ static uc_err uc_arm_context_save(struct uc_struct *uc, uc_context *context) p += sizeof(uint32_t) * nr; \ } p = context->data; - memcpy(p, uc->cpu->env_ptr, uc->cpu_context_size); - p += uc->cpu_context_size; + memcpy(p, uc->cpu->env_ptr, CONTEXT_COPYSIZE); + p += CONTEXT_COPYSIZE; + memset(p, 0, CONTEXT_ZEROSIZE); + p += CONTEXT_ZEROSIZE; nr = cpu->pmsav7_dregion; ARM_ENV_SAVE(env->pmsav7.drbar) @@ -739,8 +750,8 @@ static uc_err uc_arm_context_restore(struct uc_struct *uc, uc_context *context) ctx_nr = *(uint32_t *)p; \ if (ctx_nr != 0) { \ p += sizeof(uint32_t); \ - if (field && ctx_nr == nr) { \ - memcpy(field, p, sizeof(uint32_t) * ctx_nr); \ + if (field && nr != 0) { \ + memcpy(field, p, sizeof(uint32_t) * (ctx_nr > nr ? nr : ctx_nr)); \ } \ p += sizeof(uint32_t) * ctx_nr; \ } else { \ @@ -748,8 +759,8 @@ static uc_err uc_arm_context_restore(struct uc_struct *uc, uc_context *context) } p = context->data; - memcpy(uc->cpu->env_ptr, p, uc->cpu_context_size); - p += uc->cpu_context_size; + memcpy(uc->cpu->env_ptr, p, CONTEXT_COPYSIZE); + p += CONTEXT_COPYSIZE + CONTEXT_ZEROSIZE; nr = cpu->pmsav7_dregion; ARM_ENV_RESTORE(env->pmsav7.drbar) @@ -765,8 +776,6 @@ static uc_err uc_arm_context_restore(struct uc_struct *uc, uc_context *context) ARM_ENV_RESTORE(env->sau.rlar) #undef ARM_ENV_RESTORE - // Overwrite uc to our uc - env->uc = uc; return UC_ERR_OK; } @@ -793,7 +802,7 @@ void uc_init(struct uc_struct *uc) uc->cpus_init = arm_cpus_init; uc->insn_hook_validate = arm_insn_hook_validate; uc->opcode_hook_invalidate = arm_opcode_hook_invalidate; - uc->cpu_context_size = offsetof(CPUARMState, cpu_watchpoint); + uc->cpu_context_size = CONTEXT_COPYSIZE + CONTEXT_ZEROSIZE; uc->context_size = uc_arm_context_size; uc->context_save = uc_arm_context_save; uc->context_restore = uc_arm_context_restore; diff --git a/tests/unit/test_arm.c b/tests/unit/test_arm.c index 3b0c7915cb..438aaa4e65 100644 --- a/tests/unit/test_arm.c +++ b/tests/unit/test_arm.c @@ -1062,6 +1062,31 @@ static void test_arm_hook_insn_wfi(void) OK(uc_close(uc)); } +static void test_arm_context(void) +{ + for (int model = 0; model < UC_CPU_ARM_ENDING; model++) { + uc_engine *uc; + uc_engine *uc2; + uc_context *ctx; + + OK(uc_open(UC_ARCH_ARM, UC_MODE_ARM, &uc)); + OK(uc_ctl_set_cpu_model(uc, model)); + + OK(uc_context_alloc(uc, &ctx)); + OK(uc_context_save(uc, ctx)); + + OK(uc_open(UC_ARCH_ARM, UC_MODE_ARM, &uc2)); + OK(uc_ctl_set_cpu_model(uc2, model)); + + OK(uc_context_restore(uc2, ctx)); + + // One of these will abort on a double-free for bug-afflicted CPU models. + OK(uc_context_free(ctx)); + OK(uc_close(uc)); + OK(uc_close(uc2)); + } +} + TEST_LIST = {{"test_arm_nop", test_arm_nop}, {"test_arm_thumb_sub", test_arm_thumb_sub}, {"test_armeb_sub", test_armeb_sub}, @@ -1094,4 +1119,5 @@ TEST_LIST = {{"test_arm_nop", test_arm_nop}, {"test_arm_v7_lpae", test_arm_v7_lpae}, {"test_arm_svc_hvc_syndrome", test_arm_svc_hvc_syndrome}, {"test_arm_hook_insn_wfi", test_arm_hook_insn_wfi}, + {"test_arm_context", test_arm_context}, {NULL, NULL}}; diff --git a/tests/unit/test_arm64.c b/tests/unit/test_arm64.c index 86f81595fe..f1ab5c3d6c 100644 --- a/tests/unit/test_arm64.c +++ b/tests/unit/test_arm64.c @@ -695,6 +695,31 @@ static void test_arm64_pc_guarantee(void) OK(uc_close(uc)); } +static void test_arm64_context(void) +{ + for (int model = 0; model < UC_CPU_ARM64_ENDING; model++) { + uc_engine *uc; + uc_engine *uc2; + uc_context *ctx; + + OK(uc_open(UC_ARCH_ARM64, UC_MODE_ARM, &uc)); + OK(uc_ctl_set_cpu_model(uc, model)); + + OK(uc_context_alloc(uc, &ctx)); + OK(uc_context_save(uc, ctx)); + + OK(uc_open(UC_ARCH_ARM64, UC_MODE_ARM, &uc2)); + OK(uc_ctl_set_cpu_model(uc2, model)); + + OK(uc_context_restore(uc2, ctx)); + + // One of these will abort on a double-free for bug-afflicted CPU models. + OK(uc_context_free(ctx)); + OK(uc_close(uc)); + OK(uc_close(uc2)); + } +} + TEST_LIST = {{"test_arm64_until", test_arm64_until}, {"test_arm64_code_patching", test_arm64_code_patching}, {"test_arm64_code_patching_count", test_arm64_code_patching_count}, @@ -714,4 +739,5 @@ TEST_LIST = {{"test_arm64_until", test_arm64_until}, {"test_arm64_mem_prot_regress", test_arm64_mem_prot_regress}, {"test_arm64_mem_hook_read_write", test_arm64_mem_hook_read_write}, {"test_arm64_pc_guarantee", test_arm64_pc_guarantee}, + {"test_arm64_context", test_arm64_context}, {NULL, NULL}};