removed unsafe codes on runc client and shim

Signed-off-by: Yuna Tomida <ytomida.mmm@gmail.com>

crates/runc/src/io.rs

@@ -19,7 +19,7 @@ use std::io::Result;
 #[cfg(not(feature = "async"))]
 use std::io::{Read, Write};
 use std::os::unix::fs::OpenOptionsExt;
-use std::os::unix::io::{AsRawFd, FromRawFd};
+use std::os::unix::io::AsRawFd;
 use std::process::Stdio;
 use std::sync::Mutex;
 
@@ -253,12 +253,8 @@ pub struct NullIo {
 
 impl NullIo {
     pub fn new() -> std::io::Result<Self> {
-        let fd = nix::fcntl::open(
+        let f = OpenOptions::new().read(true).open("/dev/null")?;
-            "/dev/null",
+        let dev_null = Mutex::new(Some(f));
-            nix::fcntl::OFlag::O_RDONLY,
-            nix::sys::stat::Mode::empty(),
-        )?;
-        let dev_null = unsafe { Mutex::new(Some(std::fs::File::from_raw_fd(fd))) };
         Ok(Self { dev_null })
     }
 }

crates/shim/src/asynchronous/mod.rs

@@ -14,6 +14,7 @@
    limitations under the License.
 */
 
+use std::convert::TryFrom;
 use std::os::unix::fs::FileTypeExt;
 use std::os::unix::io::AsRawFd;
 use std::os::unix::net::UnixListener;
@@ -27,8 +28,12 @@ use std::{env, process};
 use async_trait::async_trait;
 use command_fds::{CommandFdExt, FdMapping};
 use futures::StreamExt;
-use libc::{c_int, pid_t, SIGCHLD, SIGINT, SIGPIPE, SIGTERM};
+use libc::{SIGCHLD, SIGINT, SIGPIPE, SIGTERM};
 use log::{debug, error, info, warn};
+use nix::errno::Errno;
+use nix::sys::signal::Signal;
+use nix::sys::wait::{self, WaitPidFlag, WaitStatus};
+use nix::unistd::Pid;
 use signal_hook_tokio::Signals;
 use tokio::io::AsyncWriteExt;
 use tokio::sync::Notify;
@@ -304,28 +309,43 @@ async fn handle_signals(signals: Signals) {
                 debug!("received {}", sig);
             }
             SIGCHLD => loop {
-                let result = asyncify(move || -> Result<(pid_t, c_int)> {
+                // Note: see comment at the counterpart in synchronous/mod.rs for details.
-                    let mut status: c_int = -1;
+                match asyncify(move || {
-                    let pid = unsafe { libc::waitpid(-1, &mut status, libc::WNOHANG) };
+                    Ok(wait::waitpid(
-                    if pid <= 0 {
+                        Some(Pid::from_raw(-1)),
-                        return Err(other!("wait finished"));
+                        Some(WaitPidFlag::WNOHANG),
-                    }
+                    )?)
-                    let status = libc::WEXITSTATUS(status);
-                    Ok((pid, status))
                 })
-                .await;
+                .await
-
+                {
-                if let Ok((res_pid, status)) = result {
+                    Ok(WaitStatus::Exited(pid, status)) => {
-                    monitor_notify_by_pid(res_pid, status)
+                        monitor_notify_by_pid(pid.as_raw(), status)
-                        .await
+                            .await
-                        .unwrap_or_else(|e| {
+                            .unwrap_or_else(|e| error!("failed to send exit event {}", e))
-                            error!("failed to send pid exit event {}", e);
+                    }
-                        })
+                    Ok(WaitStatus::Signaled(pid, sig, _)) => {
-                } else {
+                        debug!("child {} terminated({})", pid, sig);
-                    break;
+                        let exit_code = 128 + sig as i32;
+                        monitor_notify_by_pid(pid.as_raw(), exit_code)
+                            .await
+                            .unwrap_or_else(|e| error!("failed to send signal event {}", e))
+                    }
+                    Err(Error::Nix(Errno::ECHILD)) => {
+                        break;
+                    }
+                    Err(e) => {
+                        warn!("error occurred in signal handler: {}", e);
+                    }
+                    _ => {}
                 }
             },
-            _ => {}
+            _ => {
+                if let Ok(sig) = Signal::try_from(sig) {
+                    debug!("received {}", sig);
+                } else {
+                    warn!("received invalid signal {}", sig);
+                }
+            }
         }
     }
 }

crates/shim/src/synchronous/mod.rs

@@ -32,6 +32,7 @@
 //! ```
 //!
 
+use std::convert::TryFrom;
 use std::env;
 use std::fs;
 use std::io::Write;
@@ -42,8 +43,12 @@ use std::process::{self, Command, Stdio};
 use std::sync::{Arc, Condvar, Mutex};
 
 use command_fds::{CommandFdExt, FdMapping};
-use libc::{c_int, pid_t, SIGCHLD, SIGINT, SIGPIPE, SIGTERM};
+use libc::{SIGCHLD, SIGINT, SIGPIPE, SIGTERM};
 pub use log::{debug, error, info, warn};
+use nix::errno::Errno;
+use nix::sys::signal::Signal;
+use nix::sys::wait::{self, WaitPidFlag, WaitStatus};
+use nix::unistd::Pid;
 use signal_hook::iterator::Signals;
 
 use crate::protos::protobuf::Message;
@@ -230,23 +235,34 @@ fn handle_signals(mut signals: Signals) {
                     debug!("received {}", sig);
                 }
                 SIGCHLD => loop {
-                    unsafe {
+                    // Note that this thread sticks to child even it is suspended.
-                        let pid: pid_t = -1;
+                    match wait::waitpid(Some(Pid::from_raw(-1)), Some(WaitPidFlag::WNOHANG)) {
-                        let mut status: c_int = 0;
+                        Ok(WaitStatus::Exited(pid, status)) => {
-                        let options: c_int = libc::WNOHANG;
+                            monitor::monitor_notify_by_pid(pid.as_raw(), status)
-                        let res_pid = libc::waitpid(pid, &mut status, options);
+                                .unwrap_or_else(|e| error!("failed to send exit event {}", e))
-                        let status = libc::WEXITSTATUS(status);
-                        if res_pid <= 0 {
-                            break;
-                        } else {
-                            monitor::monitor_notify_by_pid(res_pid, status).unwrap_or_else(|e| {
-                                error!("failed to send exit event {}", e);
-                            });
                         }
+                        Ok(WaitStatus::Signaled(pid, sig, _)) => {
+                            debug!("child {} terminated({})", pid, sig);
+                            let exit_code = 128 + sig as i32;
+                            monitor::monitor_notify_by_pid(pid.as_raw(), exit_code)
+                                .unwrap_or_else(|e| error!("failed to send signal event {}", e))
+                        }
+                        Err(Errno::ECHILD) => {
+                            break;
+                        }
+                        Err(e) => {
+                            // stick until all children will be successfully waited, even some unexpected error occurs.
+                            warn!("error occurred in signal handler: {}", e);
+                        }
+                        _ => {} // stick until exit
                     }
                 },
                 _ => {
-                    debug!("received {}", sig);
+                    if let Ok(sig) = Signal::try_from(sig) {
+                        debug!("received {}", sig);
+                    } else {
+                        warn!("received invalid signal {}", sig);
+                    }
                 }
             }
         }