qemu config refactoring

libafl_qemu/src/emu/builder.rs

@@ -1,4 +1,7 @@
-use std::{fmt::Debug, marker::PhantomData};
+use std::{
+    fmt::{Debug, Formatter},
+    marker::PhantomData,
+};
 
 use libafl::{
     inputs::{HasTargetBytes, UsesInput},
@@ -17,10 +20,9 @@ use crate::{
 };
 
 #[derive(Clone, Debug)]
-enum QemuBuilder {
-    Qemu(Qemu),
-    QemuConfig(QemuConfig),
-    QemuString(Vec<String>),
+pub enum QemuParams {
+    Config(QemuConfig),
+    Cli(Vec<String>),
 }
 
 #[derive(Clone, Debug)]
@@ -32,7 +34,7 @@ where
     driver: ED,
     snapshot_manager: SM,
     command_manager: CM,
-    qemu_builder: Option<QemuBuilder>,
+    qemu_parameters: Option<QemuParams>,
     phantom: PhantomData<S>,
 }
 
@@ -47,7 +49,7 @@ where
             driver: NopEmulatorDriver,
             snapshot_manager: NopSnapshotManager,
             command_manager: NopCommandManager,
-            qemu_builder: None,
+            qemu_parameters: None,
             phantom: PhantomData,
         }
     }
@@ -67,7 +69,7 @@ where
             command_manager: StdCommandManager::default(),
             snapshot_manager: StdSnapshotManager::default(),
             driver: StdEmulatorDriver::builder().build(),
-            qemu_builder: None,
+            qemu_parameters: None,
             phantom: PhantomData,
         }
     }
@@ -85,7 +87,7 @@ where
             command_manager: StdCommandManager::default(),
             snapshot_manager: FastSnapshotManager::default(),
             driver: StdEmulatorDriver::builder().build(),
-            qemu_builder: None,
+            qemu_parameters: None,
             phantom: PhantomData,
         }
     }
@@ -99,14 +101,14 @@ where
         driver: ED,
         command_manager: CM,
         snapshot_manager: SM,
-        qemu_builder: Option<QemuBuilder>,
+        qemu_parameters: Option<QemuParams>,
     ) -> Self {
         Self {
             modules,
             command_manager,
             driver,
             snapshot_manager,
-            qemu_builder,
+            qemu_parameters,
             phantom: PhantomData,
         }
     }
@@ -116,20 +118,13 @@ where
         CM: CommandManager<ED, ET, S, SM>,
         ET: EmulatorModuleTuple<S>,
     {
-        let qemu_builder = self.qemu_builder.ok_or(QemuInitError::EmptyArgs)?;
+        let qemu_parameters = self.qemu_parameters.ok_or(QemuInitError::EmptyArgs)?;
 
         let mut emulator_hooks = unsafe { EmulatorHooks::new(QemuHooks::get_unchecked()) };
 
         self.modules.pre_qemu_init_all(&mut emulator_hooks);
 
-        let qemu: Qemu = match qemu_builder {
-            QemuBuilder::Qemu(qemu) => qemu,
-            QemuBuilder::QemuConfig(qemu_config) => {
-                let res: Result<Qemu, QemuInitError> = qemu_config.into();
-                res?
-            }
-            QemuBuilder::QemuString(qemu_string) => Qemu::init(&qemu_string)?,
-        };
+        let qemu = Qemu::init_with_params(&qemu_parameters)?;
 
         unsafe {
             Ok(Emulator::new_with_qemu(
@@ -156,7 +151,7 @@ where
             self.driver,
             self.command_manager,
             self.snapshot_manager,
-            Some(QemuBuilder::QemuConfig(qemu_config)),
+            Some(QemuParams::Config(qemu_config)),
         )
     }
 
@@ -167,18 +162,7 @@ where
             self.driver,
             self.command_manager,
             self.snapshot_manager,
-            Some(QemuBuilder::QemuString(qemu_cli)),
-        )
-    }
-
-    #[must_use]
-    pub fn qemu(self, qemu: Qemu) -> EmulatorBuilder<CM, ED, ET, S, SM> {
-        EmulatorBuilder::new(
-            self.modules,
-            self.driver,
-            self.command_manager,
-            self.snapshot_manager,
-            Some(QemuBuilder::Qemu(qemu)),
+            Some(QemuParams::Cli(qemu_cli)),
         )
     }
 
@@ -192,7 +176,7 @@ where
             self.driver,
             self.command_manager,
             self.snapshot_manager,
-            self.qemu_builder,
+            self.qemu_parameters,
         )
     }
 
@@ -202,7 +186,7 @@ where
             driver,
             self.command_manager,
             self.snapshot_manager,
-            self.qemu_builder,
+            self.qemu_parameters,
         )
     }
 
@@ -215,7 +199,7 @@ where
             self.driver,
             command_manager,
             self.snapshot_manager,
-            self.qemu_builder,
+            self.qemu_parameters,
         )
     }
 
@@ -225,7 +209,7 @@ where
             self.driver,
             self.command_manager,
             self.snapshot_manager,
-            self.qemu_builder,
+            self.qemu_parameters,
         )
     }
 
@@ -238,7 +222,7 @@ where
             self.driver,
             self.command_manager,
             snapshot_manager,
-            self.qemu_builder,
+            self.qemu_parameters,
         )
     }
 }

libafl_qemu/src/qemu/config.rs

@@ -2,20 +2,13 @@ use core::{
     fmt,
     fmt::{Display, Formatter},
 };
-use std::{
-    path::{Path, PathBuf},
-    sync::OnceLock,
-};
+use std::path::{Path, PathBuf};
 
 use getset::Getters;
 use libafl_derive;
 use strum_macros;
 use typed_builder::TypedBuilder;
 
-use crate::{Qemu, QemuInitError};
-
-pub(super) static QEMU_CONFIG: OnceLock<QemuConfig> = OnceLock::new();
-
 #[cfg(feature = "systemmode")]
 #[derive(Debug, strum_macros::Display, Clone)]
 #[strum(prefix = "-accel ", serialize_all = "lowercase")]
@@ -304,14 +297,16 @@ impl<R: AsRef<Path>> From<R> for Program {
 }
 
 #[derive(Debug, Clone, libafl_derive::Display, TypedBuilder, Getters)]
-#[builder(build_method(into = Result<Qemu, QemuInitError>), builder_method(vis = "pub(crate)",
+#[builder(builder_method(
+    vis = "pub(crate)",
     doc = "Since Qemu is a zero sized struct, this is not a completely standard builder pattern. \
     The Qemu configuration is not stored in the Qemu struct after build() but in QEMU_CONFIG \
     Therefore, to use the derived builder and avoid boilerplate a builder for QemuConfig is \
     derived. \
     The QemuConfig::builder is called in Qemu::builder() which is the only place where it should \
     be called, in this way the one to one matching of Qemu and QemuConfig is enforced. Therefore \
-    its visibility is pub(crate)"))]
+    its visibility is pub(crate)"
+))]
 #[getset(get = "pub")]
 pub struct QemuConfig {
     #[cfg(feature = "systemmode")]
@@ -350,40 +345,28 @@ pub struct QemuConfig {
     program: Program,
 } // Adding something here? Please leave Program as the last field
 
-impl From<QemuConfig> for Result<Qemu, QemuInitError> {
-    /// This method is necessary to make the API resemble a typical builder pattern, i.e.
-    /// `Qemu::builder().foo(bar).build()`, while still leveraging `TypedBuilder` for this
-    /// non-standard use case where `Qemu` doesn't store the configuration.
-    /// Internally, `TypedBuilder` is used to generate a builder for `QemuConfig`.
-    /// This `QemuConfig.into()` method is used by the derived `QemuConfigBuilder.build()`
-    /// to go from `QemuConfigBuilder` to `QemuConfig`, and finally to `Qemu` in one fn.
-    ///
-    /// # Errors
-    /// returns `QemuInitError` if the Qemu initialization fails, including cases where Qemu has
-    /// already been initialized.
-    fn from(config: QemuConfig) -> Self {
-        let args = config
+impl From<&QemuConfig> for Vec<String> {
+    /// Generate the QEMU-compatible initialization cli string from the QEMU config.
+    fn from(config: &QemuConfig) -> Self {
+        config
             .to_string()
             .split(' ')
             .map(ToString::to_string)
-            .collect::<Vec<String>>();
-        let qemu = Qemu::init(&args)?;
-        QEMU_CONFIG
-            .set(config)
-            .map_err(|_| unreachable!("BUG: QEMU_CONFIG was already set but Qemu was not init!"))?;
-        Ok(qemu)
+            .collect::<Vec<String>>()
     }
 }
 
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::Qemu;
 
     #[test]
     #[cfg(feature = "usermode")]
     fn usermode() {
         let program = "/bin/pwd";
-        let qemu = Qemu::builder().program("/bin/pwd").build().unwrap();
+        let qemu_config = QemuConfig::builder().program("/bin/pwd").build();
+        let qemu = Qemu::init_with_config(&qemu_config).unwrap();
         let config = qemu.get_config().unwrap();
         assert_eq!(config.to_string().trim(), program.trim());
     }

libafl_qemu/src/qemu/mod.rs

@@ -14,6 +14,7 @@ use std::{
     mem::MaybeUninit,
     ops::Range,
     pin::Pin,
+    sync::OnceLock,
 };
 
 use libafl_bolts::os::unix_signals::Signal;
@@ -28,10 +29,10 @@ use libafl_qemu_sys::{
 use num_traits::Num;
 use strum::IntoEnumIterator;
 
-use crate::{GuestAddrKind, GuestReg, Regs};
+use crate::{GuestAddrKind, GuestReg, QemuParams, Regs};
 
 pub mod config;
-use config::{QemuConfig, QemuConfigBuilder, QEMU_CONFIG};
+use config::QemuConfig;
 
 #[cfg(feature = "usermode")]
 mod usermode;
@@ -49,6 +50,8 @@ pub use hooks::*;
 
 static mut QEMU_IS_INITIALIZED: bool = false;
 
+pub(super) static QEMU_CONFIG: OnceLock<QemuConfig> = OnceLock::new();
+
 #[derive(Debug)]
 pub enum QemuError {
     Init(QemuInitError),
@@ -574,10 +577,21 @@ impl From<u8> for HookData {
 
 #[allow(clippy::unused_self)]
 impl Qemu {
-    /// For more details about the parameters check
-    /// [the QEMU documentation](https://www.qemu.org/docs/master/about/).
-    pub fn builder() -> QemuConfigBuilder {
-        QemuConfig::builder()
+    pub fn init_with_params(params: &QemuParams) -> Result<Self, QemuInitError> {
+        match params {
+            QemuParams::Config(config) => Self::init_with_config(config),
+            QemuParams::Cli(cli) => Self::init(cli.as_ref()),
+        }
+    }
+
+    pub fn init_with_config(config: &QemuConfig) -> Result<Self, QemuInitError> {
+        let qemu_args: Vec<String> = config.into();
+
+        QEMU_CONFIG
+            .set(config.clone())
+            .map_err(|_| unreachable!("BUG: QEMU_CONFIG was already set but Qemu was not init!"))?;
+
+        Self::init(qemu_args.as_ref())
     }
 
     #[allow(clippy::must_use_candidate, clippy::similar_names)]