Remove unsafe inprocess access functions, document remaining function… (#3306)

* Remove unsafe inprocess access functions, document remaining functions better

* more docs

Author: domenukk

fuzzers/forkserver/forkserver_simple/Cargo.lock

@@ -527,8 +527,8 @@ impl InProcessExecutorHandlerData {
     }
 }
 
-/// Exception handling needs some nasty unsafe.
-pub static mut GLOBAL_STATE: InProcessExecutorHandlerData = InProcessExecutorHandlerData {
+/// Exception handling needs some nasty globals.
+pub(crate) static mut GLOBAL_STATE: InProcessExecutorHandlerData = InProcessExecutorHandlerData {
     // The state ptr for signal handling
     state_ptr: null_mut(),
     // The event manager ptr for signal handling
@@ -560,44 +560,12 @@ pub static mut GLOBAL_STATE: InProcessExecutorHandlerData = InProcessExecutorHan
 /// Get the inprocess State
 ///
 /// # Safety
+/// This is a *very* dangerous and mainly for internal use (unless you _really_ know what you're doing.
+/// The type is not checked and needs to be specified correctly.
 /// Only safe if not called twice and if the state is not accessed from another borrow while this one is alive.
 #[must_use]
 pub unsafe fn inprocess_get_state<'a, S>() -> Option<&'a mut S> {
+    // # Safety
+    // As unsafe as it gets, but the function is documented accordingly.
     unsafe { (GLOBAL_STATE.state_ptr as *mut S).as_mut() }
 }
-
-/// Get the `EventManager`
-///
-/// # Safety
-/// Only safe if not called twice and if the event manager is not accessed from another borrow while this one is alive.
-#[must_use]
-pub unsafe fn inprocess_get_event_manager<'a, EM>() -> Option<&'a mut EM> {
-    unsafe { (GLOBAL_STATE.event_mgr_ptr as *mut EM).as_mut() }
-}
-
-/// Gets the inprocess [`crate::fuzzer::Fuzzer`]
-///
-/// # Safety
-/// Only safe if not called twice and if the fuzzer is not accessed from another borrow while this one is alive.
-#[must_use]
-pub unsafe fn inprocess_get_fuzzer<'a, F>() -> Option<&'a mut F> {
-    unsafe { (GLOBAL_STATE.fuzzer_ptr as *mut F).as_mut() }
-}
-
-/// Gets the inprocess [`Executor`]
-///
-/// # Safety
-/// Only safe if not called twice and if the executor is not accessed from another borrow while this one is alive.
-#[must_use]
-pub unsafe fn inprocess_get_executor<'a, E>() -> Option<&'a mut E> {
-    unsafe { (GLOBAL_STATE.executor_ptr as *mut E).as_mut() }
-}
-
-/// Gets the inprocess input
-///
-/// # Safety
-/// Only safe if not called concurrently and if the input is not used mutably while this reference is alive.
-#[must_use]
-pub unsafe fn inprocess_get_input<'a, I>() -> Option<&'a I> {
-    unsafe { (GLOBAL_STATE.current_input_ptr as *const I).as_ref() }
-}

libafl/src/executors/hooks/inprocess.rs

@@ -51,6 +51,8 @@ pub mod unix_signal_handler {
             info: &mut siginfo_t,
             context: Option<&mut ucontext_t>,
         ) {
+            // # Safety
+            // This runs in a signal handler, no other threads access these variables/borrows anymore.
             unsafe {
                 let data = &raw mut GLOBAL_STATE;
                 let (max_depth_reached, signal_depth) = (*data).signal_handler_enter();
@@ -97,6 +99,9 @@ pub mod unix_signal_handler {
         I: Input + Clone,
     {
         let old_hook = panic::take_hook();
+        // # Safety
+        // The panic handler should only run when all other execution stopped.
+        // At this point, accessing the global state should be sound.
         panic::set_hook(Box::new(move |panic_info| unsafe {
             old_hook(panic_info);
             let data = &raw mut GLOBAL_STATE;

libafl/src/executors/hooks/unix.rs

@@ -86,6 +86,8 @@ where
         input: &I,
         executor_ptr: *const c_void,
     ) {
+        // # Safety
+        // This writes pointers to global state. Only unsafe if the state is they are accessed incorrectly.
         unsafe {
             let data = &raw mut GLOBAL_STATE;
             write_volatile(
@@ -114,6 +116,8 @@ where
     /// This function marks the boundary between the fuzzer and the target
     #[inline]
     pub fn leave_target(&mut self, _fuzzer: &mut Z, _state: &mut S, _mgr: &mut EM, _input: &I) {
+        // # Safety
+        // We set the global pointer to null, no direct safety concerns arise.
         unsafe {
             let data = &raw mut GLOBAL_STATE;
 

libafl/src/executors/inprocess/inner.rs

@@ -45,7 +45,11 @@ pub const CMPLOG_KIND_RTN: u8 = 1;
 
 // EXTERNS, GLOBALS
 
-#[cfg(any(feature = "cmplog", feature = "sancov_cmplog", feature = "sancov_value_profile"))]
+#[cfg(any(
+    feature = "cmplog",
+    feature = "sancov_cmplog",
+    feature = "sancov_value_profile"
+))]
 // void __libafl_targets_cmplog_instructions(uintptr_t k, uint8_t size, uint64_t arg1, uint64_t arg2)
 unsafe extern "C" {
     /// Logs an instruction for feedback during fuzzing
@@ -58,7 +62,12 @@ unsafe extern "C" {
     pub fn __libafl_targets_cmplog_routines(k: usize, ptr1: *const u8, ptr2: *const u8);
 
     /// Cmplog routines but with len specified.
-    pub fn __libafl_targets_cmplog_routines_len(k: usize, ptr1: *const u8, ptr2: *const u8, len: usize);
+    pub fn __libafl_targets_cmplog_routines_len(
+        k: usize,
+        ptr1: *const u8,
+        ptr2: *const u8,
+        len: usize,
+    );
 
     /// Pointer to the `CmpLog` map
     pub static mut libafl_cmplog_map_ptr: *mut CmpLogMap;
@@ -72,7 +81,12 @@ unsafe extern "C" {
     /// Logs an AFL++ style routine for feedback during fuzzing
     pub fn __libafl_targets_cmplog_routines_extended(k: usize, ptr1: *const u8, ptr2: *const u8);
     /// Extended cmplog routines but with len specified.
-    pub fn __libafl_targets_cmplog_routines_extended_len(k: usize, ptr1: *const u8, ptr2: *const u8, len: usize);
+    pub fn __libafl_targets_cmplog_routines_extended_len(
+        k: usize,
+        ptr1: *const u8,
+        ptr2: *const u8,
+        len: usize,
+    );
 }
 
 #[cfg(feature = "cmplog_extended_instrumentation")]

libafl_targets/src/cmps/mod.rs

@@ -86,7 +86,12 @@ pub unsafe extern "C" fn __sanitizer_weak_hook_strncmp(
                 }
                 actual_len += 1;
             }
-            crate::__libafl_targets_cmplog_routines_len(k, s1 as *const u8, s2 as *const u8, actual_len);
+            crate::__libafl_targets_cmplog_routines_len(
+                k,
+                s1 as *const u8,
+                s2 as *const u8,
+                actual_len,
+            );
         }
     }
 }
@@ -132,7 +137,12 @@ pub unsafe extern "C" fn __sanitizer_weak_hook_strcmp(
                 }
                 actual_len += 1;
             }
-            crate::__libafl_targets_cmplog_routines_len(k, s1 as *const u8, s2 as *const u8, actual_len);
+            crate::__libafl_targets_cmplog_routines_len(
+                k,
+                s1 as *const u8,
+                s2 as *const u8,
+                actual_len,
+            );
         }
     }
 }