Avoid memory corruption in arm context save/restore code

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);
 }

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,17 +750,17 @@ 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 {                                                                   \
         p += sizeof(uint32_t);                                                 \
     }
 
     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;

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}};

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}};