Skip to content

Commit

Permalink
Make clone unsafe (#1993)
Browse files Browse the repository at this point in the history
* Make `clone` unsafe

There are many features of `clone` that may cause memory unsafety when
called. This documents one of them and references `fork()`, which is
already unsafe to call.

* Adjust the clone entry in the CHANGELOG

---------

Co-authored-by: Alan Somers <asomers@gmail.com>
  • Loading branch information
djkoloski and asomers authored Aug 27, 2023
1 parent 93d43a7 commit 56e1277
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 15 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
See ([#2097](https://github.com/nix-rust/nix/pull/2097))
- Refactored `name` parameter of `mq_open` and `mq_unlink` to be generic over
`NixPath`. See ([#2102](https://github.com/nix-rust/nix/pull/2102)).
- Made `clone` unsafe, like `fork`.
([#1993](https://github.com/nix-rust/nix/pull/1993))

### Fixed
- Fix: send `ETH_P_ALL` in htons format
Expand Down
38 changes: 23 additions & 15 deletions src/sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,17 @@ mod sched_linux_like {
/// address need not be the highest address of the region. Nix will take
/// care of that requirement. The user only needs to provide a reference to
/// a normally allocated buffer.
pub fn clone(
///
/// # Safety
///
/// Because `clone` creates a child process with its stack located in
/// `stack` without specifying the size of the stack, special care must be
/// taken to ensure that the child process does not overflow the provided
/// stack space.
///
/// See [`fork`](crate::unistd::fork) for additional safety concerns related
/// to executing child processes.
pub unsafe fn clone(
mut cb: CloneCb,
stack: &mut [u8],
flags: CloneFlags,
Expand All @@ -106,20 +116,18 @@ mod sched_linux_like {
(*cb)() as c_int
}

let res = unsafe {
let combined = flags.bits() | signal.unwrap_or(0);
let ptr = stack.as_mut_ptr().add(stack.len());
let ptr_aligned = ptr.sub(ptr as usize % 16);
libc::clone(
mem::transmute(
callback
as extern "C" fn(*mut Box<dyn FnMut() -> isize>) -> i32,
),
ptr_aligned as *mut c_void,
combined,
&mut cb as *mut _ as *mut c_void,
)
};
let combined = flags.bits() | signal.unwrap_or(0);
let ptr = stack.as_mut_ptr().add(stack.len());
let ptr_aligned = ptr.sub(ptr as usize % 16);
let res = libc::clone(
mem::transmute(
callback
as extern "C" fn(*mut Box<dyn FnMut() -> isize>) -> i32,
),
ptr_aligned as *mut c_void,
combined,
&mut cb as *mut _ as *mut c_void,
);

Errno::result(res).map(Pid::from_raw)
}
Expand Down

0 comments on commit 56e1277

Please sign in to comment.