Fix qemu snapshot module unmap (#3084)

* fix qemu usermode snapshot unmap

* fix phys addr computation for qemu. some cleanup.

Author: rmalmain

fuzzers/binary_only/qemu_cmin/Justfile

@@ -42,4 +42,4 @@ test:
 
 [unix]
 clean:
-    cargo clean
+    cargo clean

libafl_qemu/libafl_qemu_build/src/bindings.rs

@@ -169,6 +169,7 @@ pub fn generate(
         .allowlist_function("qemu_plugin_hwaddr_phys_addr")
         .allowlist_function("qemu_plugin_get_hwaddr")
         .allowlist_function("qemu_target_page_size")
+        .allowlist_function("qemu_target_page_mask")
         .allowlist_function("syx_.*")
         .allowlist_function("device_list_all")
         .allowlist_function("libafl_.*")

libafl_qemu/src/modules/usermode/snapshot.rs

@@ -154,7 +154,6 @@ pub struct SnapshotModule {
     pub mmap_limit: usize,
     pub stop_execution: Option<StopExecutionCallback>,
     pub empty: bool,
-    pub accurate_unmap: bool,
     pub interval_filter: IntervalSnapshotFilters,
     auto_reset: bool,
 }
@@ -187,7 +186,6 @@ impl SnapshotModule {
             mmap_limit: 0,
             stop_execution: None,
             empty: true,
-            accurate_unmap: false,
             interval_filter: IntervalSnapshotFilters::new(),
             auto_reset: true,
         }
@@ -206,7 +204,6 @@ impl SnapshotModule {
             mmap_limit: 0,
             stop_execution: None,
             empty: true,
-            accurate_unmap: false,
             interval_filter,
             auto_reset: true,
         }
@@ -225,16 +222,11 @@ impl SnapshotModule {
             mmap_limit,
             stop_execution: Some(stop_execution),
             empty: true,
-            accurate_unmap: false,
             interval_filter: IntervalSnapshotFilters::new(),
             auto_reset: true,
         }
     }
 
-    pub fn use_accurate_unmapping(&mut self) {
-        self.accurate_unmap = true;
-    }
-
     pub fn use_manual_reset(&mut self) {
         self.auto_reset = false;
     }
@@ -583,10 +575,13 @@ impl SnapshotModule {
         found
     }
 
+    /// Unmap is allowed if it is not part of the pre-snapshot region. maybe check if it's part
+    /// of qemu's guest memory or not?
     pub fn is_unmap_allowed(&mut self, start: GuestAddr, mut size: usize) -> bool {
         if size % SNAPSHOT_PAGE_SIZE != 0 {
             size = size + (SNAPSHOT_PAGE_SIZE - size % SNAPSHOT_PAGE_SIZE);
         }
+
         self.maps
             .tree
             .query(start..(start + (size as GuestAddr)))
@@ -803,9 +798,8 @@ where
             );
         }
 
-        if !self.accurate_unmap {
-            emulator_modules.pre_syscalls(Hook::Function(filter_mmap_snapshot::<ET, I, S>));
-        }
+        emulator_modules.pre_syscalls(Hook::Function(filter_mmap_snapshot::<ET, I, S>));
+
         emulator_modules.post_syscalls(Hook::Function(trace_mmap_snapshot::<ET, I, S>));
     }
 
@@ -870,6 +864,7 @@ pub fn trace_write_n_snapshot<ET, I, S>(
     h.access(addr, size);
 }
 
+/// Do not consider munmap syscalls that are not allowed
 #[expect(clippy::too_many_arguments)]
 pub fn filter_mmap_snapshot<ET, I, S>(
     _qemu: Qemu,
@@ -896,6 +891,7 @@ where
             return SyscallHookResult::new(Some(0));
         }
     }
+
     SyscallHookResult::new(None)
 }
 
@@ -1007,7 +1003,7 @@ where
                 }
             } else if sys_const == SYS_munmap {
                 let h = emulator_modules.get_mut::<SnapshotModule>().unwrap();
-                if !h.accurate_unmap && !h.is_unmap_allowed(a0, a1 as usize) {
+                if h.is_unmap_allowed(a0, a1 as usize) {
                     h.remove_mapped(a0, a1 as usize);
                 }
             }

libafl_qemu/src/qemu/systemmode.rs

@@ -96,11 +96,13 @@ impl CPU {
                 page as GuestVirtAddr,
                 attrs.as_mut_ptr(),
             );
+            let mask = Qemu::get_unchecked().target_page_mask();
+            let offset = vaddr & (mask as GuestVirtAddr);
             #[expect(clippy::cast_sign_loss)]
             if paddr == (-1i64 as GuestPhysAddr) {
                 None
             } else {
-                Some(paddr)
+                Some(paddr + offset)
             }
         }
     }
@@ -180,11 +182,6 @@ impl CPU {
 
 #[expect(clippy::unused_self)]
 impl Qemu {
-    #[must_use]
-    pub fn guest_page_size(&self) -> usize {
-        4096
-    }
-
     /// Write a value to a physical guest address, including ROM areas.
     ///
     /// # Safety
@@ -313,6 +310,11 @@ impl Qemu {
     pub fn target_page_size(&self) -> usize {
         unsafe { libafl_qemu_sys::qemu_target_page_size() }
     }
+
+    #[must_use]
+    pub fn target_page_mask(&self) -> usize {
+        unsafe { libafl_qemu_sys::qemu_target_page_mask() as usize }
+    }
 }
 
 impl QemuMemoryChunk {
@@ -426,7 +428,7 @@ impl Iterator for PhysMemoryIter {
                 }
             };
             let start_phys_addr: GuestPhysAddr = self.cpu.get_phys_addr(*vaddr)?;
-            let phys_page_size = self.qemu.guest_page_size();
+            let phys_page_size = self.qemu.target_page_size();
 
             // TODO: Turn this into a generic function
             let mut size_taken: usize = std::cmp::min(