Fix Frida CI for Windows, Clippy (#1430)

* Fix Frida for Windows

* more fix

* clippy in pthreads

fuzzers/frida_gdiplus/src/fuzzer.rs

@@ -17,8 +17,8 @@ use libafl::{
     corpus::{CachedOnDiskCorpus, Corpus, OnDiskCorpus},
     events::{launcher::Launcher, llmp::LlmpRestartingEventManager, EventConfig},
     executors::{inprocess::InProcessExecutor, ExitKind, ShadowExecutor},
-    feedback_and_fast, feedback_or, feedback_or_fast,
+    feedback_or, feedback_or_fast,
-    feedbacks::{ConstFeedback, CrashFeedback, MaxMapFeedback, TimeFeedback, TimeoutFeedback},
+    feedbacks::{CrashFeedback, MaxMapFeedback, TimeFeedback, TimeoutFeedback},
     fuzzer::{Fuzzer, StdFuzzer},
     inputs::{BytesInput, HasTargetBytes},
     monitors::MultiMonitor,
@@ -32,6 +32,8 @@ use libafl::{
     state::{HasCorpus, HasMetadata, StdState},
     Error,
 };
+#[cfg(unix)]
+use libafl::{feedback_and_fast, feedbacks::ConstFeedback};
 use libafl_bolts::{
     cli::{parse_args, FuzzerOptions},
     current_nanos,

libafl_frida/src/executor.rs

@@ -1,5 +1,5 @@
 use core::fmt::{self, Debug, Formatter};
-use std::{ffi::c_void, marker::PhantomData, process};
+use std::{ffi::c_void, marker::PhantomData};
 
 use frida_gum::{
     stalker::{NoneEventSink, Stalker},
@@ -35,7 +35,7 @@ where
 {
     base: InProcessExecutor<'a, H, OT, S>,
     // thread_id for the Stalker
-    thread_id: u32,
+    thread_id: Option<u32>,
     /// Frida's dynamic rewriting engine
     stalker: Stalker<'a>,
     /// User provided callback for instrumentation
@@ -87,11 +87,15 @@ where
             } else {
                 self.followed = true;
                 let transformer = self.helper.transformer();
-                self.stalker.follow::<NoneEventSink>(
+                if let Some(thread_id) = self.thread_id {
-                    self.thread_id.try_into().unwrap(),
+                    self.stalker.follow::<NoneEventSink>(
-                    transformer,
+                        thread_id.try_into().unwrap(),
-                    None,
+                        transformer,
-                );
+                        None,
+                    );
+                } else {
+                    self.stalker.follow_me::<NoneEventSink>(transformer, None);
+                }
             }
         }
         let res = self.base.run_target(fuzzer, state, mgr, input);
@@ -162,7 +166,7 @@ where
         base: InProcessExecutor<'a, H, OT, S>,
         helper: &'c mut FridaInstrumentationHelper<'b, RT>,
     ) -> Self {
-        Self::on_thread(gum, base, helper, process::id())
+        Self::_on_thread(gum, base, helper, None)
     }
 
     /// Creates a new [`FridaInProcessExecutor`] tracking the given `thread_id`.
@@ -171,6 +175,16 @@ where
         base: InProcessExecutor<'a, H, OT, S>,
         helper: &'c mut FridaInstrumentationHelper<'b, RT>,
         thread_id: u32,
+    ) -> Self {
+        Self::_on_thread(gum, base, helper, Some(thread_id))
+    }
+
+    /// Creates a new [`FridaInProcessExecutor`] tracking the given `thread_id`, of `thread_id` is provided.
+    fn _on_thread(
+        gum: &'a Gum,
+        base: InProcessExecutor<'a, H, OT, S>,
+        helper: &'c mut FridaInstrumentationHelper<'b, RT>,
+        thread_id: Option<u32>,
     ) -> Self {
         let mut stalker = Stalker::new(gum);
         // Include the current module (the fuzzer) in stalked ranges. We clone the ranges so that

libafl_frida/src/pthread_hook.rs

@@ -1,5 +1,4 @@
 use std::{
-    cell::UnsafeCell,
     convert::{TryFrom, TryInto},
     sync::RwLock,
 };
@@ -21,49 +20,43 @@ type pthread_introspection_hook_t = extern "C" fn(
 );
 
 extern "C" {
-    fn pthread_introspection_hook_install(
+    fn pthread_introspection_hook_install(hook: *const libc::c_void) -> *const libc::c_void;
-        hook: *const libc::c_void,
-    ) -> pthread_introspection_hook_t;
 }
 
-struct PreviousHook(UnsafeCell<Option<pthread_introspection_hook_t>>);
+struct PreviousHook(*const pthread_introspection_hook_t);
 
 impl PreviousHook {
     /// Dispatch to the previous hook, if it is set.
-    pub fn dispatch(
+    pub unsafe fn dispatch(
         &self,
         event: libc::c_uint,
         thread: libc::pthread_t,
         addr: *const libc::c_void,
         size: libc::size_t,
     ) {
-        let inner = unsafe { *self.0.get() };
+        let inner = self.0;
-        if inner.is_none() {
+        if inner.is_null() {
             return;
         }
-        let inner = inner.unwrap();
+        unsafe { (*inner)(event, thread, addr, size) };
-        inner(event, thread, addr, size);
     }
 
     /// Set the previous hook.
-    pub fn set(&self, hook: pthread_introspection_hook_t) {
+    pub fn set(&mut self, hook: *const pthread_introspection_hook_t) {
-        unsafe {
+        self.0 = hook;
-            *self.0.get() = Some(hook);
-        }
     }
 
     /// Ensure the previous hook is installed again.
-    pub fn reset(&self) {
+    pub fn reset(&mut self) {
-        let inner = unsafe { *self.0.get() };
+        let inner = self.0;
-        if inner.is_none() {
+        if inner.is_null() {
             unsafe {
                 pthread_introspection_hook_install(std::ptr::null());
             }
             return;
         }
-        let inner = inner.unwrap();
         unsafe {
-            *self.0.get() = None;
+            self.0 = std::ptr::null();
             pthread_introspection_hook_install(inner as *const libc::c_void);
         }
     }
@@ -74,7 +67,7 @@ impl PreviousHook {
 unsafe impl Sync for PreviousHook {}
 
 #[allow(non_upper_case_globals)]
-static PREVIOUS_HOOK: PreviousHook = PreviousHook(UnsafeCell::new(None));
+static mut PREVIOUS_HOOK: PreviousHook = PreviousHook(std::ptr::null());
 
 static CURRENT_HOOK: RwLock<Option<PthreadIntrospectionHook>> = RwLock::new(None);
 
@@ -87,7 +80,7 @@ extern "C" fn pthread_introspection_hook(
     if let Some(ref hook) = *CURRENT_HOOK.read().unwrap() {
         hook(event.try_into().unwrap(), thread, addr, size);
     }
-    PREVIOUS_HOOK.dispatch(event, thread, addr, size);
+    unsafe { PREVIOUS_HOOK.dispatch(event, thread, addr, size) };
 }
 
 /// Closure type for `pthread_introspection` hooks.
@@ -153,7 +146,10 @@ impl From<EventType> for libc::c_uint {
 /// thread id=0x16bf67000 event=Terminate addr=0x16bd60000 size=208000
 /// thread id=0x16bf67000 event=Destroy addr=0x16bf67000 size=4000
 /// ```
-pub fn install<H>(hook: H)
+///
+/// # Safety
+/// Potential data race when if called at the same time as `install` or `reset` from another thread
+pub unsafe fn install<H>(hook: H)
 where
     H: Fn(EventType, libc::pthread_t, *const libc::c_void, libc::size_t) + Send + Sync + 'static,
 {
@@ -165,9 +161,10 @@ where
     };
 
     // Allow because we're sure this isn't from a different code generation unit.
-    #[allow(clippy::fn_address_comparisons, clippy::fn_null_check)]
+    if !(prev).is_null() && prev != pthread_introspection_hook as *const libc::c_void {
-    if !(prev as *const libc::c_void).is_null() && prev != pthread_introspection_hook {
+        unsafe {
-        PREVIOUS_HOOK.set(prev);
+            PREVIOUS_HOOK.set(prev as *const pthread_introspection_hook_t);
+        }
     }
 }
 
@@ -179,8 +176,11 @@ where
 ///# use std::thread;
 /// pthread_hook::reset();
 /// ```
-pub fn reset() {
+///
-    PREVIOUS_HOOK.reset();
+/// # Safety
+/// Potential data race when if called at the same time as `install` or `reset` from another thread
+pub unsafe fn reset() {
+    unsafe { PREVIOUS_HOOK.reset() };
 }
 
 /// The following tests fail if they are not run sequentially.
@@ -204,7 +204,7 @@ mod test {
         });
         thread::sleep(Duration::from_millis(50));
 
-        super::reset();
+        unsafe { super::reset() };
         assert!(!*triggered.lock().unwrap());
     }
 
@@ -214,19 +214,21 @@ mod test {
         let triggered: Arc<Mutex<bool>> = Arc::new(Mutex::new(false));
 
         let inner_triggered = triggered.clone();
-        super::install(move |event, _, _, _| {
+        unsafe {
-            if event == super::EventType::Create {
+            super::install(move |event, _, _, _| {
-                let mut triggered = inner_triggered.lock().unwrap();
+                if event == super::EventType::Create {
-                *triggered = true;
+                    let mut triggered = inner_triggered.lock().unwrap();
-            }
+                    *triggered = true;
-        });
+                }
+            })
+        };
 
         thread::spawn(|| {
             thread::sleep(Duration::from_millis(1));
         });
         thread::sleep(Duration::from_millis(50));
 
-        super::reset();
+        unsafe { super::reset() };
         assert!(*triggered.lock().unwrap());
     }
 
@@ -236,19 +238,21 @@ mod test {
         let triggered: Arc<Mutex<bool>> = Arc::new(Mutex::new(false));
 
         let inner_triggered = triggered.clone();
-        super::install(move |event, _, _, _| {
+        unsafe {
-            if event == super::EventType::Start {
+            super::install(move |event, _, _, _| {
-                let mut triggered = inner_triggered.lock().unwrap();
+                if event == super::EventType::Start {
-                *triggered = true;
+                    let mut triggered = inner_triggered.lock().unwrap();
-            }
+                    *triggered = true;
-        });
+                }
+            })
+        };
 
         thread::spawn(|| {
             thread::sleep(Duration::from_millis(1));
         });
         thread::sleep(Duration::from_millis(50));
 
-        super::reset();
+        unsafe { super::reset() };
         assert!(*triggered.lock().unwrap());
     }
 
@@ -258,14 +262,16 @@ mod test {
         let triggered: Arc<Mutex<bool>> = Arc::new(Mutex::new(false));
 
         let inner_triggered = triggered.clone();
-        super::install(move |event, _, _, _| {
+        unsafe {
-            if event == super::EventType::Start {
+            super::install(move |event, _, _, _| {
-                let mut triggered = inner_triggered.lock().unwrap();
+                if event == super::EventType::Start {
-                *triggered = true;
+                    let mut triggered = inner_triggered.lock().unwrap();
-            }
+                    *triggered = true;
-        });
+                }
+            })
+        };
 
-        super::reset();
+        unsafe { super::reset() };
 
         thread::spawn(|| {
             thread::sleep(Duration::from_millis(1));