Skip to content

Commit

Permalink
mark fork as unsafe
Browse files Browse the repository at this point in the history
Reasoning: After a fork() in a multithreaded program, only async-signal-safe functions may be called by the child.
Calling everything else (including malloc(), mutexes) is undefined behavior.
  • Loading branch information
jorickert committed Apr 23, 2019
1 parent d8f3f74 commit a3136ac
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 13 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ 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
([#1020](https://github.com/nix-rust/nix/pull/1020))
### Fixed
- Fixed multiple bugs when using `sendmsg` and `recvmsg` with ancillary control messages
([#1020](https://github.com/nix-rust/nix/pull/1020))
Expand Down
6 changes: 3 additions & 3 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl ForkResult {
/// ```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);
/// }
Expand Down Expand Up @@ -222,9 +222,9 @@ impl ForkResult {
///
/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.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,
Expand Down
2 changes: 1 addition & 1 deletion test/sys/test_ptrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/sys/test_uio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test/sys/test_wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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)));
Expand All @@ -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();
Expand Down Expand Up @@ -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),
}
Expand Down
6 changes: 3 additions & 3 deletions test/test_unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit a3136ac

Please sign in to comment.