to extract a pidfd we must consume the child

As long as a pidfd is on a child it can be safely reaped. Taking it
would mean the child would now have to be awaited through its pid, but could also
be awaited through the pidfd. This could then suffer from a recycling race.
This commit is contained in:
The 8472 2024-06-13 23:55:03 +02:00
parent 0787c7308c
commit 8abf149bde
3 changed files with 19 additions and 12 deletions

View file

@ -19,7 +19,7 @@ struct InnerPidFd;
///
/// 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`].
/// from the [`Child`] by calling [`pidfd`] or [`into_pidfd`].
///
/// Example:
/// ```no_run
@ -33,7 +33,7 @@ struct InnerPidFd;
/// .expect("Failed to spawn child");
///
/// let pidfd = child
/// .take_pidfd()
/// .into_pidfd()
/// .expect("Failed to retrieve pidfd");
///
/// // The file descriptor will be closed when `pidfd` is dropped.
@ -44,7 +44,7 @@ struct InnerPidFd;
/// [`create_pidfd`]: CommandExt::create_pidfd
/// [`Child`]: process::Child
/// [`pidfd`]: fn@ChildExt::pidfd
/// [`take_pidfd`]: ChildExt::take_pidfd
/// [`into_pidfd`]: ChildExt::into_pidfd
/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html
#[derive(Debug)]
#[repr(transparent)]
@ -159,18 +159,26 @@ pub trait ChildExt: Sealed {
/// [`Child`]: process::Child
fn pidfd(&self) -> Result<&PidFd>;
/// Takes ownership of the [`PidFd`] created for this [`Child`], if available.
/// Returns the [`PidFd`] created for this [`Child`], if available.
/// Otherwise self is returned.
///
/// A pidfd will only be available if its creation was requested with
/// [`create_pidfd`] when the corresponding [`Command`] was created.
///
/// Taking ownership of the PidFd consumes the Child to avoid pid reuse
/// races. Use [`pidfd`] and [`BorrowedFd::try_clone_to_owned`] if
/// you don't want to disassemble the Child yet.
///
/// 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
/// [`pidfd`]: ChildExt::pidfd
/// [`Child`]: process::Child
fn take_pidfd(&mut self) -> Result<PidFd>;
fn into_pidfd(self) -> crate::result::Result<PidFd, Self>
where
Self: Sized;
}
/// Os-specific extensions for [`Command`]
@ -181,7 +189,7 @@ pub trait CommandExt: Sealed {
/// spawned by this [`Command`].
/// By default, no pidfd will be created.
///
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
/// The pidfd can be retrieved from the child with [`pidfd`] or [`into_pidfd`].
///
/// A pidfd will only be created if it is possible to do so
/// in a guaranteed race-free manner. Otherwise, [`pidfd`] will return an error.
@ -195,7 +203,7 @@ pub trait CommandExt: Sealed {
/// [`Command`]: process::Command
/// [`Child`]: process::Child
/// [`pidfd`]: fn@ChildExt::pidfd
/// [`take_pidfd`]: ChildExt::take_pidfd
/// [`into_pidfd`]: ChildExt::into_pidfd
fn create_pidfd(&mut self, val: bool) -> &mut process::Command;
}

View file

@ -50,14 +50,13 @@ fn test_pidfd() {
return;
}
let mut child = Command::new("sleep")
let child = Command::new("sleep")
.arg("1000")
.create_pidfd(true)
.spawn()
.expect("executing 'sleep' failed");
let fd = child.take_pidfd().unwrap();
drop(child);
let fd = child.into_pidfd().unwrap();
assert_matches!(fd.try_wait(), Ok(None));
fd.kill().expect("kill failed");

View file

@ -1098,12 +1098,12 @@ mod linux_child_ext {
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
}
fn take_pidfd(&mut self) -> io::Result<os::PidFd> {
fn into_pidfd(mut self) -> Result<os::PidFd, Self> {
self.handle
.pidfd
.take()
.map(|fd| <os::PidFd as FromInner<imp::PidFd>>::from_inner(fd))
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
.ok_or_else(|| self)
}
}
}