mark fork as unsafe

CHANGELOG.md

@@ -41,7 +41,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
   ([#1020](https://github.com/nix-rust/nix/pull/1020))
 - Replaced `CmsgSpace` with the `cmsg_space` macro.
   ([#1020](https://github.com/nix-rust/nix/pull/1020))
-
+- Marked `fork` as unsafe function
+  ([#1047](https://github.com/nix-rust/nix/pull/1047))
+
 ### Fixed
 - Fixed multiple bugs when using `sendmsg` and `recvmsg` with ancillary control messages
   ([#1020](https://github.com/nix-rust/nix/pull/1020))

src/unistd.rs

@@ -185,14 +185,19 @@ impl ForkResult {
 /// Create a new child process duplicating the parent process ([see
 /// fork(2)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html)).
 ///
-/// After calling the fork system call (successfully) two processes will
-/// be created that are identical with the exception of their pid and the
-/// return value of this function.  As an example:
+/// After calling the fork system call (successfully) a new process (child) will
+/// be created that is a copy of the calling process (parent). If the parent process is
+/// multithreaded, only the calling thread will be cloned.
+/// The child process differs in a few aspects from the parent process, most notably the pid and the
+/// return value of this function. For a list of all differences in the child and parent
+/// see the [man page].
+///
+/// As an example:
 ///
 /// ```no_run
 /// use nix::unistd::{fork, ForkResult};
 ///
-/// match fork() {
+/// match unsafe {fork()} {
 ///    Ok(ForkResult::Parent { child, .. }) => {
 ///        println!("Continuing execution in parent process, new child has pid: {}", child);
 ///    }
@@ -210,21 +215,26 @@ impl ForkResult {
 /// I'm a new child process
 /// ```
 ///
+///
 /// # Safety
+/// Make sure to read the [man page].
+/// Fork has some properties that can lead to unintended behavior.
+/// Especially the handling of mutexes and open files can be tricky.
 ///
 /// 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
+/// and `_exit` are safe to call by the child until it calls `execve`(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
+/// [man page]: http://man7.org/linux/man-pages/man2/fork.2.html
 #[inline]
-pub fn fork() -> Result<ForkResult> {
+pub unsafe fn fork() -> Result<ForkResult> {
     use self::ForkResult::*;
-    let res = unsafe { libc::fork() };
+    let res = libc::fork();
 
     Errno::result(res).map(|res| match res {
         0 => Child,

test/sys/test_ptrace.rs

@@ -74,7 +74,7 @@ fn test_ptrace_cont() {
         return;
     }
 
-    match fork().expect("Error: Fork Failed") {
+    match unsafe {fork()}.expect("Error: Fork Failed") {
         Child => {
             ptrace::traceme().unwrap();
             // As recommended by ptrace(2), raise SIGTRAP to pause the child

test/sys/test_uio.rs

@@ -207,7 +207,7 @@ fn test_process_vm_readv() {
     let mut vector = vec![1u8, 2, 3, 4, 5];
 
     let (r, w) = pipe().unwrap();
-    match fork().expect("Error: Fork Failed") {
+    match unsafe {fork()}.expect("Error: Fork Failed") {
         Parent { child } => {
             close(w).unwrap();
             // wait for child

test/sys/test_wait.rs

@@ -10,7 +10,7 @@ fn test_wait_signal() {
     let _ = ::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().expect("Error: Fork Failed") {
+    match unsafe {fork().expect("Error: Fork Failed")} {
       Child => {
           pause();
           unsafe { _exit(123) }
@@ -27,7 +27,7 @@ fn test_wait_exit() {
     let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is async-signal-safe.
-    match fork().expect("Error: Fork Failed") {
+    match unsafe { fork()}.expect("Error: Fork Failed") {
       Child => unsafe { _exit(12); },
       Parent { child } => {
           assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 12)));
@@ -47,7 +47,7 @@ fn test_waitstatus_from_raw() {
 fn test_waitstatus_pid() {
     let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
-    match fork().unwrap() {
+    match unsafe {fork()}.unwrap() {
         Child => unsafe { _exit(0) },
         Parent { child } => {
             let status = waitpid(child, None).unwrap();
@@ -96,7 +96,7 @@ mod ptrace {
     fn test_wait_ptrace() {
         let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
-        match fork().expect("Error: Fork Failed") {
+        match unsafe {fork()}.expect("Error: Fork Failed") {
             Child => ptrace_child(),
             Parent { child } => ptrace_parent(child),
         }

test/test_unistd.rs

@@ -19,7 +19,7 @@ fn test_fork_and_waitpid() {
     let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is signal-safe
-    match fork().expect("Error: Fork Failed") {
+    match unsafe {fork()}.expect("Error: Fork Failed") {
         Child => unsafe { _exit(0) },
         Parent { child } => {
             // assert that child was created and pid > 0
@@ -47,7 +47,7 @@ fn test_wait() {
     let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is signal-safe
-    match fork().expect("Error: Fork Failed") {
+    match unsafe {fork()}.expect("Error: Fork Failed") {
         Child => unsafe { _exit(0) },
         Parent { child } => {
             let wait_status = wait();
@@ -191,7 +191,7 @@ macro_rules! execve_test_factory(
         // 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() {
+        match unsafe {fork()}.unwrap() {
             Child => {
                 // Close stdout.
                 close(1).unwrap();