From d7b5d55408b17171f290a75a7c57d3d4c007d058 Mon Sep 17 00:00:00 2001
From: Dominik Maier <domenukk@gmail.com>
Date: Wed, 3 Jul 2024 16:25:12 +0200
Subject: [PATCH] Make sure inmemory_ondisk corpus catches filesystem errors
 correctly (#2361)

* Make sure inmemory_ondisk corpus catches filesystem errors correctly

* clip

* change names to be clearer
---
 libafl/src/corpus/inmemory_ondisk.rs | 66 +++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 16 deletions(-)

diff --git a/libafl/src/corpus/inmemory_ondisk.rs b/libafl/src/corpus/inmemory_ondisk.rs
index 19f0a04bb7..25dad5af7c 100644
--- a/libafl/src/corpus/inmemory_ondisk.rs
+++ b/libafl/src/corpus/inmemory_ondisk.rs
@@ -9,6 +9,7 @@ use core::cell::RefCell;
 use std::{fs, fs::File, io::Write};
 use std::{
     fs::OpenOptions,
+    io,
     path::{Path, PathBuf},
 };
 
@@ -26,6 +27,25 @@ use crate::{
     Error, HasMetadata,
 };
 
+/// Creates the given `path` and returns an error if it fails.
+/// If the create succeeds, it will return the file.
+/// If the create fails for _any_ reason, including, but not limited to, a preexisting existing file of that name,
+/// it will instead return the respective [`io::Error`].
+fn create_new<P: AsRef<Path>>(path: P) -> Result<File, io::Error> {
+    OpenOptions::new().write(true).create_new(true).open(path)
+}
+
+/// Tries to create the given `path` and returns `None` _only_ if the file already existed.
+/// If the create succeeds, it will return the file.
+/// If the create fails for some other reason, it will instead return the respective [`io::Error`].
+fn try_create_new<P: AsRef<Path>>(path: P) -> Result<Option<File>, io::Error> {
+    match create_new(path) {
+        Ok(ret) => Ok(Some(ret)),
+        Err(err) if err.kind() == io::ErrorKind::AlreadyExists => Ok(None),
+        Err(err) => Err(err),
+    }
+}
+
 /// A corpus able to store [`Testcase`]s to disk, while also keeping all of them in memory.
 ///
 /// Metadata is written to a `.<filename>.metadata` file in the same folder by default.
@@ -295,7 +315,7 @@ where
     ) -> Result<Self, Error> {
         match fs::create_dir_all(dir_path) {
             Ok(()) => {}
-            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
+            Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}
             Err(e) => return Err(e.into()),
         }
         Ok(InMemoryOnDiskCorpus {
@@ -331,16 +351,11 @@ where
                 let new_lock_filename = format!(".{new_filename}.lafl_lock");
 
                 // Try to create lock file for new testcases
-                if OpenOptions::new()
-                    .create_new(true)
-                    .write(true)
-                    .open(self.dir_path.join(new_lock_filename))
-                    .is_err()
-                {
+                if let Err(err) = create_new(self.dir_path.join(&new_lock_filename)) {
                     *testcase.filename_mut() = Some(old_filename);
-                    return Err(Error::illegal_state(
-                        "unable to create lock file for new testcase",
-                    ));
+                    return Err(Error::illegal_state(format!(
+                        "Unable to create lock file {new_lock_filename} for new testcase: {err}"
+                    )));
                 }
             }
 
@@ -387,12 +402,7 @@ where
                 let lockfile_name = format!(".{file_name}.lafl_lock");
                 let lockfile_path = self.dir_path.join(lockfile_name);
 
-                if OpenOptions::new()
-                    .write(true)
-                    .create_new(true)
-                    .open(lockfile_path)
-                    .is_ok()
-                {
+                if try_create_new(lockfile_path)?.is_some() {
                     break file_name;
                 }
 
@@ -465,3 +475,27 @@ where
         &self.dir_path
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::{env, fs, io::Write};
+
+    use super::{create_new, try_create_new};
+
+    #[test]
+    fn test() {
+        let tmp = env::temp_dir();
+        let path = tmp.join("testfile.tmp");
+        _ = fs::remove_file(&path);
+        let mut f = create_new(&path).unwrap();
+        f.write_all(&[0; 1]).unwrap();
+
+        match try_create_new(&path) {
+            Ok(None) => (),
+            Ok(_) => panic!("File {path:?} did not exist even though it should have?"),
+            Err(e) => panic!("An unexpected error occurred: {e}"),
+        };
+        drop(f);
+        fs::remove_file(path).unwrap();
+    }
+}