From 619fd96868b2cc83b7f205ff11ff897aebb5ef93 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Sat, 6 Feb 2021 14:15:49 +0100 Subject: [PATCH] Add PidFd type and seal traits Improve docs Split do_fork into two Make do_fork unsafe Add target attribute to create_pidfd field in Command Add method to get create_pidfd value --- library/std/src/os/linux/process.rs | 152 ++++++++++--- .../src/sys/unix/process/process_common.rs | 43 +++- .../std/src/sys/unix/process/process_unix.rs | 208 ++++++++++-------- src/test/ui/command/command-create-pidfd.rs | 6 +- 4 files changed, 289 insertions(+), 120 deletions(-) diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index 661d3cef7a0..435c0227c7e 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -2,40 +2,142 @@ #![unstable(feature = "linux_pidfd", issue = "none")] -use crate::process; -use crate::sys_common::AsInnerMut; use crate::io::Result; +use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use crate::process; +use crate::sys::fd::FileDesc; +use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; -/// Os-specific extensions to [`process::Child`] +/// This type represents a file descriptor that refers to a process. /// -/// [`process::Child`]: crate::process::Child -pub trait ChildExt { - /// Obtains the pidfd created for this child process, if available. - /// - /// A pidfd will only ever be available if `create_pidfd(true)` was called - /// when the corresponding `Command` was created. - /// - /// Even if `create_pidfd(true)` is called, a pidfd may not be available - /// due to an older version of Linux being in use, or if - /// some other error occured. - /// - /// See `man pidfd_open` for more details about pidfds. - fn pidfd(&self) -> Result; +/// A `PidFd` can be obtained by setting the corresponding option on [`Command`] +/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved +/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`]. +/// +/// Example: +/// ```no_run +/// #![feature(linux_pidfd)] +/// use std::os::linux::process::{CommandExt, ChildExt}; +/// use std::process::Command; +/// +/// let mut child = Command::new("echo") +/// .create_pidfd(true) +/// .spawn() +/// .expect("Failed to spawn child"); +/// +/// let pidfd = child +/// .take_pidfd() +/// .expect("Failed to retrieve pidfd"); +/// +/// // The file descriptor will be closed when `pidfd` is dropped. +/// ``` +/// Refer to the man page of `pidfd_open(2)` for further details. +/// +/// [`Command`]: process::Command +/// [`create_pidfd`]: CommandExt::create_pidfd +/// [`Child`]: process::Child +/// [`pidfd`]: fn@ChildExt::pidfd +/// [`take_pidfd`]: ChildExt::take_pidfd +#[derive(Debug)] +pub struct PidFd { + inner: FileDesc, } -/// Os-specific extensions to [`process::Command`] +impl AsInner for PidFd { + fn as_inner(&self) -> &FileDesc { + &self.inner + } +} + +impl FromInner for PidFd { + fn from_inner(inner: FileDesc) -> PidFd { + PidFd { inner } + } +} + +impl IntoInner for PidFd { + fn into_inner(self) -> FileDesc { + self.inner + } +} + +impl AsRawFd for PidFd { + fn as_raw_fd(&self) -> RawFd { + self.as_inner().raw() + } +} + +impl FromRawFd for PidFd { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + Self::from_inner(FileDesc::new(fd)) + } +} + +impl IntoRawFd for PidFd { + fn into_raw_fd(self) -> RawFd { + self.into_inner().into_raw() + } +} + +mod private_child_ext { + pub trait Sealed {} + impl Sealed for crate::process::Child {} +} + +/// Os-specific extensions for [`Child`] /// -/// [`process::Command`]: crate::process::Command -pub trait CommandExt { - /// Sets whether or this `Command` will attempt to create a pidfd - /// for the child. If this method is never called, a pidfd will - /// not be crated. +/// [`Child`]: process::Child +pub trait ChildExt: private_child_ext::Sealed { + /// Obtains a reference to the [`PidFd`] created for this [`Child`], if available. /// - /// The pidfd can be retrieved from the child via [`ChildExt::pidfd`] + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. + /// + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn pidfd(&self) -> Result<&PidFd>; + + /// Takes ownership of the [`PidFd`] created for this [`Child`], if available. + /// + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. + /// + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn take_pidfd(&mut self) -> Result; +} + +mod private_command_ext { + pub trait Sealed {} + impl Sealed for crate::process::Command {} +} + +/// Os-specific extensions for [`Command`] +/// +/// [`Command`]: process::Command +pub trait CommandExt: private_command_ext::Sealed { + /// Sets whether a [`PidFd`](struct@PidFd) should be created for the [`Child`] + /// spawned by this [`Command`]. + /// By default, no pidfd will be created. + /// + /// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`]. /// /// A pidfd will only be created if it is possible to do so - /// in a guaranteed race-free manner (e.g. if the `clone3` system call is - /// supported). Otherwise, [`ChildExit::pidfd`] will return an error. + /// in a guaranteed race-free manner (e.g. if the `clone3` system call + /// is supported). Otherwise, [`pidfd`] will return an error. + /// + /// [`Command`]: process::Command + /// [`Child`]: process::Child + /// [`pidfd`]: fn@ChildExt::pidfd + /// [`take_pidfd`]: ChildExt::take_pidfd fn create_pidfd(&mut self, val: bool) -> &mut process::Command; } diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index f7a7a9968b8..80cae5a3d79 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -79,7 +79,8 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, - pub(crate) make_pidfd: bool, + #[cfg(target_os = "linux")] + create_pidfd: bool, } // Create a new type for argv, so that we can make it `Send` and `Sync` @@ -125,6 +126,7 @@ pub enum Stdio { } impl Command { + #[cfg(not(target_os = "linux"))] pub fn new(program: &OsStr) -> Command { let mut saw_nul = false; let program = os2c(program, &mut saw_nul); @@ -142,7 +144,28 @@ impl Command { stdin: None, stdout: None, stderr: None, - make_pidfd: false, + } + } + + #[cfg(target_os = "linux")] + pub fn new(program: &OsStr) -> Command { + let mut saw_nul = false; + let program = os2c(program, &mut saw_nul); + Command { + argv: Argv(vec![program.as_ptr(), ptr::null()]), + args: vec![program.clone()], + program, + env: Default::default(), + cwd: None, + uid: None, + gid: None, + saw_nul, + closures: Vec::new(), + groups: None, + stdin: None, + stdout: None, + stderr: None, + create_pidfd: false, } } @@ -178,9 +201,21 @@ impl Command { pub fn groups(&mut self, groups: &[gid_t]) { self.groups = Some(Box::from(groups)); } - + + #[cfg(target_os = "linux")] pub fn create_pidfd(&mut self, val: bool) { - self.make_pidfd = val; + self.create_pidfd = val; + } + + #[cfg(not(target_os = "linux"))] + #[allow(dead_code)] + pub fn get_create_pidfd(&self) -> bool { + false + } + + #[cfg(target_os = "linux")] + pub fn get_create_pidfd(&self) -> bool { + self.create_pidfd } pub fn saw_nul(&self) -> bool { diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 674c694e4fc..f7112a73efa 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -8,7 +8,9 @@ use crate::ptr; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; -use crate::sync::atomic::{AtomicBool, Ordering}; + +#[cfg(target_os = "linux")] +use crate::os::linux::process::PidFd; #[cfg(target_os = "linux")] use crate::sys::weak::syscall; @@ -66,7 +68,7 @@ impl Command { // in its own process. Thus the parent drops the lock guard while the child // forgets it to avoid unlocking it on a new thread, which would be invalid. let env_lock = sys::os::env_read_lock(); - let (pid, pidfd) = self.do_fork()?; + let (pid, pidfd) = unsafe { self.do_fork()? }; if pid == 0 { crate::panic::always_abort(); @@ -95,7 +97,7 @@ impl Command { drop(env_lock); drop(output); - let mut p = Process { pid, status: None, pidfd }; + let mut p = Process::new(pid, pidfd); let mut bytes = [0; 8]; // loop to handle EINTR @@ -127,84 +129,91 @@ impl Command { } } - // Attempts to fork the process. If successful, returns - // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. - fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> { - // If we fail to create a pidfd for any reason, this will - // stay as -1, which indicates an error - let mut pidfd: libc::pid_t = -1; - - // On Linux, attempt to use the `clone3` syscall, which - // supports more argument (in particular, the ability to create a pidfd). - // If this fails, we will fall through this block to a call to `fork()` - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - static HAS_CLONE3: AtomicBool = AtomicBool::new(true); - - const CLONE_PIDFD: u64 = 0x00001000; - - #[repr(C)] - struct clone_args { - flags: u64, - pidfd: u64, - child_tid: u64, - parent_tid: u64, - exit_signal: u64, - stack: u64, - stack_size: u64, - tls: u64, - set_tid: u64, - set_tid_size: u64, - cgroup: u64, - } - - syscall! { - fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long - } - - if HAS_CLONE3.load(Ordering::Relaxed) { - let mut flags = 0; - if self.make_pidfd { - flags |= CLONE_PIDFD; - } - - let mut args = clone_args { - flags, - pidfd: &mut pidfd as *mut libc::pid_t as u64, - child_tid: 0, - parent_tid: 0, - exit_signal: libc::SIGCHLD as u64, - stack: 0, - stack_size: 0, - tls: 0, - set_tid: 0, - set_tid_size: 0, - cgroup: 0 - }; - - let args_ptr = &mut args as *mut clone_args; - let args_size = crate::mem::size_of::(); - - let res = cvt(unsafe { clone3(args_ptr, args_size) }); - match res { - Ok(n) => return Ok((n, pidfd)), - Err(e) => match e.raw_os_error() { - // Multiple threads can race to execute this store, - // but that's fine - that just means that multiple threads - // will have tried and failed to execute the same syscall, - // with no other side effects. - Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), - _ => return Err(e) - } - } - } - } - } - // If we get here, we are either not on Linux, - // or we are on Linux and the 'clone3' syscall does not exist - cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd)) + // Attempts to fork the process. If successful, returns Ok((0, -1)) + // in the child, and Ok((child_pid, -1)) in the parent. + #[cfg(not(target_os = "linux"))] + unsafe fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + cvt(libc::fork()).map(|res| (res, -1)) } + // Attempts to fork the process. If successful, returns Ok((0, -1)) + // in the child, and Ok((child_pid, child_pidfd)) in the parent. + #[cfg(target_os = "linux")] + unsafe fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + use crate::sync::atomic::{AtomicBool, Ordering}; + + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } + + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long + } + + // If we fail to create a pidfd for any reason, this will + // stay as -1, which indicates an error. + let mut pidfd: pid_t = -1; + + // Attempt to use the `clone3` syscall, which supports more arguments + // (in particular, the ability to create a pidfd). If this fails, + // we will fall through this block to a call to `fork()` + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.get_create_pidfd() { + flags |= CLONE_PIDFD; + } + + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0, + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(clone3(args_ptr, args_size)); + match res { + Ok(n) => return Ok((n as pid_t, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + // Fallback to fork if `EPERM` is returned. (e.g. blocked by seccomp) + Some(libc::EPERM) => {} + _ => return Err(e), + }, + } + } + + // If we get here, the 'clone3' syscall does not exist + // or we do not have permission to call it + cvt(libc::fork()).map(|res| (res, pidfd)) + } pub fn exec(&mut self, default: Stdio) -> io::Error { let envp = self.capture_env(); @@ -360,6 +369,8 @@ impl Command { #[cfg(not(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), )))] fn posix_spawn( &mut self, @@ -374,6 +385,8 @@ impl Command { #[cfg(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), ))] fn posix_spawn( &mut self, @@ -388,6 +401,7 @@ impl Command { || (self.env_saw_path() && !self.program_is_path()) || !self.get_closures().is_empty() || self.get_groups().is_some() + || self.get_create_pidfd() { return Ok(None); } @@ -432,7 +446,7 @@ impl Command { None => None, }; - let mut p = Process { pid: 0, status: None }; + let mut p = Process::new(0, -1); struct PosixSpawnFileActions<'a>(&'a mut MaybeUninit); @@ -522,14 +536,26 @@ pub struct Process { pid: pid_t, status: Option, // On Linux, stores the pidfd created for this child. - // This is -1 if the user did not request pidfd creation, + // This is None if the user did not request pidfd creation, // or if the pidfd could not be created for some reason // (e.g. the `clone3` syscall was not available). #[cfg(target_os = "linux")] - pidfd: libc::c_int, + pidfd: Option, } impl Process { + #[cfg(target_os = "linux")] + fn new(pid: pid_t, pidfd: pid_t) -> Self { + use crate::sys_common::FromInner; + let pidfd = (pidfd >= 0).then(|| PidFd::from_inner(sys::fd::FileDesc::new(pidfd))); + Process { pid, status: None, pidfd } + } + + #[cfg(not(target_os = "linux"))] + fn new(pid: pid_t, _pidfd: pid_t) -> Self { + Process { pid, status: None } + } + pub fn id(&self) -> u32 { self.pid as u32 } @@ -669,12 +695,18 @@ impl ExitStatusError { #[cfg(target_os = "linux")] #[unstable(feature = "linux_pidfd", issue = "none")] impl crate::os::linux::process::ChildExt for crate::process::Child { - fn pidfd(&self) -> crate::io::Result { - if self.handle.pidfd > 0 { - Ok(self.handle.pidfd) - } else { - Err(crate::io::Error::from(crate::io::ErrorKind::Other)) - } + fn pidfd(&self) -> io::Result<&PidFd> { + self.handle + .pidfd + .as_ref() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) + } + + fn take_pidfd(&mut self) -> io::Result { + self.handle + .pidfd + .take() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) } } diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs index 248ae3457d7..db728be09df 100644 --- a/src/test/ui/command/command-create-pidfd.rs +++ b/src/test/ui/command/command-create-pidfd.rs @@ -1,13 +1,13 @@ // run-pass -// linux-only - pidfds are a linux-specific concept +// only-linux - pidfds are a linux-specific concept #![feature(linux_pidfd)] use std::os::linux::process::{CommandExt, ChildExt}; use std::process::Command; fn main() { - // We don't assert the precise value, since the standard libarary - // may be opened other file descriptors before our code ran. + // We don't assert the precise value, since the standard library + // might have opened other file descriptors before our code runs. let _ = Command::new("echo") .create_pidfd(true) .spawn()