diff --git a/src/unistd.rs b/src/unistd.rs index 25237a07ef..19f82ef6db 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -191,6 +191,18 @@ impl ForkResult { /// Continuing execution in parent process, new child has pid: 1234 /// I'm a new child process /// ``` +/// +/// # Safety +/// +/// In a multithreaded program, only [async-signal-safe] functions like `pause` +/// and `_exit` may be called by the child (the parent isn't restricted). Note +/// that memory allocation may **not** be async-signal-safe and thus must be +/// prevented. +/// +/// Those functions are only a small subset of your operating system's API, so +/// special care must be taken to only invoke code you can control and audit. +/// +/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.html #[inline] pub fn fork() -> Result<ForkResult> { use self::ForkResult::*; @@ -687,7 +699,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> { /// use nix::unistd::close; /// /// fn main() { -/// let mut f = tempfile::tempfile().unwrap(); +/// let f = tempfile::tempfile().unwrap(); /// close(f.as_raw_fd()).unwrap(); // Bad! f will also close on drop! /// } /// ``` @@ -700,7 +712,7 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> { /// use nix::unistd::close; /// /// fn main() { -/// let mut f = tempfile::tempfile().unwrap(); +/// let f = tempfile::tempfile().unwrap(); /// close(f.into_raw_fd()).unwrap(); // Good. into_raw_fd consumes f /// } /// ``` diff --git a/test/sys/test_wait.rs b/test/sys/test_wait.rs index 5f6c923184..210e6bc8ce 100644 --- a/test/sys/test_wait.rs +++ b/test/sys/test_wait.rs @@ -2,15 +2,16 @@ use nix::unistd::*; use nix::unistd::ForkResult::*; use nix::sys::signal::*; use nix::sys::wait::*; -use libc::exit; +use libc::_exit; #[test] fn test_wait_signal() { #[allow(unused_variables)] let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test"); + // Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe. match fork() { - Ok(Child) => pause().unwrap_or(()), + Ok(Child) => pause().unwrap_or_else(|_| unsafe { _exit(123) }), Ok(Parent { child }) => { kill(child, Some(SIGKILL)).ok().expect("Error: Kill Failed"); assert_eq!(waitpid(child, None), Ok(WaitStatus::Signaled(child, SIGKILL, false))); @@ -25,8 +26,9 @@ fn test_wait_exit() { #[allow(unused_variables)] let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test"); + // Safe: Child only calls `_exit`, which is async-signal-safe. match fork() { - Ok(Child) => unsafe { exit(12); }, + Ok(Child) => unsafe { _exit(12); }, Ok(Parent { child }) => { assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 12))); }, @@ -46,13 +48,14 @@ mod ptrace { use nix::unistd::*; use nix::unistd::ForkResult::*; use std::{ptr, process}; + use libc::_exit; fn ptrace_child() -> ! { let _ = ptrace(PTRACE_TRACEME, Pid::from_raw(0), ptr::null_mut(), ptr::null_mut()); // As recommended by ptrace(2), raise SIGTRAP to pause the child // until the parent is ready to continue let _ = raise(SIGTRAP); - process::exit(0) + unsafe { _exit(0) } } fn ptrace_parent(child: Pid) { diff --git a/test/test_unistd.rs b/test/test_unistd.rs index fd4e327b56..302c1cfaee 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -11,16 +11,16 @@ use std::io::Write; use std::os::unix::prelude::*; use tempfile::tempfile; use tempdir::TempDir; -use libc::off_t; -use std::process::exit; +use libc::{_exit, off_t}; #[test] fn test_fork_and_waitpid() { #[allow(unused_variables)] let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test"); - let pid = fork(); - match pid { - Ok(Child) => exit(0), + + // Safe: Child only calls `_exit`, which is signal-safe + match fork() { + Ok(Child) => unsafe { _exit(0) }, Ok(Parent { child }) => { // assert that child was created and pid > 0 let child_raw: ::libc::pid_t = child.into(); @@ -48,9 +48,11 @@ fn test_wait() { // Grab FORK_MTX so wait doesn't reap a different test's child process #[allow(unused_variables)] let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test"); + + // Safe: Child only calls `_exit`, which is signal-safe let pid = fork(); match pid { - Ok(Child) => exit(0), + Ok(Child) => unsafe { _exit(0) }, Ok(Parent { child }) => { let wait_status = wait(); @@ -111,6 +113,9 @@ macro_rules! execve_test_factory( // data from `reader`. let (reader, writer) = pipe().unwrap(); + // Safe: Child calls `exit`, `dup`, `close` and the provided `exec*` family function. + // NOTE: Technically, this makes the macro unsafe to use because you could pass anything. + // The tests make sure not to do that, though. match fork().unwrap() { Child => { #[cfg(not(target_os = "android"))]