From a0143d02a61c728cf2d2b30cf30fe045418f0458 Mon Sep 17 00:00:00 2001 From: Dominik Maier Date: Mon, 18 Mar 2024 08:46:48 +0100 Subject: [PATCH] Rename OSError -> OsError and merge with redundant Error::File (#1944) * OSError -> OsError * Move File errors to io Errors * Removing File errors * fixes :) * More format * fix libfuzzer runtime --- libafl/src/events/tcp.rs | 2 +- libafl/src/executors/forkserver.rs | 8 ++-- libafl/src/stages/dump.rs | 10 ++++- libafl_bolts/src/lib.rs | 39 ++++++++-------- libafl_bolts/src/llmp.rs | 4 +- libafl_bolts/src/os/mod.rs | 6 ++- libafl_bolts/src/shmem.rs | 44 +++++++------------ .../libafl_libfuzzer_runtime/src/corpus.rs | 6 +-- .../libafl_libfuzzer_runtime/src/merge.rs | 3 +- .../libafl_libfuzzer_runtime/src/options.rs | 6 +-- libafl_qemu/src/snapshot.rs | 2 +- scripts/fmt_all.sh | 7 +++ 12 files changed, 69 insertions(+), 68 deletions(-) diff --git a/libafl/src/events/tcp.rs b/libafl/src/events/tcp.rs index daefd57d19..4f647f24f9 100644 --- a/libafl/src/events/tcp.rs +++ b/libafl/src/events/tcp.rs @@ -1202,7 +1202,7 @@ where return Err(Error::shutting_down()); } - Err(Error::File(_, _)) => { + Err(Error::OsError(..)) => { // port was likely already bound let mgr = TcpEventManager::::with_hooks( &("127.0.0.1", self.broker_port), diff --git a/libafl/src/executors/forkserver.rs b/libafl/src/executors/forkserver.rs index d11b61a02c..18588a93ec 100644 --- a/libafl/src/executors/forkserver.rs +++ b/libafl/src/executors/forkserver.rs @@ -445,10 +445,10 @@ impl Forkserver { pub fn read_st_timed(&mut self, timeout: &TimeSpec) -> Result, Error> { let mut buf: [u8; 4] = [0_u8; 4]; let Some(st_read) = self.st_pipe.read_end() else { - return Err(Error::file(io::Error::new( - ErrorKind::BrokenPipe, - "Read pipe end was already closed", - ))); + return Err(Error::os_error( + io::Error::new(ErrorKind::BrokenPipe, "Read pipe end was already closed"), + "read_st_timed failed", + )); }; // # Safety diff --git a/libafl/src/stages/dump.rs b/libafl/src/stages/dump.rs index 1daf0794d9..61a8f80d8d 100644 --- a/libafl/src/stages/dump.rs +++ b/libafl/src/stages/dump.rs @@ -142,13 +142,19 @@ where let corpus_dir = corpus_dir.into(); if let Err(e) = fs::create_dir(&corpus_dir) { if !corpus_dir.is_dir() { - return Err(Error::file(e)); + return Err(Error::os_error( + e, + format!("Error creating directory {corpus_dir:?}"), + )); } } let solutions_dir = solutions_dir.into(); if let Err(e) = fs::create_dir(&solutions_dir) { if !corpus_dir.is_dir() { - return Err(Error::file(e)); + return Err(Error::os_error( + e, + format!("Error creating directory {solutions_dir:?}"), + )); } } Ok(Self { diff --git a/libafl_bolts/src/lib.rs b/libafl_bolts/src/lib.rs index 2c8919250e..49cc417308 100644 --- a/libafl_bolts/src/lib.rs +++ b/libafl_bolts/src/lib.rs @@ -295,9 +295,6 @@ pub enum Error { /// Compression error #[cfg(feature = "gzip")] Compression(ErrorBacktrace), - /// File related error - #[cfg(feature = "std")] - File(io::Error, ErrorBacktrace), /// Optional val was supposed to be set, but isn't. EmptyOptional(String, ErrorBacktrace), /// Key not in Map @@ -316,9 +313,9 @@ pub enum Error { Unsupported(String, ErrorBacktrace), /// Shutting down, not really an error. ShuttingDown, - /// OS error from `std::io::Error::last_os_error`; + /// OS error, wrapping a [`std::io::Error`] #[cfg(feature = "std")] - OSError(io::Error, String, ErrorBacktrace), + OsError(io::Error, String, ErrorBacktrace), /// Something else happened Unknown(String, ErrorBacktrace), } @@ -338,12 +335,6 @@ impl Error { pub fn compression() -> Self { Error::Compression(ErrorBacktrace::new()) } - #[cfg(feature = "std")] - /// File related error - #[must_use] - pub fn file(arg: io::Error) -> Self { - Error::File(arg, ErrorBacktrace::new()) - } /// Optional val was supposed to be set, but isn't. #[must_use] pub fn empty_optional(arg: S) -> Self @@ -413,14 +404,27 @@ impl Error { { Error::Unsupported(arg.into(), ErrorBacktrace::new()) } + /// OS error with additional message #[cfg(feature = "std")] - /// OS error from `std::io::Error::last_os_error`; #[must_use] pub fn os_error(err: io::Error, msg: S) -> Self where S: Into, { - Error::OSError(err, msg.into(), ErrorBacktrace::new()) + Error::OsError(err, msg.into(), ErrorBacktrace::new()) + } + /// OS error from [`std::io::Error::last_os_error`] with additional message + #[cfg(feature = "std")] + #[must_use] + pub fn last_os_error(msg: S) -> Self + where + S: Into, + { + Error::OsError( + io::Error::last_os_error(), + msg.into(), + ErrorBacktrace::new(), + ) } /// Something else happened #[must_use] @@ -444,11 +448,6 @@ impl Display for Error { write!(f, "Error in decompression")?; display_error_backtrace(f, b) } - #[cfg(feature = "std")] - Self::File(err, b) => { - write!(f, "File IO failed: {:?}", &err)?; - display_error_backtrace(f, b) - } Self::EmptyOptional(s, b) => { write!(f, "Optional value `{0}` was not set", &s)?; display_error_backtrace(f, b) @@ -487,7 +486,7 @@ impl Display for Error { } Self::ShuttingDown => write!(f, "Shutting down!"), #[cfg(feature = "std")] - Self::OSError(err, s, b) => { + Self::OsError(err, s, b) => { write!(f, "OS error: {0}: {1}", &s, err)?; display_error_backtrace(f, b) } @@ -544,7 +543,7 @@ impl From for Error { #[cfg(feature = "std")] impl From for Error { fn from(err: io::Error) -> Self { - Self::file(err) + Self::os_error(err, "io::Error ocurred") } } diff --git a/libafl_bolts/src/llmp.rs b/libafl_bolts/src/llmp.rs index 3d1b786766..e983c76892 100644 --- a/libafl_bolts/src/llmp.rs +++ b/libafl_bolts/src/llmp.rs @@ -705,7 +705,7 @@ where let _listener_thread = broker.launch_listener(Listener::Tcp(listener))?; Ok(LlmpConnection::IsBroker { broker }) } - Err(Error::File(e, _)) if e.kind() == ErrorKind::AddrInUse => { + Err(Error::OsError(e, ..)) if e.kind() == ErrorKind::AddrInUse => { // We are the client :) log::info!("We're the client (internal port already bound by broker, {e:#?})"); Ok(LlmpConnection::IsClient { @@ -2623,7 +2623,7 @@ where .expect("B2B: Error forwarding message. Exiting."); } Err(e) => { - if let Error::File(e, _) = e { + if let Error::OsError(e, ..) = e { if e.kind() == ErrorKind::UnexpectedEof { log::info!( "Broker {peer_address} seems to have disconnected, exiting" diff --git a/libafl_bolts/src/os/mod.rs b/libafl_bolts/src/os/mod.rs index 2af85e2651..69ff1ad954 100644 --- a/libafl_bolts/src/os/mod.rs +++ b/libafl_bolts/src/os/mod.rs @@ -107,7 +107,7 @@ pub fn startable_self() -> Result { #[cfg(all(unix, feature = "std"))] pub fn dup(fd: RawFd) -> Result { match unsafe { libc::dup(fd) } { - -1 => Err(Error::file(std::io::Error::last_os_error())), + -1 => Err(Error::last_os_error(format!("Error calling dup({fd})"))), new_fd => Ok(new_fd), } } @@ -119,7 +119,9 @@ pub fn dup(fd: RawFd) -> Result { #[cfg(all(unix, feature = "std"))] pub fn dup2(fd: RawFd, device: RawFd) -> Result<(), Error> { match unsafe { libc::dup2(fd, device) } { - -1 => Err(Error::file(std::io::Error::last_os_error())), + -1 => Err(Error::last_os_error(format!( + "Error calling dup2({fd}, {device})" + ))), _ => Ok(()), } } diff --git a/libafl_bolts/src/shmem.rs b/libafl_bolts/src/shmem.rs index 3ce44c138d..6394685fdb 100644 --- a/libafl_bolts/src/shmem.rs +++ b/libafl_bolts/src/shmem.rs @@ -590,7 +590,7 @@ pub mod unix_shmem { use alloc::string::ToString; use core::{ptr, slice}; - use std::{io, io::Write, process}; + use std::{io::Write, process}; use libc::{ c_int, c_uchar, close, ftruncate, mmap, munmap, shm_open, shm_unlink, shmat, shmctl, @@ -648,21 +648,17 @@ pub mod unix_shmem { 0o600, ); if shm_fd == -1 { - return Err(Error::os_error( - io::Error::last_os_error(), - format!("Failed to shm_open map with id {filename_path:?}",), - )); + return Err(Error::last_os_error(format!( + "Failed to shm_open map with id {filename_path:?}", + ))); } /* configure the size of the shared memory segment */ if ftruncate(shm_fd, map_size.try_into()?) != 0 { shm_unlink(filename_path.as_ptr() as *const _); - return Err(Error::os_error( - io::Error::last_os_error(), - format!( - "setup_shm(): ftruncate() failed for map with id {filename_path:?}", - ), - )); + return Err(Error::last_os_error(format!( + "setup_shm(): ftruncate() failed for map with id {filename_path:?}", + ))); } /* map the shared memory segment to the address space of the process */ @@ -677,10 +673,9 @@ pub mod unix_shmem { if map == libc::MAP_FAILED || map.is_null() { close(shm_fd); shm_unlink(filename_path.as_ptr() as *const _); - return Err(Error::os_error( - io::Error::last_os_error(), - format!("mmap() failed for map with id {filename_path:?}",), - )); + return Err(Error::last_os_error(format!( + "mmap() failed for map with id {filename_path:?}", + ))); } Ok(Self { @@ -708,10 +703,9 @@ pub mod unix_shmem { ); if map == libc::MAP_FAILED || map.is_null() { close(shm_fd); - return Err(Error::os_error( - io::Error::last_os_error(), - format!("mmap() failed for map with fd {shm_fd:?}"), - )); + return Err(Error::last_os_error(format!( + "mmap() failed for map with fd {shm_fd:?}" + ))); } Ok(Self { @@ -854,10 +848,7 @@ pub mod unix_shmem { let map = shmat(os_id, ptr::null(), 0) as *mut c_uchar; if map as c_int == -1 || map.is_null() { - return Err(Error::os_error( - io::Error::last_os_error(), - "Failed to map the shared mapping", - )); + return Err(Error::last_os_error("Failed to map the shared mapping")); } Ok(Self { @@ -875,10 +866,9 @@ pub mod unix_shmem { let map = shmat(id_int, ptr::null(), 0) as *mut c_uchar; if map.is_null() || map == ptr::null_mut::().wrapping_sub(1) { - return Err(Error::os_error( - io::Error::last_os_error(), - format!("Failed to map the shared mapping with id {id_int}"), - )); + return Err(Error::last_os_error(format!( + "Failed to map the shared mapping with id {id_int}" + ))); } Ok(Self { id, map, map_size }) diff --git a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/corpus.rs b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/corpus.rs index 9f473bf22c..64f6bace30 100644 --- a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/corpus.rs +++ b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/corpus.rs @@ -114,7 +114,7 @@ where let path = self.corpus_dir.join(&name); match input.to_file(&path) { - Err(Error::File(e, _)) if e.kind() == ErrorKind::AlreadyExists => { + Err(Error::OsError(e, ..)) if e.kind() == ErrorKind::AlreadyExists => { // we do not care if the file already exists; in this case, we assume it is equal } res => res?, @@ -192,7 +192,7 @@ where Error::empty("The testcase, when being saved, must have a file path!") })?; match input.to_file(path) { - Err(Error::File(e, _)) if e.kind() == ErrorKind::AlreadyExists => { + Err(Error::OsError(e, ..)) if e.kind() == ErrorKind::AlreadyExists => { // we do not care if the file already exists; in this case, we assume it is equal Ok(()) } @@ -250,7 +250,7 @@ where Error::illegal_state("Should have set the path in the LibfuzzerCrashCauseFeedback.") })?; match input.to_file(path) { - Err(Error::File(e, _)) if e.kind() == ErrorKind::AlreadyExists => { + Err(Error::OsError(e, ..)) if e.kind() == ErrorKind::AlreadyExists => { // we do not care if the file already exists; in this case, we assume it is equal } res => res?, diff --git a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/merge.rs b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/merge.rs index 569255ac08..cfe6cdf599 100644 --- a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/merge.rs +++ b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/merge.rs @@ -4,7 +4,8 @@ use std::{ fs::{rename, File}, io::Write, os::fd::{AsRawFd, FromRawFd}, - time::{SystemTime, UNIX_EPOCH}, ptr::addr_of_mut, + ptr::addr_of_mut, + time::{SystemTime, UNIX_EPOCH}, }; use libafl::{ diff --git a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/options.rs b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/options.rs index 8e26c73dc0..98d9172234 100644 --- a/libafl_libfuzzer/libafl_libfuzzer_runtime/src/options.rs +++ b/libafl_libfuzzer/libafl_libfuzzer_runtime/src/options.rs @@ -394,11 +394,7 @@ impl<'a> LibfuzzerOptionsBuilder<'a> { tui: self.tui, runs: self.runs, close_fd_mask: self.close_fd_mask, - unknown: self - .unknown - .into_iter() - .map(ToString::to_string) - .collect(), + unknown: self.unknown.into_iter().map(ToString::to_string).collect(), } } } diff --git a/libafl_qemu/src/snapshot.rs b/libafl_qemu/src/snapshot.rs index 8cac85cba1..76460cbfeb 100644 --- a/libafl_qemu/src/snapshot.rs +++ b/libafl_qemu/src/snapshot.rs @@ -462,7 +462,7 @@ impl QemuSnapshotHelper { )); } - for (i, _, _) in found { + for (i, ..) in found { new_maps.tree.delete(i); } } diff --git a/scripts/fmt_all.sh b/scripts/fmt_all.sh index e73018c042..948fbdc467 100755 --- a/scripts/fmt_all.sh +++ b/scripts/fmt_all.sh @@ -26,3 +26,10 @@ do cargo +nightly fmt --all popd || exit 1 done + +echo "[*] Formatting libafl_libfuzzer_runtime" +pushd "libafl_libfuzzer/libafl_libfuzzer_runtime" || exit 1 +cargo +nightly fmt --all +popd || exit 1 + +echo "[*] Done :)" \ No newline at end of file