From c2776e117a8f08a4e18b459459472e7b31c7e587 Mon Sep 17 00:00:00 2001 From: Andrea Fioraldi Date: Fri, 25 Nov 2022 11:57:08 +0100 Subject: [PATCH] emu::current_cpu() is now the CPU that hitted the breakpoint in fullsystem (#910) * emu::current_cpu() is now kept after vm stop and it is the CPU that hitted the breakpoint * clippy * uninit * clippy * clippy * clippy * clippy * nightly override in CI * nightly override in CI * components * components * targets * targets * clippy * clippy * clippy * clippy * clippy (again) * MaybeUninit Co-authored-by: Dominik Maier --- .github/workflows/build_and_test.yml | 7 ++++-- libafl/examples/llmp_test/main.rs | 5 +--- libafl/src/bolts/anymap.rs | 4 +-- libafl/src/bolts/fs.rs | 4 +-- libafl/src/bolts/llmp.rs | 21 ++++++---------- libafl/src/bolts/os/unix_shmem_server.rs | 7 ++---- libafl/src/bolts/tuples.rs | 2 +- libafl/src/executors/command.rs | 9 +++---- libafl/src/executors/forkserver.rs | 12 ++++----- libafl/src/mutators/token_mutations.rs | 10 ++------ libafl/src/observers/map.rs | 4 +-- libafl/src/schedulers/accounting.rs | 5 +--- libafl/src/schedulers/minimizer.rs | 5 +--- libafl_frida/src/alloc.rs | 12 ++++----- libafl_nyx/src/helper.rs | 18 +++++--------- libafl_qemu/build_linux.rs | 2 +- libafl_qemu/src/calls.rs | 2 +- libafl_qemu/src/emu.rs | 31 ++++++++++-------------- libafl_targets/src/coverage.rs | 4 --- libafl_targets/src/forkserver.rs | 2 +- libafl_targets/src/libfuzzer.rs | 4 +-- libafl_targets/src/sancov_8bit.rs | 3 --- 22 files changed, 64 insertions(+), 109 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 0fe350e86b..ef879fc800 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -20,6 +20,7 @@ jobs: with: profile: minimal toolchain: nightly + override: true - name: install mdbook uses: baptiste0928/cargo-install@v1.3.0 with: @@ -193,10 +194,12 @@ jobs: with: profile: minimal toolchain: nightly - - name: Add nightly rustfmt and clippy - run: rustup toolchain install nightly && rustup target add --toolchain nightly aarch64-unknown-none && rustup component add --toolchain nightly rust-src && rustup target add thumbv6m-none-eabi + override: true + components: rustfmt, clippy, rust-src - uses: actions/checkout@v3 - uses: Swatinem/rust-cache@v2 + - name: Add targets + run: rustup target add arm-linux-androideabi && rustup target add thumbv6m-none-eabi - name: Build aarch64-unknown-none run: cd ./fuzzers/baby_no_std && cargo +nightly build -Zbuild-std=core,alloc --target aarch64-unknown-none -v --release && cd ../.. - name: run x86_64 until panic! diff --git a/libafl/examples/llmp_test/main.rs b/libafl/examples/llmp_test/main.rs index 3506231159..48f29e6650 100644 --- a/libafl/examples/llmp_test/main.rs +++ b/libafl/examples/llmp_test/main.rs @@ -31,10 +31,7 @@ fn adder_loop(port: u16) -> ! { loop { let mut msg_counter = 0; loop { - let (sender, tag, buf) = match client.recv_buf().unwrap() { - None => break, - Some(msg) => msg, - }; + let Some((sender, tag, buf)) = client.recv_buf().unwrap() else { break }; msg_counter += 1; match tag { _TAG_SIMPLE_U32_V1 => { diff --git a/libafl/src/bolts/anymap.rs b/libafl/src/bolts/anymap.rs index 871f4d5f02..01c5c23a1e 100644 --- a/libafl/src/bolts/anymap.rs +++ b/libafl/src/bolts/anymap.rs @@ -41,7 +41,7 @@ macro_rules! impl_asany { /// Get a `type_id` from its previously unpacked `u64`. /// Opposite of [`unpack_type_id(id)`]. /// -/// # Safety +/// # Note /// Probably not safe for future compilers, fine for now. #[must_use] pub fn pack_type_id(id: u64) -> TypeId { @@ -52,7 +52,7 @@ pub fn pack_type_id(id: u64) -> TypeId { /// Unpack a `type_id` to an `u64` /// Opposite of [`pack_type_id(id)`]. /// -/// # Safety +/// # Note /// Probably not safe for future compilers, fine for now. #[must_use] pub fn unpack_type_id(id: TypeId) -> u64 { diff --git a/libafl/src/bolts/fs.rs b/libafl/src/bolts/fs.rs index 4b5e9b3a88..7ea4ad59bc 100644 --- a/libafl/src/bolts/fs.rs +++ b/libafl/src/bolts/fs.rs @@ -8,7 +8,7 @@ use core::cell::RefCell; use std::os::unix::prelude::{AsRawFd, RawFd}; use std::{ fs::{self, remove_file, File, OpenOptions}, - io::{Seek, SeekFrom, Write}, + io::{Seek, Write}, path::{Path, PathBuf}, }; @@ -124,7 +124,7 @@ impl InputFile { /// Rewinds the file to the beginning #[inline] pub fn rewind(&mut self) -> Result<(), Error> { - if let Err(err) = self.file.seek(SeekFrom::Start(0)) { + if let Err(err) = self.file.rewind() { Err(err.into()) } else { Ok(()) diff --git a/libafl/src/bolts/llmp.rs b/libafl/src/bolts/llmp.rs index 905e43d0b3..b1c25b2d75 100644 --- a/libafl/src/bolts/llmp.rs +++ b/libafl/src/bolts/llmp.rs @@ -2681,17 +2681,14 @@ where }; println!("Connected to port {port}"); - let broker_shmem_description = if let TcpResponse::BrokerConnectHello { + let TcpResponse::BrokerConnectHello { broker_shmem_description, hostname: _, - } = recv_tcp_msg(&mut stream)?.try_into()? - { - broker_shmem_description - } else { + } = recv_tcp_msg(&mut stream)?.try_into()? else { return Err(Error::illegal_state( "Received unexpected Broker Hello".to_string(), )); - }; + }; let map = LlmpSharedMap::existing( shmem_provider.shmem_from_description(broker_shmem_description)?, @@ -2706,15 +2703,11 @@ where send_tcp_msg(&mut stream, &client_hello_req)?; - let client_id = if let TcpResponse::LocalClientAccepted { client_id } = - recv_tcp_msg(&mut stream)?.try_into()? - { - client_id - } else { - return Err(Error::illegal_state( - "Unexpected Response from Broker".to_string(), + let TcpResponse::LocalClientAccepted { client_id } = recv_tcp_msg(&mut stream)?.try_into()? else { + return Err(Error::illegal_state( + "Unexpected Response from Broker".to_string(), )); - }; + }; // Set our ID to the one the broker sent us.. // This is mainly so we can filter out our own msgs later. diff --git a/libafl/src/bolts/os/unix_shmem_server.rs b/libafl/src/bolts/os/unix_shmem_server.rs index e7095a48b5..936322a088 100644 --- a/libafl/src/bolts/os/unix_shmem_server.rs +++ b/libafl/src/bolts/os/unix_shmem_server.rs @@ -348,12 +348,9 @@ impl Drop for ShMemServiceThread { fn drop(&mut self) { if self.join_handle.is_some() { println!("Stopping ShMemService"); - let mut stream = match UnixStream::connect_to_unix_addr( + let Ok(mut stream) = UnixStream::connect_to_unix_addr( &UnixSocketAddr::new(UNIX_SERVER_NAME).unwrap(), - ) { - Ok(stream) => stream, - Err(_) => return, // ignoring non-started server - }; + ) else { return }; let body = postcard::to_allocvec(&ServedShMemRequest::Exit).unwrap(); diff --git a/libafl/src/bolts/tuples.rs b/libafl/src/bolts/tuples.rs index fba72e7fad..7b4716edf3 100644 --- a/libafl/src/bolts/tuples.rs +++ b/libafl/src/bolts/tuples.rs @@ -234,7 +234,7 @@ where /// Match for a name and return the value /// -/// # Safety +/// # Note /// This operation is unsafe with Rust stable, wait for [specialization](https://stackoverflow.com/a/60138532/7658998). pub trait MatchName { /// Match for a name and return the borrowed value diff --git a/libafl/src/executors/command.rs b/libafl/src/executors/command.rs index bc5ff0cbe8..fdf977c9a3 100644 --- a/libafl/src/executors/command.rs +++ b/libafl/src/executors/command.rs @@ -535,13 +535,12 @@ impl CommandExecutorBuilder { OT: Debug + MatchName + ObserversTuple, S: UsesInput, { - let program = if let Some(program) = &self.program { - program - } else { - return Err(Error::illegal_argument( + let Some(program) = &self.program else { + return Err(Error::illegal_argument( "ComandExecutor::builder: no program set!", - )); + )); }; + let mut command = Command::new(program); match &self.input_location { InputLocation::StdIn => { diff --git a/libafl/src/executors/forkserver.rs b/libafl/src/executors/forkserver.rs index 0780bd0e07..a00001e5b8 100644 --- a/libafl/src/executors/forkserver.rs +++ b/libafl/src/executors/forkserver.rs @@ -328,15 +328,13 @@ impl Forkserver { /// Read a message from the child process. pub fn read_st_timed(&mut self, timeout: &TimeSpec) -> Result, Error> { let mut buf: [u8; 4] = [0_u8; 4]; - let st_read = match self.st_pipe.read_end() { - Some(fd) => fd, - None => { - return Err(Error::file(io::Error::new( - ErrorKind::BrokenPipe, + 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", ))); - } - }; + }; + let mut readfds = FdSet::new(); readfds.insert(st_read); // We'll pass a copied timeout to keep the original timeout intact, because select updates timeout to indicate how much time was left. See select(2) diff --git a/libafl/src/mutators/token_mutations.rs b/libafl/src/mutators/token_mutations.rs index 5aece3a54a..f93d2a7bb5 100644 --- a/libafl/src/mutators/token_mutations.rs +++ b/libafl/src/mutators/token_mutations.rs @@ -169,19 +169,13 @@ impl Tokens { if line.is_empty() || start == Some('#') { continue; } - let pos_quote = match line.find('\"') { - Some(x) => x, - None => return Err(Error::illegal_argument(format!("Illegal line: {line}"))), - }; + let Some(pos_quote) = line.find('\"') else { return Err(Error::illegal_argument(format!("Illegal line: {line}"))) }; if line.chars().nth(line.len() - 1) != Some('"') { return Err(Error::illegal_argument(format!("Illegal line: {line}"))); } // extract item - let item = match line.get(pos_quote + 1..line.len() - 1) { - Some(x) => x, - None => return Err(Error::illegal_argument(format!("Illegal line: {line}"))), - }; + let Some(item) = line.get(pos_quote + 1..line.len() - 1) else { return Err(Error::illegal_argument(format!("Illegal line: {line}"))) }; if item.is_empty() { continue; } diff --git a/libafl/src/observers/map.rs b/libafl/src/observers/map.rs index b7e99d61ed..1f9d7e372e 100644 --- a/libafl/src/observers/map.rs +++ b/libafl/src/observers/map.rs @@ -548,7 +548,7 @@ where /// Creates a new [`MapObserver`] from an [`OwnedSliceMut`] map. /// - /// # Safety + /// # Note /// Will dereference the owned slice with up to len elements. #[must_use] pub fn new_from_ownedref(name: S, map: OwnedSliceMut<'a, T>) -> Self @@ -594,7 +594,7 @@ where /// Creates a new [`MapObserver`] from an [`OwnedSliceMut`] map in differential mode. /// - /// # Safety + /// # Note /// Will dereference the owned slice with up to len elements. #[must_use] pub fn differential_from_ownedref(name: S, map: OwnedSliceMut<'a, T>) -> Self diff --git a/libafl/src/schedulers/accounting.rs b/libafl/src/schedulers/accounting.rs index 53a01aa6f1..fbd8cb8462 100644 --- a/libafl/src/schedulers/accounting.rs +++ b/libafl/src/schedulers/accounting.rs @@ -264,10 +264,7 @@ where /// Cull the `Corpus` #[allow(clippy::unused_self)] pub fn accounting_cull(&self, state: &mut CS::State) -> Result<(), Error> { - let top_rated = match state.metadata().get::() { - None => return Ok(()), - Some(val) => val, - }; + let Some(top_rated) = state.metadata().get::() else { return Ok(()) }; for (_key, idx) in &top_rated.map { let mut entry = state.corpus().get(*idx)?.borrow_mut(); diff --git a/libafl/src/schedulers/minimizer.rs b/libafl/src/schedulers/minimizer.rs index 2eacb50478..af4ada3c47 100644 --- a/libafl/src/schedulers/minimizer.rs +++ b/libafl/src/schedulers/minimizer.rs @@ -273,10 +273,7 @@ where /// Cull the `Corpus` using the `MinimizerScheduler` #[allow(clippy::unused_self)] pub fn cull(&self, state: &mut CS::State) -> Result<(), Error> { - let top_rated = match state.metadata().get::() { - None => return Ok(()), - Some(val) => val, - }; + let Some(top_rated) = state.metadata().get::() else { return Ok(()) }; let mut acc = HashSet::new(); diff --git a/libafl_frida/src/alloc.rs b/libafl_frida/src/alloc.rs index 5f67d8ae86..05554083e3 100644 --- a/libafl_frida/src/alloc.rs +++ b/libafl_frida/src/alloc.rs @@ -349,15 +349,13 @@ impl Allocator { #[allow(clippy::missing_safety_doc)] pub unsafe fn release(&mut self, ptr: *mut c_void) { //println!("freeing address: {:?}", ptr); - let mut metadata = if let Some(metadata) = self.allocations.get_mut(&(ptr as usize)) { - metadata - } else { + let Some(metadata) = self.allocations.get_mut(&(ptr as usize)) else { if !ptr.is_null() { - AsanErrors::get_mut() + AsanErrors::get_mut() .report_error(AsanError::UnallocatedFree((ptr as usize, Backtrace::new()))); - } - return; - }; + } + return; + }; if metadata.freed { AsanErrors::get_mut().report_error(AsanError::DoubleFree(( diff --git a/libafl_nyx/src/helper.rs b/libafl_nyx/src/helper.rs index 6a27cc61c8..e069569735 100644 --- a/libafl_nyx/src/helper.rs +++ b/libafl_nyx/src/helper.rs @@ -60,21 +60,15 @@ impl NyxHelper { parent_cpu_id: Option, initial_timeout: Duration, ) -> Result { - let sharedir = match target_dir.to_str() { - Some(x) => x, - None => return Err(Error::illegal_argument("can't convert sharedir to str")), - }; + let Some(sharedir) = target_dir.to_str() else { return Err(Error::illegal_argument("can't convert sharedir to str")) }; let work_dir = target_dir.join("workdir"); let work_dir = work_dir.to_str().expect("unable to convert workdir to str"); let nyx_type = if parallel_mode { - let parent_cpu_id = match parent_cpu_id { - None => { - return Err(Error::illegal_argument( - "please set parent_cpu_id in nyx parallel mode", - )) - } - Some(x) => x, - }; + let Some(parent_cpu_id) = parent_cpu_id else { + return Err(Error::illegal_argument( + "please set parent_cpu_id in nyx parallel mode", + )) + }; if cpu_id == parent_cpu_id { NyxProcessType::PARENT } else { diff --git a/libafl_qemu/build_linux.rs b/libafl_qemu/build_linux.rs index be3f79d1ac..72f5636d1e 100644 --- a/libafl_qemu/build_linux.rs +++ b/libafl_qemu/build_linux.rs @@ -4,7 +4,7 @@ use which::which; const QEMU_URL: &str = "https://github.com/AFLplusplus/qemu-libafl-bridge"; const QEMU_DIRNAME: &str = "qemu-libafl-bridge"; -const QEMU_REVISION: &str = "f26a5ca6137bb5d4d0dcfe5451fb16d4c0551c4e"; +const QEMU_REVISION: &str = "6db12fe4df0eb1305261cf07d1995f21eb262392"; fn build_dep_check(tools: &[&str]) { for tool in tools { diff --git a/libafl_qemu/src/calls.rs b/libafl_qemu/src/calls.rs index b29db4a1a6..61488d56bd 100644 --- a/libafl_qemu/src/calls.rs +++ b/libafl_qemu/src/calls.rs @@ -50,7 +50,7 @@ impl QemuHelper for QemuCallTracerHelper where S: UsesInput, { - fn init_hooks<'a, QT>(&self, hooks: &QemuHooks<'a, QT, S>) + fn init_hooks(&self, hooks: &QemuHooks<'_, QT, S>) where QT: QemuHelperTuple, { diff --git a/libafl_qemu/src/emu.rs b/libafl_qemu/src/emu.rs index 4c4c860186..280d0b4699 100644 --- a/libafl_qemu/src/emu.rs +++ b/libafl_qemu/src/emu.rs @@ -1,13 +1,14 @@ //! Expose QEMU user `LibAFL` C api to Rust -#[cfg(emulation_mode = "usermode")] use core::mem::MaybeUninit; use core::{ convert::Into, ffi::c_void, - ptr::{addr_of, addr_of_mut, copy_nonoverlapping, null}, + ptr::{addr_of, copy_nonoverlapping, null}, }; -use std::{ffi::CString, slice::from_raw_parts, str::from_utf8_unchecked}; +#[cfg(emulation_mode = "systemmode")] +use std::ffi::CString; +use std::{slice::from_raw_parts, str::from_utf8_unchecked}; #[cfg(emulation_mode = "usermode")] use libc::c_int; @@ -539,7 +540,6 @@ impl CPU { pub fn write_reg(&self, reg: R, val: T) -> Result<(), String> where - T: Num + PartialOrd + Copy, R: Into, { let reg = reg.into(); @@ -553,16 +553,17 @@ impl CPU { pub fn read_reg(&self, reg: R) -> Result where - T: Num + PartialOrd + Copy, R: Into, { - let reg = reg.into(); - let mut val = T::zero(); - let success = unsafe { libafl_qemu_read_reg(self.ptr, reg, addr_of_mut!(val) as *mut u8) }; - if success == 0 { - Err(format!("Failed to read register {reg}")) - } else { - Ok(val) + unsafe { + let reg = reg.into(); + let mut val = MaybeUninit::uninit(); + let success = libafl_qemu_read_reg(self.ptr, reg, val.as_mut_ptr() as *mut u8); + if success == 0 { + Err(format!("Failed to read register {reg}")) + } else { + Ok(val.assume_init()) + } } } @@ -794,12 +795,6 @@ impl Emulator { pub fn load_addr(&self) -> GuestAddr { unsafe { libafl_load_addr() as GuestAddr } } - #[cfg(emulation_mode = "systemmode")] - #[must_use] - pub fn load_addr(&self) -> GuestAddr { - // Only work if the binary is linked to the correct address and not pie - return 0x0 as GuestAddr; - } #[cfg(emulation_mode = "usermode")] #[must_use] diff --git a/libafl_targets/src/coverage.rs b/libafl_targets/src/coverage.rs index 25d7123625..9b69115286 100644 --- a/libafl_targets/src/coverage.rs +++ b/libafl_targets/src/coverage.rs @@ -37,10 +37,6 @@ pub use __afl_acc_memop_ptr as ACCOUNTING_MEMOP_MAP_PTR; pub use __afl_area_ptr as EDGES_MAP_PTR; /// Return Tokens from the compile-time token section -/// -/// # Safety -/// -/// This fn is safe to call, as long as the compilation did not break, previously #[cfg(any(target_os = "linux", target_vendor = "apple"))] pub fn autotokens() -> Result { unsafe { diff --git a/libafl_targets/src/forkserver.rs b/libafl_targets/src/forkserver.rs index 0eb40afb44..4519c66945 100644 --- a/libafl_targets/src/forkserver.rs +++ b/libafl_targets/src/forkserver.rs @@ -7,7 +7,7 @@ extern "C" { /// Start the forkserver from this point. Any shared memory must be created before. /// -/// # Safety +/// # Note /// /// The forkserver logic is written in C and this code is a wrapper. pub fn start_forkserver() -> ! { diff --git a/libafl_targets/src/libfuzzer.rs b/libafl_targets/src/libfuzzer.rs index d9d06300dc..db86929043 100644 --- a/libafl_targets/src/libfuzzer.rs +++ b/libafl_targets/src/libfuzzer.rs @@ -14,7 +14,7 @@ extern "C" { /// Calls the (native) libfuzzer initialize function. /// Returns the value returned by the init function. -/// # Safety +/// # Note /// Calls the libfuzzer-style init function which is native code. #[allow(clippy::similar_names)] #[allow(clippy::must_use_candidate)] // nobody uses that return code... @@ -31,7 +31,7 @@ pub fn libfuzzer_initialize(args: &[String]) -> i32 { } /// Call a single input of a libfuzzer-style cpp-harness -/// # Safety +/// # Note /// Calls the libfuzzer harness. We actually think the target is unsafe and crashes eventually, that's why we do all this fuzzing. #[allow(clippy::must_use_candidate)] pub fn libfuzzer_test_one_input(buf: &[u8]) -> i32 { diff --git a/libafl_targets/src/sancov_8bit.rs b/libafl_targets/src/sancov_8bit.rs index 1bf9714780..07d9519aa6 100644 --- a/libafl_targets/src/sancov_8bit.rs +++ b/libafl_targets/src/sancov_8bit.rs @@ -7,9 +7,6 @@ use core::slice::from_raw_parts_mut; pub static mut COUNTERS_MAPS: Vec<&'static mut [u8]> = Vec::new(); /// Initialize the sancov `8-bit-counters` - usually called by `llvm`. -/// -/// # Safety -/// Set up our coverage maps. #[no_mangle] #[allow(clippy::cast_sign_loss)] #[allow(clippy::not_unsafe_ptr_arg_deref)]