sys_util: add safe wrappers getpid,geteuid,getguid,waitpid,kill

These functions are trivially safe and by adding them to sys_util, we
can remove some unsafe blocks from crosvm. This CL also replaces the
unsafe call sites with the safe alternatives.

There are no previous usages of gete{g,u}id(2), but they will be needed
in a future change.

TEST=None
BUG=None

Change-Id: Ief8787b298cfaa5b7fd1b83f0eba6660369e687d
Reviewed-on: https://chromium-review.googlesource.com/634268
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

3 files changed, 95 insertions, 45 deletions

diff --git a/src/main.rs b/src/main.rs
index 9233629..5cc9532 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -31,22 +31,18 @@ use std::io::{stdin, stdout};
 use std::net;
 use std::os::unix::net::UnixDatagram;
 use std::path::{Path, PathBuf};
-use std::ptr;
 use std::string::String;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex, Barrier};
 use std::thread::{spawn, sleep, JoinHandle};
 use std::time::Duration;
 
-use libc::getpid;
-
 use clap::{Arg, App, SubCommand};
 
-
 use io_jail::Minijail;
 use kvm::*;
 use sys_util::{GuestAddress, GuestMemory, EventFd, TempDir, Terminal, Poller, Pollable, Scm,
-               register_signal_handler, Killable, SignalFd, syslog};
+               register_signal_handler, Killable, SignalFd, kill_process_group, reap_child, syslog};
 
 use device_manager::*;
 use vm_control::{VmRequest, VmResponse};
@@ -192,34 +188,21 @@ fn wait_all_children() -> bool {
     const CHILD_WAIT_MAX_ITER: isize = 10;
     const CHILD_WAIT_MS: u64 = 10;
     for _ in 0..CHILD_WAIT_MAX_ITER {
-        // waitpid() is safe when used in this manner; we will check
-        // without blocking if there are any child processes that
-        // are still running or need their exit statuses reaped.
         loop {
-            let ret = unsafe {
-                libc::waitpid(-1, ptr::null_mut(), libc::WNOHANG)
-            };
-            // waitpid() returns -1 when there are no children left, and
-            // returns 0 when there are children alive but not yet exited.
-            if ret < 0 {
-                let err = sys_util::Error::last().errno();
-                // We expect ECHILD which indicates that there were
-                // no children left.
-                if err == libc::ECHILD {
-                    return true;
-                }
-                else {
-                    warn!("waitpid returned {} while waiting for children",
-                          err);
+            match reap_child() {
+                Ok(0) => break,
+                // We expect ECHILD which indicates that there were no children left.
+                Err(e) if e.errno() == libc::ECHILD => return true,
+                Err(e) => {
+                    warn!("error while waiting for children: {:?}", e);
+                    return false;
                 }
-                return false;
-            } else if ret == 0 {
-                break;
+                // We reaped one child, so continue reaping.
+                _ => {},
             }
         }
-        // There's no timeout option for waitpid, so our only recourse
-        // is to use WNOHANG and sleep while waiting for the children
-        // to exit.
+        // There's no timeout option for waitpid which reap_child calls internally, so our only
+        // recourse is to sleep while waiting for the children to exit.
         sleep(Duration::from_millis(CHILD_WAIT_MS));
     }
 
@@ -241,9 +224,7 @@ fn run_config(cfg: Config) -> Result<()> {
     if let Some(ref path) = cfg.socket_path {
         let path = Path::new(path);
         let control_socket = if path.is_dir() {
-                // Getting the pid is always safe and never fails.
-                let pid = unsafe { getpid() };
-                UnixDatagram::bind(path.join(format!("crosvm-{}.sock", pid)))
+                UnixDatagram::bind(path.join(format!("crosvm-{}.sock", getpid())))
             } else {
                 UnixDatagram::bind(path)
             }
@@ -805,13 +786,10 @@ fn main() {
             // take some time for the processes to shut down.
             if !wait_all_children() {
                 // We gave them a chance, and it's too late.
-                // A pid of 0 will kill any processes left in our process group,
-                // which is safe for us to do (we spawned them).
                 warn!("not all child processes have exited; sending SIGKILL");
-                let ret = unsafe { libc::kill(0, libc::SIGKILL) };
-                if ret < 0 {
+                if let Err(e) = kill_process_group() {
                     // We're now at the mercy of the OS to clean up after us.
-                    warn!("unable to kill all child processes");
+                    warn!("unable to kill all child processes: {:?}", e);
                 }
             }
 
diff --git a/sys_util/src/fork.rs b/sys_util/src/fork.rs
index 7692959..72fb0f0 100644
--- a/sys_util/src/fork.rs
+++ b/sys_util/src/fork.rs
@@ -96,14 +96,7 @@ pub fn clone_process<F>(ns: CloneNamespace, post_clone_cb: F) -> result::Result<
 mod tests {
     use super::*;
     use libc;
-    use EventFd;
-    use syscall_defines::linux::LinuxSyscall::SYS_getpid;
-
-    // This bypasses libc's caching getpid() wrapper which is invalid because we do a raw clone
-    // syscall in this module.
-    fn getpid() -> pid_t {
-        unsafe { libc::syscall(SYS_getpid as i64) as i32 }
-    }
+    use {getpid, EventFd};
 
     fn wait_process() -> libc::c_int {
         let mut status: libc::c_int = 0;
diff --git a/sys_util/src/lib.rs b/sys_util/src/lib.rs
index 33f0b3e..3574f98 100644
--- a/sys_util/src/lib.rs
+++ b/sys_util/src/lib.rs
@@ -49,3 +49,82 @@ pub use sock_ctrl_msg::*;
 pub use mmap::Error as MmapError;
 pub use guest_memory::Error as GuestMemoryError;
 pub use signalfd::Error as SignalFdError;
+
+use std::ptr;
+
+use libc::{kill, syscall, waitpid, pid_t, uid_t, gid_t, SIGKILL, WNOHANG};
+
+use syscall_defines::linux::LinuxSyscall::SYS_getpid;
+
+/// This bypasses `libc`'s caching `getpid(2)` wrapper which can be invalid if a raw clone was used
+/// elsewhere.
+#[inline(always)]
+pub fn getpid() -> pid_t {
+    // Safe because this syscall can never fail and we give it a valid syscall number.
+    unsafe { syscall(SYS_getpid as i64) as pid_t }
+}
+
+/// Safe wrapper for `geteuid(2)`.
+#[inline(always)]
+pub fn geteuid() -> uid_t {
+    // trivially safe
+    unsafe { libc::geteuid() }
+}
+
+/// Safe wrapper for `getegid(2)`.
+#[inline(always)]
+pub fn getegid() -> gid_t {
+    // trivially safe
+    unsafe { libc::getegid() }
+}
+
+/// Reaps a child process that has terminated.
+///
+/// Returns `Ok(pid)` where `pid` is the process that was reaped or `Ok(0)` if none of the children
+/// have terminated. An `Error` is with `errno == ECHILD` if there are no children left to reap.
+///
+/// # Examples
+///
+/// Reaps all child processes until there are no terminated children to reap.
+///
+/// ```
+/// # extern crate libc;
+/// # extern crate sys_util;
+/// fn reap_children() {
+///     loop {
+///         match sys_util::reap_child() {
+///             Ok(0) => println!("no children ready to reap"),
+///             Ok(pid) => {
+///                 println!("reaped {}", pid);
+///                 continue
+///             },
+///             Err(e) if e.errno() == libc::ECHILD => println!("no children left"),
+///             Err(e) => println!("error reaping children: {:?}", e),
+///         }
+///         break
+///     }
+/// }
+/// ```
+pub fn reap_child() -> Result<pid_t> {
+    // Safe because we pass in no memory, prevent blocking with WNOHANG, and check for error.
+    let ret = unsafe { waitpid(-1, ptr::null_mut(), WNOHANG) };
+    if ret == -1 {
+        errno_result()
+    } else {
+        Ok(ret)
+    }
+}
+
+/// Kill all processes in the current process group.
+///
+/// On success, this kills all processes in the current process group, including the current
+/// process, meaning this will not return. This is equivalent to a call to `kill(0, SIGKILL)`.
+pub fn kill_process_group() -> Result<()> {
+    let ret = unsafe { kill(0, SIGKILL) };
+    if ret == -1 {
+        errno_result()
+    } else {
+        // Kill succeeded, so this process never reaches here.
+        unreachable!();
+    }
+}