Split up HasTimeout trait, use it for command configurator (AFLplusplus#3476)

* Split up HasTimeout trait, use it for command configurator

* fmt

* fmt

Author: domenukk

crates/libafl/src/executors/combined.rs

@@ -8,7 +8,7 @@ use libafl_bolts::tuples::RefIndexable;
 use super::HasTimeout;
 use crate::{
     Error,
-    executors::{Executor, ExitKind, HasObservers},
+    executors::{Executor, ExitKind, HasObservers, SetTimeout},
 };
 
 /// A [`CombinedExecutor`] wraps a primary executor, forwarding its methods, and a secondary one
@@ -56,12 +56,6 @@ where
     A: HasTimeout,
     B: HasTimeout,
 {
-    #[inline]
-    fn set_timeout(&mut self, timeout: Duration) {
-        self.primary.set_timeout(timeout);
-        self.secondary.set_timeout(timeout);
-    }
-
     #[inline]
     fn timeout(&self) -> Duration {
         assert!(
@@ -72,6 +66,18 @@ where
     }
 }
 
+impl<A, B> SetTimeout for CombinedExecutor<A, B>
+where
+    A: SetTimeout,
+    B: SetTimeout,
+{
+    #[inline]
+    fn set_timeout(&mut self, timeout: Duration) {
+        self.primary.set_timeout(timeout);
+        self.secondary.set_timeout(timeout);
+    }
+}
+
 impl<A, B> HasObservers for CombinedExecutor<A, B>
 where
     A: HasObservers,

crates/libafl/src/executors/command.rs

@@ -57,12 +57,12 @@ use typed_builder::TypedBuilder;
 
 #[cfg(all(target_family = "unix", feature = "fork"))]
 use super::forkserver::ConfigTarget;
-use super::{HasTimeout, StdChildArgs, StdChildArgsInner};
+use super::{StdChildArgs, StdChildArgsInner};
 #[cfg(target_os = "linux")]
 use crate::executors::hooks::ExecutorHooksTuple;
 use crate::{
     Error,
-    executors::{Executor, ExitKind, HasObservers},
+    executors::{Executor, ExitKind, HasObservers, HasTimeout, SetTimeout},
     inputs::{HasTargetBytes, ToTargetBytes},
     observers::{ObserversTuple, StdErrObserver, StdOutObserver},
     state::HasExecutions,
@@ -204,19 +204,26 @@ impl CommandConfigurator<Child> for StdCommandConfigurator {
             }
         }
     }
+}
 
-    fn exec_timeout(&self) -> Duration {
+impl HasTimeout for StdCommandConfigurator {
+    fn timeout(&self) -> Duration {
         self.timeout
     }
-    fn exec_timeout_mut(&mut self) -> &mut Duration {
-        &mut self.timeout
+}
+
+impl SetTimeout for StdCommandConfigurator {
+    fn set_timeout(&mut self, timeout: Duration) {
+        self.timeout = timeout;
     }
 }
 
 /// Linux specific [`CommandConfigurator`] that leverages `ptrace`
 ///
 /// This configurator was primarily developed to be used in conjunction with
 /// [`crate::executors::hooks::intel_pt::IntelPTHook`]
+///
+/// Use `PTraceCommandConfigurator::builder().timeout` to set an initial timeout
 #[cfg(all(feature = "intel_pt", target_os = "linux"))]
 #[derive(Debug, Clone, PartialEq, Eq, TypedBuilder)]
 pub struct PTraceCommandConfigurator {
@@ -300,15 +307,13 @@ impl CommandConfigurator<Pid> for PTraceCommandConfigurator {
             Err(e) => Err(Error::unknown(format!("Fork failed: {e}"))),
         }
     }
+}
 
-    fn exec_timeout(&self) -> Duration {
+#[cfg(all(feature = "intel_pt", target_os = "linux"))]
+impl HasTimeout for PTraceCommandConfigurator {
+    fn timeout(&self) -> Duration {
         Duration::from_secs(u64::from(self.timeout))
     }
-
-    /// Use [`PTraceCommandConfigurator::builder()`](PTraceCommandConfigurator) instead
-    fn exec_timeout_mut(&mut self) -> &mut Duration {
-        unimplemented!("Use [`PTraceCommandConfigurator::builder().timeout`] instead")
-    }
 }
 
 /// A `CommandExecutor` is a wrapper around [`Command`] to execute a target as a child process.
@@ -372,7 +377,7 @@ impl<C, HT, I, OT, S, T> CommandExecutor<C, HT, I, OT, S, T> {
 impl<HT, I, OT, S, T> CommandExecutor<Child, HT, I, OT, S, T>
 where
     S: HasExecutions,
-    T: CommandConfigurator<Child> + Debug,
+    T: CommandConfigurator<Child> + HasTimeout + Debug,
     OT: ObserversTuple<I, S>,
 {
     fn execute_input_with_command<TB: ToTargetBytes<I>>(
@@ -390,7 +395,7 @@ where
             .spawn_child(target_bytes_converter.to_target_bytes(input))?;
 
         let exit_kind = child
-            .wait_timeout(self.configurator.exec_timeout())
+            .wait_timeout(self.configurator.timeout())
             .expect("waiting on child failed")
             .map(|status| self.configurator.exit_kind_from_status(&status))
             .unwrap_or_else(|| {
@@ -431,7 +436,7 @@ where
 impl<EM, HT, I, OT, S, T, Z> Executor<EM, I, S, Z> for CommandExecutor<Child, HT, I, OT, S, T>
 where
     S: HasExecutions,
-    T: CommandConfigurator<Child> + Debug,
+    T: CommandConfigurator<Child> + HasTimeout + Debug,
     OT: MatchName + ObserversTuple<I, S>,
     Z: ToTargetBytes<I>,
 {
@@ -449,16 +454,21 @@ where
 // this only works on unix because of the reliance on checking the process signal for detecting OOM
 impl<C, HT, I, OT, S, T> HasTimeout for CommandExecutor<C, HT, I, OT, S, T>
 where
-    T: CommandConfigurator<C>,
+    T: HasTimeout,
 {
     #[inline]
     fn timeout(&self) -> Duration {
-        self.configurator.exec_timeout()
+        self.configurator.timeout()
     }
+}
 
+impl<C, HT, I, OT, S, T> SetTimeout for CommandExecutor<C, HT, I, OT, S, T>
+where
+    T: SetTimeout,
+{
     #[inline]
     fn set_timeout(&mut self, timeout: Duration) {
-        *self.configurator.exec_timeout_mut() = timeout;
+        self.configurator.set_timeout(timeout);
     }
 }
 
@@ -742,7 +752,7 @@ impl CommandExecutorBuilder {
 /// use libafl::{
 ///     Error, HasTargetBytesConverter,
 ///     corpus::Corpus,
-///     executors::{Executor, command::CommandConfigurator},
+///     executors::{Executor, HasTimeout, command::CommandConfigurator},
 ///     inputs::{BytesInput, HasTargetBytes, Input, ToTargetBytes},
 ///     state::HasExecutions,
 /// };
@@ -763,13 +773,12 @@ impl CommandExecutorBuilder {
 ///         stdin.write_all(&target_bytes)?;
 ///         Ok(child)
 ///     }
+/// }
 ///
-///     fn exec_timeout(&self) -> Duration {
+/// impl HasTimeout for MyExecutor {
+///     fn timeout(&self) -> Duration {
 ///         Duration::from_secs(5)
 ///     }
-///     fn exec_timeout_mut(&mut self) -> &mut Duration {
-///         todo!()
-///     }
 /// }
 ///
 /// fn make_executor<EM, S, Z>() -> impl Executor<EM, BytesInput, S, Z>
@@ -793,11 +802,6 @@ pub trait CommandConfigurator<C>: Sized {
     /// Spawns a new process with the given configuration.
     fn spawn_child(&mut self, target_bytes: OwnedSlice<'_, u8>) -> Result<C, Error>;
 
-    /// Provides timeout duration for execution of the child process.
-    fn exec_timeout(&self) -> Duration;
-    /// Set the timeout duration for execution of the child process.
-    fn exec_timeout_mut(&mut self) -> &mut Duration;
-
     /// Maps the exit status of the child process to an `ExitKind`.
     #[cfg(unix)]
     #[inline]

crates/libafl/src/executors/differential.rs

@@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize};
 use super::HasTimeout;
 use crate::{
     Error,
-    executors::{Executor, ExitKind, HasObservers},
+    executors::{Executor, ExitKind, HasObservers, SetTimeout},
     observers::{DifferentialObserversTuple, ObserversTuple},
 };
 
@@ -117,12 +117,6 @@ where
     A: HasTimeout,
     B: HasTimeout,
 {
-    #[inline]
-    fn set_timeout(&mut self, timeout: core::time::Duration) {
-        self.primary.set_timeout(timeout);
-        self.secondary.set_timeout(timeout);
-    }
-
     #[inline]
     fn timeout(&self) -> core::time::Duration {
         assert!(
@@ -133,6 +127,18 @@ where
     }
 }
 
+impl<A, B, DOT, I, OTA, OTB, S> SetTimeout for DiffExecutor<A, B, DOT, I, OTA, OTB, S>
+where
+    A: SetTimeout,
+    B: SetTimeout,
+{
+    #[inline]
+    fn set_timeout(&mut self, timeout: core::time::Duration) {
+        self.primary.set_timeout(timeout);
+        self.secondary.set_timeout(timeout);
+    }
+}
+
 /// Proxy the observers of the inner executors
 #[derive(Serialize, Deserialize, Debug)]
 #[serde(

crates/libafl/src/executors/forkserver.rs

@@ -46,7 +46,7 @@ use crate::observers::{
 };
 use crate::{
     Error,
-    executors::{Executor, ExitKind, HasObservers},
+    executors::{Executor, ExitKind, HasObservers, SetTimeout},
     inputs::{Input, ToTargetBytes},
     mutators::Tokens,
     observers::{MapObserver, Observer, ObserversTuple},
@@ -1543,7 +1543,9 @@ impl<I, OT, S, SHM> HasTimeout for ForkserverExecutor<I, OT, S, SHM> {
     fn timeout(&self) -> Duration {
         self.timeout.into()
     }
+}
 
+impl<I, OT, S, SHM> SetTimeout for ForkserverExecutor<I, OT, S, SHM> {
     #[inline]
     fn set_timeout(&mut self, timeout: Duration) {
         self.timeout = TimeSpec::from_duration(timeout);

crates/libafl/src/executors/mod.rs

@@ -136,11 +136,14 @@ pub trait Executor<EM, I, S, Z> {
     ) -> Result<ExitKind, Error>;
 }
 
-/// A trait that allows to get/set an `Executor`'s timeout thresold
+/// A trait that allows to get an `Executor`'s timeout threshold
 pub trait HasTimeout {
     /// Get a timeout
     fn timeout(&self) -> Duration;
+}
 
+/// A trait that allows to set an `Executor`'s timeout threshold
+pub trait SetTimeout {
     /// Set timeout
     fn set_timeout(&mut self, timeout: Duration);
 }

crates/libafl/src/executors/nop.rs

@@ -5,6 +5,7 @@ use core::time::Duration;
 use libafl_bolts::tuples::RefIndexable;
 
 use super::{Executor, ExitKind, HasObservers, HasTimeout};
+use crate::executors::SetTimeout;
 
 /// [`NopExecutor`] is an executor that does nothing
 pub type NopExecutor = ConstantExecutor<()>;
@@ -64,7 +65,9 @@ impl<OT> HasTimeout for ConstantExecutor<OT> {
     fn timeout(&self) -> Duration {
         self.tm
     }
+}
 
+impl<OT> SetTimeout for ConstantExecutor<OT> {
     fn set_timeout(&mut self, timeout: Duration) {
         self.tm = timeout;
     }

crates/libafl/src/executors/sand.rs

@@ -16,6 +16,7 @@ use libafl_bolts::{
 use super::{Executor, ExecutorsTuple, ExitKind, HasObservers, HasTimeout};
 use crate::{
     HasNamedMetadata,
+    executors::SetTimeout,
     observers::{MapObserver, classify_counts},
 };
 
@@ -105,7 +106,12 @@ where
     fn timeout(&self) -> core::time::Duration {
         self.executor.timeout()
     }
+}
 
+impl<E, ET, C, O> SetTimeout for SANDExecutor<E, ET, C, O>
+where
+    E: SetTimeout,
+{
     fn set_timeout(&mut self, timeout: core::time::Duration) {
         self.executor.set_timeout(timeout);
     }

crates/libafl/src/executors/shadow.rs

@@ -8,7 +8,7 @@ use core::{
 
 use libafl_bolts::tuples::RefIndexable;
 
-use super::HasTimeout;
+use super::{HasTimeout, SetTimeout};
 use crate::{
     Error,
     executors::{Executor, ExitKind, HasObservers},
@@ -96,16 +96,22 @@ impl<E, I, S, SOT> HasTimeout for ShadowExecutor<E, I, S, SOT>
 where
     E: HasTimeout,
 {
-    #[inline]
-    fn set_timeout(&mut self, timeout: Duration) {
-        self.executor.set_timeout(timeout);
-    }
     #[inline]
     fn timeout(&self) -> Duration {
         self.executor.timeout()
     }
 }
 
+impl<E, I, S, SOT> SetTimeout for ShadowExecutor<E, I, S, SOT>
+where
+    E: SetTimeout,
+{
+    #[inline]
+    fn set_timeout(&mut self, timeout: Duration) {
+        self.executor.set_timeout(timeout);
+    }
+}
+
 impl<E, I, S, SOT> HasObservers for ShadowExecutor<E, I, S, SOT>
 where
     E: HasObservers,

crates/libafl/src/stages/verify_timeouts.rs

@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize, de::DeserializeOwned};
 
 use crate::{
     Evaluator, HasMetadata,
-    executors::{Executor, HasObservers, HasTimeout},
+    executors::{Executor, HasObservers, SetTimeout},
     inputs::BytesInput,
     observers::ObserversTuple,
     stages::{Restartable, Stage},
@@ -82,7 +82,7 @@ impl<I> TimeoutsToVerify<I> {
 impl<E, EM, I, S, Z> Stage<E, EM, S, Z> for VerifyTimeoutsStage<E, I, S>
 where
     E::Observers: ObserversTuple<I, S>,
-    E: Executor<EM, I, S, Z> + HasObservers + HasTimeout,
+    E: Executor<EM, I, S, Z> + HasObservers + SetTimeout,
     Z: Evaluator<E, EM, I, S>,
     S: HasMetadata,
     I: Debug + Serialize + DeserializeOwned + Default + 'static + Clone,

crates/libafl_nyx/src/executor.rs

@@ -6,7 +6,7 @@ use std::{
 
 use libafl::{
     Error,
-    executors::{Executor, ExitKind, HasObservers, HasTimeout},
+    executors::{Executor, ExitKind, HasObservers, HasTimeout, SetTimeout},
     inputs::HasTargetBytes,
     observers::{ObserversTuple, StdOutObserver},
     state::HasExecutions,
@@ -148,7 +148,9 @@ impl<S, OT> HasTimeout for NyxExecutor<S, OT> {
     fn timeout(&self) -> core::time::Duration {
         self.helper.timeout
     }
+}
 
+impl<S, OT> SetTimeout for NyxExecutor<S, OT> {
     fn set_timeout(&mut self, timeout: core::time::Duration) {
         let micros = 1000000;
         let mut timeout_secs = timeout.as_secs();