Make crashdumps optional in config (#3409)

Author: Porges

src/ApiService/ApiService/onefuzzlib/Defs.cs

@@ -192,7 +192,7 @@ public static class Defs {
                         ),
                         new ContainerDefinition(
                             Type:ContainerType.Crashdumps,
-                            Compare: Compare.Equal,
+                            Compare: Compare.AtMost,
                             Value:1,
                             Permissions: ContainerPermission.Write
                             ),
@@ -287,7 +287,7 @@ public static class Defs {
                         ),
                         new ContainerDefinition(
                             Type:ContainerType.Crashdumps,
-                            Compare: Compare.Equal,
+                            Compare: Compare.AtMost,
                             Value:1,
                             Permissions: ContainerPermission.Write
                             ),

src/agent/onefuzz-task/src/local/libfuzzer_fuzz.rs

@@ -51,7 +51,7 @@ pub fn build_fuzz_config(
         inputs,
         readonly_inputs,
         crashes,
-        crashdumps,
+        crashdumps: Some(crashdumps),
         target_exe,
         target_env,
         target_options,

src/agent/onefuzz-task/src/local/template.rs

@@ -144,7 +144,9 @@ impl TaskConfig {
                     inputs: context.to_monitored_sync_dir("inputs", &config.inputs)?,
                     readonly_inputs: Some(ri?),
                     crashes: context.to_monitored_sync_dir("crashes", &config.crashes)?,
-                    crashdumps: context.to_monitored_sync_dir("crashdumps", &config.crashdumps)?,
+                    crashdumps: Some(
+                        context.to_monitored_sync_dir("crashdumps", &config.crashdumps)?,
+                    ),
                     target_exe: config.target_exe.clone(),
                     target_env: config.target_env.clone(),
                     target_options: config.target_options.clone(),

src/agent/onefuzz-task/src/tasks/fuzz/libfuzzer/common.rs

@@ -74,7 +74,7 @@ pub struct Config<L: LibFuzzerType + Send + Sync + ?Sized> {
     pub inputs: SyncedDir,
     pub readonly_inputs: Option<Vec<SyncedDir>>,
     pub crashes: SyncedDir,
-    pub crashdumps: SyncedDir,
+    pub crashdumps: Option<SyncedDir>,
     pub target_exe: PathBuf,
     pub target_env: HashMap<String, String>,
     pub target_options: Vec<String>,
@@ -131,7 +131,13 @@ where
         let resync = self.continuous_sync_inputs();
         let new_inputs = self.config.inputs.monitor_results(new_coverage, true);
         let new_crashes = self.config.crashes.monitor_results(new_result, true);
-        let new_crashdumps = self.config.crashdumps.monitor_results(new_crashdump, true);
+        let new_crashdumps = async {
+            if let Some(crashdumps) = &self.config.crashdumps {
+                crashdumps.monitor_results(new_crashdump, true).await
+            } else {
+                Ok(())
+            }
+        };
 
         let (stats_sender, stats_receiver) = mpsc::unbounded_channel();
         let report_stats = report_runtime_stats(stats_receiver, hb_client);
@@ -350,64 +356,66 @@ where
             }
         }
 
-        // check for core dumps on Linux:
-        // note that collecting the dumps must be enabled by the template
-        #[cfg(target_os = "linux")]
-        if let Some(pid) = pid {
-            // expect crash dump to exist in CWD
-            let filename = format!("core.{pid}");
-            let dest_filename = dump_file_name.as_deref().unwrap_or(OsStr::new(&filename));
-            let dest_path = self.config.crashdumps.local_path.join(dest_filename);
-            match tokio::fs::rename(&filename, &dest_path).await {
-                Ok(()) => {
-                    info!(
-                        "moved crash dump {} to output directory: {}",
-                        filename,
-                        dest_path.display()
-                    );
-                }
-                Err(e) => {
-                    if e.kind() == std::io::ErrorKind::NotFound {
-                        // okay, no crash dump found
-                        info!("no crash dump found with name: {}", filename);
-                    } else {
-                        return Err(e).context("moving crash dump to output directory");
+        if let Some(crashdumps) = &self.config.crashdumps {
+            // check for core dumps on Linux:
+            // note that collecting the dumps must be enabled by the template
+            #[cfg(target_os = "linux")]
+            if let Some(pid) = pid {
+                // expect crash dump to exist in CWD
+                let filename = format!("core.{pid}");
+                let dest_filename = dump_file_name.as_deref().unwrap_or(OsStr::new(&filename));
+                let dest_path = crashdumps.local_path.join(dest_filename);
+                match tokio::fs::rename(&filename, &dest_path).await {
+                    Ok(()) => {
+                        info!(
+                            "moved crash dump {} to output directory: {}",
+                            filename,
+                            dest_path.display()
+                        );
+                    }
+                    Err(e) => {
+                        if e.kind() == std::io::ErrorKind::NotFound {
+                            // okay, no crash dump found
+                            info!("no crash dump found with name: {}", filename);
+                        } else {
+                            return Err(e).context("moving crash dump to output directory");
+                        }
                     }
                 }
+            } else {
+                warn!("no PID found for libfuzzer process");
             }
-        } else {
-            warn!("no PID found for libfuzzer process");
-        }
 
-        // check for crash dumps on Windows:
-        #[cfg(target_os = "windows")]
-        {
-            let dumpfile_extension = Some(std::ffi::OsStr::new("dmp"));
-
-            let mut working_dir = tokio::fs::read_dir(".").await?;
-            let mut found_dump = false;
-            while let Some(next) = working_dir.next_entry().await? {
-                if next.path().extension() == dumpfile_extension {
-                    // Windows dumps get a fixed filename so we will generate a random one,
-                    // if there's no valid target crash name:
-                    let dest_filename =
-                        dump_file_name.unwrap_or_else(|| uuid::Uuid::new_v4().to_string().into());
-                    let dest_path = self.config.crashdumps.local_path.join(&dest_filename);
-                    tokio::fs::rename(next.path(), &dest_path)
-                        .await
-                        .context("moving crash dump to output directory")?;
-                    info!(
-                        "moved crash dump {} to output directory: {}",
-                        next.path().display(),
-                        dest_path.display()
-                    );
-                    found_dump = true;
-                    break;
+            // check for crash dumps on Windows:
+            #[cfg(target_os = "windows")]
+            {
+                let dumpfile_extension = Some(std::ffi::OsStr::new("dmp"));
+
+                let mut working_dir = tokio::fs::read_dir(".").await?;
+                let mut found_dump = false;
+                while let Some(next) = working_dir.next_entry().await? {
+                    if next.path().extension() == dumpfile_extension {
+                        // Windows dumps get a fixed filename so we will generate a random one,
+                        // if there's no valid target crash name:
+                        let dest_filename = dump_file_name
+                            .unwrap_or_else(|| uuid::Uuid::new_v4().to_string().into());
+                        let dest_path = crashdumps.local_path.join(&dest_filename);
+                        tokio::fs::rename(next.path(), &dest_path)
+                            .await
+                            .context("moving crash dump to output directory")?;
+                        info!(
+                            "moved crash dump {} to output directory: {}",
+                            next.path().display(),
+                            dest_path.display()
+                        );
+                        found_dump = true;
+                        break;
+                    }
                 }
-            }
 
-            if !found_dump {
-                info!("no crash dump found with extension .dmp");
+                if !found_dump {
+                    info!("no crash dump found with extension .dmp");
+                }
             }
         }
 
@@ -425,7 +433,9 @@ where
 
         // output directories (init):
         self.config.crashes.init().await?;
-        self.config.crashdumps.init().await?;
+        if let Some(crashdumps) = &self.config.crashdumps {
+            crashdumps.init().await?;
+        }
 
         Ok(())
     }

src/agent/onefuzz-task/src/tasks/fuzz/supervisor.rs

@@ -41,7 +41,7 @@ use futures::TryFutureExt;
 pub struct SupervisorConfig {
     pub inputs: SyncedDir,
     pub crashes: SyncedDir,
-    pub crashdumps: SyncedDir,
+    pub crashdumps: Option<SyncedDir>,
     pub supervisor_exe: String,
     pub supervisor_env: HashMap<String, String>,
     pub supervisor_options: Vec<String>,
@@ -82,12 +82,29 @@ pub async fn spawn(config: SupervisorConfig) -> Result<(), Error> {
     let monitor_crashes = crashes.monitor_results(new_result, false);
 
     // setup crashdumps
-    let crashdumps = SyncedDir {
-        local_path: runtime_dir.path().join("crashdumps"),
-        remote_path: config.crashdumps.remote_path.clone(),
+    let (crashdump_dir, monitor_crashdumps) = {
+        let crashdump_dir = if let Some(crashdumps) = &config.crashdumps {
+            let dir = SyncedDir {
+                local_path: runtime_dir.path().join("crashdumps"),
+                remote_path: crashdumps.remote_path.clone(),
+            };
+            dir.init().await?;
+            Some(dir)
+        } else {
+            None
+        };
+
+        let monitor_dir = crashdump_dir.clone();
+        let monitor_crashdumps = async move {
+            if let Some(crashdumps) = monitor_dir {
+                crashdumps.monitor_results(new_crashdump, false).await
+            } else {
+                Ok(())
+            }
+        };
+
+        (crashdump_dir, monitor_crashdumps)
     };
-    crashdumps.init().await?;
-    let monitor_crashdumps = crashdumps.monitor_results(new_crashdump, false);
 
     // setup coverage
     if let Some(coverage) = &config.coverage {
@@ -148,7 +165,7 @@ pub async fn spawn(config: SupervisorConfig) -> Result<(), Error> {
         &runtime_dir.path(),
         &config,
         &crashes,
-        &crashdumps,
+        crashdump_dir.as_ref(),
         &inputs,
         reports_dir.path().to_path_buf(),
     )
@@ -217,7 +234,7 @@ async fn start_supervisor(
     runtime_dir: impl AsRef<Path>,
     config: &SupervisorConfig,
     crashes: &SyncedDir,
-    crashdumps: &SyncedDir,
+    crashdumps: Option<&SyncedDir>,
     inputs: &SyncedDir,
     reports_dir: PathBuf,
 ) -> Result<Child> {
@@ -233,7 +250,9 @@ async fn start_supervisor(
         .supervisor_options(&config.supervisor_options)
         .runtime_dir(&runtime_dir)
         .crashes(&crashes.local_path)
-        .crashdumps(&crashdumps.local_path)
+        .set_optional_ref(&crashdumps, |expand, crashdumps| {
+            expand.crashdumps(&crashdumps.local_path)
+        })
         .input_corpus(&inputs.local_path)
         .reports_dir(reports_dir)
         .setup_dir(&config.common.setup_dir)
@@ -410,7 +429,7 @@ mod tests {
             target_options,
             inputs: corpus_dir.clone(),
             crashes: crashes.clone(),
-            crashdumps: crashdumps.clone(),
+            crashdumps: Some(crashdumps.clone()),
             tools: None,
             wait_for_files: None,
             stats_file: None,
@@ -447,7 +466,7 @@ mod tests {
             runtime_dir,
             &config,
             &crashes,
-            &crashdumps,
+            Some(&crashdumps),
             &corpus_dir,
             reports_dir,
         )