From 10d7226b19345d6808d03ac34be3f82a85229a4d Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Wed, 8 Mar 2023 00:00:50 -0500 Subject: other: deny missing safety docs and add them (#1053) --- .../data_harvester/processes/macos/sysctl_bindings.rs | 7 ++++++- src/app/data_harvester/processes/unix.rs | 17 +++++++++-------- src/app/process_killer.rs | 3 +++ src/bin/main.rs | 1 + src/lib.rs | 1 + 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/app/data_harvester/processes/macos/sysctl_bindings.rs b/src/app/data_harvester/processes/macos/sysctl_bindings.rs index e56d79b8..420c6a2e 100644 --- a/src/app/data_harvester/processes/macos/sysctl_bindings.rs +++ b/src/app/data_harvester/processes/macos/sysctl_bindings.rs @@ -264,12 +264,13 @@ pub(crate) struct eproc { /// Obtains the [`kinfo_proc`] given a process PID. /// -/// From [heim](https://github.com/heim-rs/heim/blob/master/heim-process/src/sys/macos/bindings/process.rs#L235). +/// Based on the implementation from [heim](https://github.com/heim-rs/heim/blob/master/heim-process/src/sys/macos/bindings/process.rs#L235). pub(crate) fn kinfo_process(pid: Pid) -> Result { let mut name: [i32; 4] = [CTL_KERN, KERN_PROC, KERN_PROC_PID, pid]; let mut size = mem::size_of::(); let mut info = mem::MaybeUninit::::uninit(); + // SAFETY: libc binding, we assume all arguments are valid. let result = unsafe { libc::sysctl( name.as_mut_ptr(), @@ -290,6 +291,10 @@ pub(crate) fn kinfo_process(pid: Pid) -> Result { bail!("failed to get process for pid {pid}"); } + // SAFETY: info is initialized if result succeeded and returned a non-negative result. If sysctl failed, it returns + // -1 with errno set. + // + // Source: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/sysctl.3.html unsafe { Ok(info.assume_init()) } } diff --git a/src/app/data_harvester/processes/unix.rs b/src/app/data_harvester/processes/unix.rs index 75f70bec..3e459e06 100644 --- a/src/app/data_harvester/processes/unix.rs +++ b/src/app/data_harvester/processes/unix.rs @@ -18,15 +18,16 @@ impl UserTable { let passwd = unsafe { libc::getpwuid(uid) }; if passwd.is_null() { - return Err(error::BottomError::QueryError("Missing passwd".into())); - } - - let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) } - .to_str()? - .to_string(); - self.uid_user_mapping.insert(uid, username.clone()); + Err(error::BottomError::QueryError("Missing passwd".into())) + } else { + // SAFETY: We return early if passwd is null. + let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) } + .to_str()? + .to_string(); + self.uid_user_mapping.insert(uid, username.clone()); - Ok(username) + Ok(username) + } } } } diff --git a/src/app/process_killer.rs b/src/app/process_killer.rs index d93c6f07..cf23b076 100644 --- a/src/app/process_killer.rs +++ b/src/app/process_killer.rs @@ -19,6 +19,7 @@ struct Process(HANDLE); #[cfg(target_os = "windows")] impl Process { fn open(pid: u32) -> Result { + // SAFETY: Windows API call, tread carefully with the args. match unsafe { OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE, false, pid) } { Ok(process) => Ok(Process(process)), Err(_) => Err("process may have already been terminated.".to_string()), @@ -26,6 +27,7 @@ impl Process { } fn kill(self) -> Result<(), String> { + // SAFETY: Windows API call, tread carefully with the args. let result = unsafe { TerminateProcess(self.0, 1) }; if result.0 == 0 { return Err("process may have already been terminated.".to_string()); @@ -49,6 +51,7 @@ pub fn kill_process_given_pid(pid: Pid) -> crate::utils::error::Result<()> { /// Kills a process, given a PID, for unix. #[cfg(target_family = "unix")] pub fn kill_process_given_pid(pid: Pid, signal: usize) -> crate::utils::error::Result<()> { + // SAFETY: the signal should be valid, and we act properly on an error (exit code not 0). let output = unsafe { libc::kill(pid, signal as i32) }; if output != 0 { // We had an error... diff --git a/src/bin/main.rs b/src/bin/main.rs index e6f3bf64..697fc27f 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -1,5 +1,6 @@ #![warn(rust_2018_idioms)] #![allow(clippy::uninlined_format_args)] +#![deny(clippy::missing_safety_doc)] #[allow(unused_imports)] #[cfg(feature = "log")] #[macro_use] diff --git a/src/lib.rs b/src/lib.rs index 4fcb57da..4fdf3fb6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,7 @@ #![warn(rust_2018_idioms)] #![allow(clippy::uninlined_format_args)] +#![deny(clippy::missing_safety_doc)] #[allow(unused_imports)] #[cfg(feature = "log")] #[macro_use] -- cgit v1.2.3