From c2ec99b66376cc4c47ef3c041282f8e5d003b79b Mon Sep 17 00:00:00 2001
From: The 8472 <git@infinite-source.de>
Date: Thu, 13 Jun 2024 23:55:03 +0200
Subject: [PATCH] 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.
---
 std/src/os/linux/process.rs                  | 22 +++++++++++++-------
 std/src/sys/pal/unix/linux/pidfd/tests.rs    |  5 ++---
 std/src/sys/pal/unix/process/process_unix.rs |  4 ++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/std/src/os/linux/process.rs b/std/src/os/linux/process.rs
index 4e8648eb3fc94..9195909479729 100644
--- a/std/src/os/linux/process.rs
+++ b/std/src/os/linux/process.rs
@@ -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;
 }
 
diff --git a/std/src/sys/pal/unix/linux/pidfd/tests.rs b/std/src/sys/pal/unix/linux/pidfd/tests.rs
index 42b3df6299114..6d9532f2ef1ff 100644
--- a/std/src/sys/pal/unix/linux/pidfd/tests.rs
+++ b/std/src/sys/pal/unix/linux/pidfd/tests.rs
@@ -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");
diff --git a/std/src/sys/pal/unix/process/process_unix.rs b/std/src/sys/pal/unix/process/process_unix.rs
index 479f18876e30d..32382d9a50cf4 100644
--- a/std/src/sys/pal/unix/process/process_unix.rs
+++ b/std/src/sys/pal/unix/process/process_unix.rs
@@ -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)
         }
     }
 }