Snapshot use after free (#2125)

* memory snapshots fix use after free on flatview copy

When restoring a snapshot with memory the flatview must be restored
before the memory reagions are filtered. Because the
AddressSpaceDispatcher also has pointer to the MemoryRegions and on copy
they need to be cleared. The memory_filter_subregions function frees
MemoryRegions which are not used at the time of the snapshot.

* fix some memleaks in tests

These tests has forgott to call uc_close(uc), which lead to memory
leaks. Found by the LeakSanitizer.

* memory snapshots correct clean up container memory regions

* Fix further stackoverflow in tests

---------

Co-authored-by: mio <mio@lazym.io>

qemu/softmmu/memory.c

@@ -28,6 +28,7 @@
 
 void memory_region_transaction_begin(void);
 static void memory_region_transaction_commit(MemoryRegion *mr);
+static void memory_region_destructor_container(MemoryRegion *mr);
 
 typedef struct AddrRange AddrRange;
 
@@ -87,6 +88,7 @@ static void make_contained(struct uc_struct *uc, MemoryRegion *current)
     hwaddr addr = current->addr;
     MemoryRegion *container = g_new(MemoryRegion, 1);
     memory_region_init(uc, container, int128_get64(current->size));
+    container->destructor = memory_region_destructor_container;
     memory_region_del_subregion(uc->system_memory, current);
     memory_region_add_subregion_overlap(container, 0, current, current->priority);
     memory_region_add_subregion(uc->system_memory, addr, container);
@@ -1091,6 +1093,11 @@ static void memory_region_destructor_ram(MemoryRegion *mr)
     qemu_ram_free(mr->uc, mr->ram_block);
 }
 
+static void memory_region_destructor_container(MemoryRegion *mr)
+{
+    memory_region_filter_subregions(mr, 0);
+}
+
 void memory_region_init(struct uc_struct *uc,
                         MemoryRegion *mr,
                         uint64_t size)

tests/unit/test_arm.c

@@ -905,6 +905,7 @@ static void test_arm_tcg_opcode_cmp(void)
     TEST_CHECK(cmp_info.v0 == 5 && cmp_info.v1 == 3);
     TEST_CHECK(cmp_info.pc == 0x1008);
     TEST_CHECK(cmp_info.size == 32);
+    OK(uc_close(uc));
 }
 
 static void test_arm_thumb_tcg_opcode_cmn(void)
@@ -931,6 +932,7 @@ static void test_arm_thumb_tcg_opcode_cmn(void)
     TEST_CHECK(cmp_info.v0 == 5 && cmp_info.v1 == 3);
     TEST_CHECK(cmp_info.pc == 0x1006);
     TEST_CHECK(cmp_info.size == 32);
+    OK(uc_close(uc));
 }
 
 static void test_arm_cp15_c1_c0_2(void)

tests/unit/test_arm64.c

@@ -481,6 +481,7 @@ static void test_arm64_mmu(void)
     TEST_CHECK(x1 == 0x4444444444444444);
     TEST_CHECK(x2 == 0x4444444444444444);
     free(data);
+    OK(uc_close(uc));
 }
 
 static void test_arm64_pc_wrap(void)

tests/unit/test_riscv.c

@@ -718,6 +718,7 @@ static void test_riscv_mmu(void)
     OK(uc_mem_read(uc, data_address, &data_result, sizeof(data_result)));
 
     TEST_CHECK(data_value == data_result);
+    OK(uc_close(uc));
 }
 
 static void test_riscv_priv(void)

tests/unit/test_x86.c

@@ -632,7 +632,7 @@ static void test_x86_smc_add(void)
 {
     uc_engine *uc;
     uint64_t stack_base = 0x20000;
-    int r_rsp;
+    uint64_t r_rsp;
     /*
      * mov qword ptr [rip+0x10], rax
      * mov word ptr [rip], 0x0548
@@ -647,6 +647,8 @@ static void test_x86_smc_add(void)
     r_rsp = stack_base + 0x1800;
     OK(uc_reg_write(uc, UC_X86_REG_RSP, &r_rsp));
     OK(uc_emu_start(uc, code_start, -1, 0, 0));
+
+    OK(uc_close(uc));
 }
 
 static void test_x86_smc_mem_hook_callback(uc_engine *uc, uc_mem_type t,
@@ -667,7 +669,7 @@ static void test_x86_smc_mem_hook(void)
     uc_engine *uc;
     uc_hook hook;
     uint64_t stack_base = 0x20000;
-    int r_rsp;
+    uint64_t r_rsp;
     unsigned int i = 0;
     /*
      * mov qword ptr [rip+0x29], rax
@@ -689,6 +691,8 @@ static void test_x86_smc_mem_hook(void)
     r_rsp = stack_base + 0x1800;
     OK(uc_reg_write(uc, UC_X86_REG_RSP, &r_rsp));
     OK(uc_emu_start(uc, code_start, -1, 0, 0));
+
+    OK(uc_close(uc));
 }
 
 static uint64_t test_x86_mmio_uc_mem_rw_read_callback(uc_engine *uc,
@@ -1589,6 +1593,8 @@ static void test_x86_mmu(void)
     OK(uc_mem_read(uc, 0x2000, &child, sizeof(child)));
     TEST_CHECK(parrent == 60);
     TEST_CHECK(child == 42);
+    OK(uc_context_free(context));
+    OK(uc_close(uc));
 }
 
 static bool test_x86_vtlb_callback(uc_engine *uc, uint64_t addr,
@@ -1632,6 +1638,7 @@ static void test_x86_segmentation(void)
     OK(uc_open(UC_ARCH_X86, UC_MODE_64, &uc));
     OK(uc_reg_write(uc, UC_X86_REG_GDTR, &gdtr));
     uc_assert_err(UC_ERR_EXCEPTION, uc_reg_write(uc, UC_X86_REG_FS, &fs));
+    OK(uc_close(uc));
 }
 
 static void test_x86_0xff_lcall_callback(uc_engine *uc, uint64_t address,

uc.c

@@ -2439,6 +2439,10 @@ uc_err uc_context_restore(uc_engine *uc, uc_context *context)
 
     if (uc->context_content & UC_CTL_CONTEXT_MEMORY) {
         uc->snapshot_level = context->snapshot_level;
+        if (!uc->flatview_copy(uc, uc->address_space_memory.current_map,
+                               context->fv, true)) {
+            return UC_ERR_NOMEM;
+        }
         ret = uc_restore_latest_snapshot(uc);
         if (ret != UC_ERR_OK) {
             restore_jit_state(uc);
@@ -2447,10 +2451,6 @@ uc_err uc_context_restore(uc_engine *uc, uc_context *context)
         uc_snapshot(uc);
         uc->ram_list.freed = context->ramblock_freed;
         uc->ram_list.last_block = context->last_block;
-        if (!uc->flatview_copy(uc, uc->address_space_memory.current_map,
-                               context->fv, true)) {
-            return UC_ERR_NOMEM;
-        }
         uc->tcg_flush_tlb(uc);
     }