From 89342b22c2011f455566d1b5516177b9857dd3f2 Mon Sep 17 00:00:00 2001 From: "Dongjia \"toka\" Zhang" Date: Fri, 28 Feb 2025 20:09:01 +0100 Subject: [PATCH] Revert #2935 (#3031) * revert * null check * no std --- libafl/src/executors/hooks/inprocess.rs | 61 ++++++++++++++------ libafl/src/executors/hooks/inprocess_fork.rs | 31 ++++++++-- libafl_qemu/src/executor.rs | 42 +++++--------- 3 files changed, 84 insertions(+), 50 deletions(-) diff --git a/libafl/src/executors/hooks/inprocess.rs b/libafl/src/executors/hooks/inprocess.rs index 404b8d4812..b7d8b28d92 100644 --- a/libafl/src/executors/hooks/inprocess.rs +++ b/libafl/src/executors/hooks/inprocess.rs @@ -6,7 +6,7 @@ use core::sync::atomic::{Ordering, compiler_fence}; use core::{ ffi::c_void, marker::PhantomData, - ptr::{self, null_mut}, + ptr::{null, null_mut}, time::Duration, }; @@ -43,6 +43,12 @@ use crate::{inputs::Input, observers::ObserversTuple, state::HasCurrentTestcase} /// The inmem executor's handlers. #[expect(missing_debug_implementations)] pub struct InProcessHooks { + /// On crash C function pointer + #[cfg(feature = "std")] + pub crash_handler: *const c_void, + /// On timeout C function pointer + #[cfg(feature = "std")] + pub timeout_handler: *const c_void, /// `TImer` struct #[cfg(feature = "std")] pub timer: TimerStruct, @@ -190,6 +196,19 @@ impl ExecutorHook for InProcessHooks { fn init(&mut self, _state: &mut S) {} /// Call before running a target. fn pre_exec(&mut self, _state: &mut S, _input: &I) { + #[cfg(feature = "std")] + // This is very important!!!!! + // Don't remove these pointer settings. + // Imagine there are two executors, you have to set the correct crash handlers for each of the executor. + unsafe { + let data = &raw mut GLOBAL_STATE; + assert!((*data).crash_handler == null()); + // usually timeout handler and crash handler is set together + // so no check for timeout handler is null or not + (*data).crash_handler = self.crash_handler; + (*data).timeout_handler = self.timeout_handler; + } + #[cfg(all(feature = "std", not(all(miri, target_vendor = "apple"))))] self.timer_mut().set_timer(); } @@ -200,6 +219,12 @@ impl ExecutorHook for InProcessHooks { // We're calling this only once per execution, in a single thread. #[cfg(all(feature = "std", not(all(miri, target_vendor = "apple"))))] self.timer_mut().unset_timer(); + #[cfg(feature = "std")] + unsafe { + let data = &raw mut GLOBAL_STATE; + (*data).crash_handler = null(); + (*data).timeout_handler = null(); + } } } @@ -234,18 +259,15 @@ impl InProcessHooks { setup_signal_handler(data)?; } - #[cfg(feature = "std")] - unsafe { - let data = &raw mut GLOBAL_STATE; - (*data).crash_handler = - unix_signal_handler::inproc_crash_handler:: as *const c_void; - (*data).timeout_handler = - unix_signal_handler::inproc_timeout_handler:: as *const _; - } - compiler_fence(Ordering::SeqCst); Ok(Self { #[cfg(feature = "std")] + crash_handler: unix_signal_handler::inproc_crash_handler:: + as *const c_void, + #[cfg(feature = "std")] + timeout_handler: unix_signal_handler::inproc_timeout_handler:: + as *const _, + #[cfg(feature = "std")] timer: TimerStruct::new(exec_tmout), phantom: PhantomData, }) @@ -278,7 +300,7 @@ impl InProcessHooks { >(); setup_exception_handler(data)?; compiler_fence(Ordering::SeqCst); - (*data).crash_handler = + let crash_handler = crate::executors::hooks::windows::windows_exception_handler::inproc_crash_handler::< E, EM, @@ -296,9 +318,10 @@ impl InProcessHooks { S, Z, > as *const c_void; - (*data).timeout_handler = timeout_handler; let timer = TimerStruct::new(exec_tmout, timeout_handler); ret = Ok(Self { + crash_handler, + timeout_handler, timer, phantom: PhantomData, }); @@ -336,6 +359,10 @@ impl InProcessHooks { #[cfg(not(windows))] pub fn nop() -> Self { Self { + #[cfg(feature = "std")] + crash_handler: null(), + #[cfg(feature = "std")] + timeout_handler: null(), #[cfg(feature = "std")] timer: TimerStruct::new(Duration::from_millis(5000)), phantom: PhantomData, @@ -414,7 +441,7 @@ impl InProcessExecutorHandlerData { #[cfg(all(feature = "std", any(unix, windows)))] pub(crate) unsafe fn take_current_input<'a, I>(&mut self) -> &'a I { let r = unsafe { (self.current_input_ptr as *const I).as_ref().unwrap() }; - self.current_input_ptr = ptr::null(); + self.current_input_ptr = null(); r } @@ -509,19 +536,19 @@ pub static mut GLOBAL_STATE: InProcessExecutorHandlerData = InProcessExecutorHan // The fuzzer ptr for signal handling fuzzer_ptr: null_mut(), // The executor ptr for signal handling - executor_ptr: ptr::null(), + executor_ptr: null(), // The current input for signal handling - current_input_ptr: ptr::null(), + current_input_ptr: null(), #[cfg(feature = "std")] signal_handler_depth: 0, // The crash handler fn #[cfg(feature = "std")] - crash_handler: ptr::null(), + crash_handler: null(), // The timeout handler fn #[cfg(feature = "std")] - timeout_handler: ptr::null(), + timeout_handler: null(), #[cfg(all(windows, feature = "std"))] ptp_timer: None, #[cfg(all(windows, feature = "std"))] diff --git a/libafl/src/executors/hooks/inprocess_fork.rs b/libafl/src/executors/hooks/inprocess_fork.rs index 66245cced3..ec4db4798d 100644 --- a/libafl/src/executors/hooks/inprocess_fork.rs +++ b/libafl/src/executors/hooks/inprocess_fork.rs @@ -26,6 +26,10 @@ use crate::{ /// The inmem fork executor's hooks. #[derive(Debug)] pub struct InChildProcessHooks { + /// On crash C function pointer + pub crash_handler: *const c_void, + /// On timeout C function pointer + pub timeout_handler: *const c_void, phantom: PhantomData<(I, S)>, } @@ -34,9 +38,22 @@ impl ExecutorHook for InChildProcessHooks { fn init(&mut self, _state: &mut S) {} /// Call before running a target. - fn pre_exec(&mut self, _state: &mut S, _input: &I) {} + fn pre_exec(&mut self, _state: &mut S, _input: &I) { + unsafe { + let data = &raw mut FORK_EXECUTOR_GLOBAL_DATA; + (*data).crash_handler = self.crash_handler; + (*data).timeout_handler = self.timeout_handler; + compiler_fence(Ordering::SeqCst); + } + } - fn post_exec(&mut self, _state: &mut S, _input: &I) {} + fn post_exec(&mut self, _state: &mut S, _input: &I) { + unsafe { + let data = &raw mut FORK_EXECUTOR_GLOBAL_DATA; + (*data).crash_handler = null(); + (*data).timeout_handler = null(); + } + } } impl InChildProcessHooks { @@ -53,11 +70,11 @@ impl InChildProcessHooks { #[cfg(not(miri))] setup_signal_handler(data)?; compiler_fence(Ordering::SeqCst); - (*data).crash_handler = - child_signal_handlers::child_crash_handler:: as *const c_void; - (*data).timeout_handler = - child_signal_handlers::child_timeout_handler:: as *const c_void; Ok(Self { + crash_handler: child_signal_handlers::child_crash_handler:: + as *const c_void, + timeout_handler: child_signal_handlers::child_timeout_handler:: + as *const c_void, phantom: PhantomData, }) } @@ -67,6 +84,8 @@ impl InChildProcessHooks { #[must_use] pub fn nop() -> Self { Self { + crash_handler: null(), + timeout_handler: null(), phantom: PhantomData, } } diff --git a/libafl_qemu/src/executor.rs b/libafl_qemu/src/executor.rs index 57b7e82c3b..9776c5a331 100644 --- a/libafl_qemu/src/executor.rs +++ b/libafl_qemu/src/executor.rs @@ -16,7 +16,7 @@ use libafl::{ events::{EventFirer, EventRestarter}, executors::{ Executor, ExitKind, HasObservers, - hooks::inprocess::{GLOBAL_STATE, InProcessExecutorHandlerData}, + hooks::inprocess::InProcessExecutorHandlerData, inprocess::{HasInProcessHooks, stateful::StatefulInProcessExecutor}, inprocess_fork::stateful::StatefulInProcessForkExecutor, }, @@ -251,39 +251,27 @@ where OF: Feedback, Z: HasObjective + HasScheduler + ExecutionProcessor, { - let inner = StatefulInProcessExecutor::with_timeout( + let mut inner = StatefulInProcessExecutor::with_timeout( harness_fn, emulator, observers, fuzzer, state, event_mgr, timeout, )?; - let data = &raw mut GLOBAL_STATE; + // rewrite the crash handler pointer #[cfg(feature = "usermode")] - unsafe { - // rewrite the crash handler pointer - (*data).crash_handler = + { + inner.inprocess_hooks_mut().crash_handler = inproc_qemu_crash_handler:: as *const c_void; } - unsafe { - // rewrite the timeout handler pointer - (*data).timeout_handler = inproc_qemu_timeout_handler::< - StatefulInProcessExecutor< - 'a, - EM, - Emulator, - H, - I, - OT, - S, - Z, - >, - EM, - ET, - I, - OF, - S, - Z, - > as *const c_void; - } + // rewrite the timeout handler pointer + inner.inprocess_hooks_mut().timeout_handler = inproc_qemu_timeout_handler::< + StatefulInProcessExecutor<'a, EM, Emulator, H, I, OT, S, Z>, + EM, + ET, + I, + OF, + S, + Z, + > as *const c_void; Ok(Self { inner,