Safe qemu cpu from index (#2941)

* safe cpu from index

* add comment

Author: rmalmain

fuzzers/full_system/qemu_baremetal/src/fuzzer_low_level.rs

@@ -171,7 +171,7 @@ pub fn fuzz() {
 
                     // If the execution stops at any point other than the designated breakpoint (e.g. a breakpoint on a panic method) we consider it a crash
                     let mut pcs = (0..qemu.num_cpus())
-                        .map(|i| qemu.cpu_from_index(i))
+                        .map(|i| qemu.cpu_from_index(i).unwrap())
                         .map(|cpu| -> Result<u32, QemuRWError> { cpu.read_reg(Regs::Pc) });
                     let ret = match pcs
                         .find(|pc| (breakpoint..breakpoint + 5).contains(pc.as_ref().unwrap_or(&0)))

libafl_qemu/src/emu/usermode.rs

@@ -22,8 +22,11 @@ impl<C, CM, ED, ET, I, S, SM> Emulator<C, CM, ED, ET, I, S, SM> {
         self.qemu.h2g(addr)
     }
 
+    /// Checks whether the access at address @addr of size @size is valid.
+    /// The acess is done relatively to the CPU as described by [`Qemu::access_ok`].
+    /// If no CPU is found, returns None.
     #[must_use]
-    pub fn access_ok(&self, kind: VerifyAccess, addr: GuestAddr, size: usize) -> bool {
+    pub fn access_ok(&self, kind: VerifyAccess, addr: GuestAddr, size: usize) -> Option<bool> {
         self.qemu.access_ok(kind, addr, size)
     }
 

libafl_qemu/src/qemu/mod.rs

@@ -154,7 +154,7 @@ pub struct MemAccessInfo {
 #[derive(Debug, Clone, Copy)]
 #[repr(transparent)]
 pub struct CPU {
-    ptr: CPUStatePtr,
+    cpu_ptr: CPUStatePtr,
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -305,20 +305,20 @@ impl CPU {
     #[must_use]
     #[expect(clippy::cast_sign_loss)]
     pub fn index(&self) -> usize {
-        unsafe { libafl_qemu_cpu_index(self.ptr) as usize }
+        unsafe { libafl_qemu_cpu_index(self.cpu_ptr) as usize }
     }
 
     pub fn trigger_breakpoint(&self) {
         unsafe {
-            libafl_qemu_trigger_breakpoint(self.ptr);
+            libafl_qemu_trigger_breakpoint(self.cpu_ptr);
         }
     }
 
     // TODO expose tlb_set_dirty and tlb_reset_dirty
 
     #[must_use]
     pub fn num_regs(&self) -> i32 {
-        unsafe { libafl_qemu_num_regs(self.ptr) }
+        unsafe { libafl_qemu_num_regs(self.cpu_ptr) }
     }
 
     pub fn read_reg<R>(&self, reg: R) -> Result<GuestReg, QemuRWError>
@@ -328,12 +328,12 @@ impl CPU {
         unsafe {
             let reg_id = reg.clone().into();
             let mut val = MaybeUninit::uninit();
-            let success = libafl_qemu_read_reg(self.ptr, reg_id, val.as_mut_ptr() as *mut u8);
+            let success = libafl_qemu_read_reg(self.cpu_ptr, reg_id, val.as_mut_ptr() as *mut u8);
             if success == 0 {
                 Err(QemuRWError::wrong_reg(
                     QemuRWErrorKind::Write,
                     reg,
-                    Some(self.ptr),
+                    Some(self.cpu_ptr),
                 ))
             } else {
                 #[cfg(feature = "be")]
@@ -357,12 +357,13 @@ impl CPU {
         #[cfg(not(feature = "be"))]
         let val = GuestReg::to_le(val.into());
 
-        let success = unsafe { libafl_qemu_write_reg(self.ptr, reg_id, &raw const val as *mut u8) };
+        let success =
+            unsafe { libafl_qemu_write_reg(self.cpu_ptr, reg_id, &raw const val as *mut u8) };
         if success == 0 {
             Err(QemuRWError::wrong_reg(
                 QemuRWErrorKind::Write,
                 reg,
-                Some(self.ptr),
+                Some(self.cpu_ptr),
             ))
         } else {
             Ok(())
@@ -374,7 +375,7 @@ impl CPU {
         // TODO use gdbstub's target_cpu_memory_rw_debug
         let ret = unsafe {
             libafl_qemu_sys::cpu_memory_rw_debug(
-                self.ptr,
+                self.cpu_ptr,
                 addr as GuestVirtAddr,
                 buf.as_mut_ptr() as *mut _,
                 buf.len(),
@@ -385,7 +386,7 @@ impl CPU {
         if ret != 0 {
             Err(QemuRWError::wrong_mem_location(
                 QemuRWErrorKind::Read,
-                self.ptr,
+                self.cpu_ptr,
                 addr,
                 buf.len(),
             ))
@@ -411,7 +412,7 @@ impl CPU {
         // TODO use gdbstub's target_cpu_memory_rw_debug
         let ret = unsafe {
             libafl_qemu_sys::cpu_memory_rw_debug(
-                self.ptr,
+                self.cpu_ptr,
                 addr as GuestVirtAddr,
                 buf.as_ptr() as *mut _,
                 buf.len(),
@@ -422,7 +423,7 @@ impl CPU {
         if ret != 0 {
             Err(QemuRWError::wrong_mem_location(
                 QemuRWErrorKind::Write,
-                self.ptr,
+                self.cpu_ptr,
                 addr,
                 buf.len(),
             ))
@@ -432,15 +433,15 @@ impl CPU {
     }
 
     pub fn reset(&self) {
-        unsafe { libafl_qemu_sys::cpu_reset(self.ptr) };
+        unsafe { libafl_qemu_sys::cpu_reset(self.cpu_ptr) };
     }
 
     #[must_use]
     pub fn save_state(&self) -> CPUArchState {
         unsafe {
             let mut saved = MaybeUninit::<CPUArchState>::uninit();
             copy_nonoverlapping(
-                libafl_qemu_sys::cpu_env(self.ptr.as_mut().unwrap()),
+                libafl_qemu_sys::cpu_env(self.cpu_ptr.as_mut().unwrap()),
                 saved.as_mut_ptr(),
                 1,
             );
@@ -452,15 +453,15 @@ impl CPU {
         unsafe {
             copy_nonoverlapping(
                 saved,
-                libafl_qemu_sys::cpu_env(self.ptr.as_mut().unwrap()),
+                libafl_qemu_sys::cpu_env(self.cpu_ptr.as_mut().unwrap()),
                 1,
             );
         }
     }
 
     #[must_use]
     pub fn raw_ptr(&self) -> CPUStatePtr {
-        self.ptr
+        self.cpu_ptr
     }
 
     #[must_use]
@@ -737,43 +738,62 @@ impl Qemu {
         if ptr.is_null() {
             None
         } else {
-            Some(CPU { ptr })
+            Some(CPU { cpu_ptr: ptr })
         }
     }
 
     #[must_use]
     #[expect(clippy::cast_possible_wrap)]
-    pub fn cpu_from_index(&self, index: usize) -> CPU {
-        unsafe {
-            CPU {
-                ptr: libafl_qemu_get_cpu(index as i32),
-            }
+    pub fn cpu_from_index(&self, index: usize) -> Option<CPU> {
+        let cpu_ptr = unsafe { libafl_qemu_get_cpu(index as i32) };
+
+        if cpu_ptr.is_null() {
+            None
+        } else {
+            Some(CPU { cpu_ptr })
         }
     }
 
+    /// # Safety
+    ///
+    /// CPU will be incorrect if CPU index does not exist
+    #[must_use]
+    #[expect(clippy::cast_possible_wrap)]
+    pub unsafe fn cpu_from_index_unchecked(&self, index: usize) -> CPU {
+        let cpu_ptr = unsafe { libafl_qemu_get_cpu(index as i32) };
+
+        CPU { cpu_ptr }
+    }
+
     #[must_use]
     pub fn page_from_addr(&self, addr: GuestAddr) -> GuestAddr {
         unsafe { libafl_page_from_addr(addr) }
     }
 
     /// Read a value from a guest address, taking into account the potential indirections with the current CPU.
+    /// Uses the 0th CPU if no CPU is currently running.
     pub fn read_mem(&self, addr: GuestAddr, buf: &mut [u8]) -> Result<(), QemuRWError> {
         self.current_cpu()
-            .unwrap_or_else(|| self.cpu_from_index(0))
+            .or_else(|| self.cpu_from_index(0))
+            .ok_or(QemuRWError::current_cpu_not_found(QemuRWErrorKind::Read))?
             .read_mem(addr, buf)
     }
 
     /// Read a value from a guest address, taking into account the potential indirections with the current CPU.
+    /// Uses the 0th CPU if no CPU is currently running.
     pub fn read_mem_vec(&self, addr: GuestAddr, len: usize) -> Result<Vec<u8>, QemuRWError> {
         self.current_cpu()
-            .unwrap_or_else(|| self.cpu_from_index(0))
+            .or_else(|| self.cpu_from_index(0))
+            .ok_or(QemuRWError::current_cpu_not_found(QemuRWErrorKind::Read))?
             .read_mem_vec(addr, len)
     }
 
     /// Write a value to a guest address, taking into account the potential indirections with the current CPU.
+    /// Uses the 0th CPU if no CPU is currently running.
     pub fn write_mem(&self, addr: GuestAddr, buf: &[u8]) -> Result<(), QemuRWError> {
         self.current_cpu()
-            .unwrap_or_else(|| self.cpu_from_index(0))
+            .or_else(|| self.cpu_from_index(0))
+            .ok_or(QemuRWError::current_cpu_not_found(QemuRWErrorKind::Write))?
             .write_mem(addr, buf)
     }
 
@@ -816,12 +836,12 @@ impl Qemu {
     /// This may only be safely used for valid guest addresses.
     ///
     /// In any case, no check will be performed on the correctness of the operation.
-    ///
+    /// Also, the there must be a current CPU (or a CPU at index 0).
     /// Please refer to [`CPU::read_mem`] for more details.
     pub unsafe fn read_mem_unchecked(&self, addr: GuestAddr, buf: &mut [u8]) {
         unsafe {
             self.current_cpu()
-                .unwrap_or_else(|| self.cpu_from_index(0))
+                .unwrap_or_else(|| self.cpu_from_index_unchecked(0))
                 .read_mem_unchecked(addr, buf);
         }
     }
@@ -834,11 +854,12 @@ impl Qemu {
     /// In any case, no check will be performed on the correctness of the operation.
     ///
     /// This may only be safely used for valid guest addresses.
+    /// Also, the there must be a current CPU (or a CPU at index 0).
     /// Please refer to [`CPU::write_mem`] for more details.
     pub unsafe fn write_mem_unchecked(&self, addr: GuestAddr, buf: &[u8]) {
         unsafe {
             self.current_cpu()
-                .unwrap_or_else(|| self.cpu_from_index(0))
+                .unwrap_or_else(|| self.cpu_from_index_unchecked(0))
                 .write_mem_unchecked(addr, buf);
         }
     }

libafl_qemu/src/qemu/systemmode.rs

@@ -92,7 +92,7 @@ impl CPU {
             let page = libafl_page_from_addr(vaddr as GuestUsize) as GuestVirtAddr;
             let mut attrs = MaybeUninit::<libafl_qemu_sys::MemTxAttrs>::uninit();
             let paddr = libafl_qemu_sys::cpu_get_phys_page_attrs_debug(
-                self.ptr,
+                self.cpu_ptr,
                 page as GuestVirtAddr,
                 attrs.as_mut_ptr(),
             );
@@ -134,7 +134,7 @@ impl CPU {
 
     #[must_use]
     pub fn current_paging_id(&self) -> Option<GuestPhysAddr> {
-        let paging_id = unsafe { libafl_qemu_current_paging_id(self.ptr) };
+        let paging_id = unsafe { libafl_qemu_current_paging_id(self.cpu_ptr) };
 
         if paging_id == 0 {
             None
@@ -149,10 +149,10 @@ impl CPU {
     /// no check is done on the correctness of the operation.
     /// if a problem occurred during the operation, there will be no feedback
     pub unsafe fn read_mem_unchecked(&self, addr: GuestAddr, buf: &mut [u8]) {
-        // TODO use gdbstub's target_cpu_memory_rw_debug
         unsafe {
+            // TODO use gdbstub's target_cpu_memory_rw_debug
             libafl_qemu_sys::cpu_memory_rw_debug(
-                self.ptr,
+                self.cpu_ptr,
                 addr as GuestVirtAddr,
                 buf.as_mut_ptr() as *mut _,
                 buf.len(),
@@ -167,10 +167,10 @@ impl CPU {
     /// no check is done on the correctness of the operation.
     /// if a problem occurred during the operation, there will be no feedback
     pub unsafe fn write_mem_unchecked(&self, addr: GuestAddr, buf: &[u8]) {
-        // TODO use gdbstub's target_cpu_memory_rw_debug
         unsafe {
+            // TODO use gdbstub's target_cpu_memory_rw_debug
             libafl_qemu_sys::cpu_memory_rw_debug(
-                self.ptr,
+                self.cpu_ptr,
                 addr as GuestVirtAddr,
                 buf.as_ptr() as *mut _,
                 buf.len(),
@@ -376,7 +376,7 @@ impl<'a> Iterator for HostMemoryIter<'a> {
         } else {
             // Host memory allocation is always host-page aligned, so we can freely go from host page to host page.
             let start_host_addr: *const u8 =
-                unsafe { libafl_qemu_sys::libafl_paddr2host(self.cpu.ptr, self.addr, false) };
+                unsafe { libafl_qemu_sys::libafl_paddr2host(self.cpu.cpu_ptr, self.addr, false) };
             let host_page_size = Qemu::get().unwrap().host_page_size();
             let mut size_taken: usize = std::cmp::min(
                 (start_host_addr as usize).next_multiple_of(host_page_size),
@@ -388,8 +388,9 @@ impl<'a> Iterator for HostMemoryIter<'a> {
 
             // Now self.addr is host-page aligned
             while self.remaining_len > 0 {
-                let next_page_host_addr: *const u8 =
-                    unsafe { libafl_qemu_sys::libafl_paddr2host(self.cpu.ptr, self.addr, false) };
+                let next_page_host_addr: *const u8 = unsafe {
+                    libafl_qemu_sys::libafl_paddr2host(self.cpu.cpu_ptr, self.addr, false)
+                };
 
                 // Non-contiguous, we stop here for the slice
                 if next_page_host_addr != start_host_addr {

libafl_qemu/src/qemu/usermode.rs

@@ -264,11 +264,17 @@ impl Qemu {
         unsafe { (addr as usize - guest_base) as GuestAddr }
     }
 
+    /// Tells whether access to target address @addr of size @size is valid or not.
+    /// The access is checked relatively to `current_cpu` if available, or the CPU at index 0
+    /// otherwise.
+    /// The function returns None if no CPU could be found.
     #[must_use]
-    pub fn access_ok(&self, kind: VerifyAccess, addr: GuestAddr, size: usize) -> bool {
-        self.current_cpu()
-            .unwrap_or_else(|| self.cpu_from_index(0))
-            .access_ok(kind, addr, size)
+    pub fn access_ok(&self, kind: VerifyAccess, addr: GuestAddr, size: usize) -> Option<bool> {
+        Some(
+            self.current_cpu()
+                .or_else(|| self.cpu_from_index(0))?
+                .access_ok(kind, addr, size),
+        )
     }
 
     pub fn force_dfl(&self) {