Skip to content

Commit 02d9995

Browse files
committed
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.
1 parent 32e70ca commit 02d9995

File tree

3 files changed

+19
-12
lines changed

3 files changed

+19
-12
lines changed

library/std/src/os/linux/process.rs

+15-7
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ struct InnerPidFd;
2020
///
2121
/// A `PidFd` can be obtained by setting the corresponding option on [`Command`]
2222
/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved
23-
/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`].
23+
/// from the [`Child`] by calling [`pidfd`] or [`into_pidfd`].
2424
///
2525
/// Example:
2626
/// ```no_run
@@ -34,7 +34,7 @@ struct InnerPidFd;
3434
/// .expect("Failed to spawn child");
3535
///
3636
/// let pidfd = child
37-
/// .take_pidfd()
37+
/// .into_pidfd()
3838
/// .expect("Failed to retrieve pidfd");
3939
///
4040
/// // The file descriptor will be closed when `pidfd` is dropped.
@@ -45,7 +45,7 @@ struct InnerPidFd;
4545
/// [`create_pidfd`]: CommandExt::create_pidfd
4646
/// [`Child`]: process::Child
4747
/// [`pidfd`]: fn@ChildExt::pidfd
48-
/// [`take_pidfd`]: ChildExt::take_pidfd
48+
/// [`into_pidfd`]: ChildExt::into_pidfd
4949
/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html
5050
#[derive(Debug)]
5151
#[repr(transparent)]
@@ -160,18 +160,26 @@ pub trait ChildExt: Sealed {
160160
/// [`Child`]: process::Child
161161
fn pidfd(&self) -> Result<&PidFd>;
162162

163-
/// Takes ownership of the [`PidFd`] created for this [`Child`], if available.
163+
/// Returns the [`PidFd`] created for this [`Child`], if available.
164+
/// Otherwise self is returned.
164165
///
165166
/// A pidfd will only be available if its creation was requested with
166167
/// [`create_pidfd`] when the corresponding [`Command`] was created.
167168
///
169+
/// Taking ownership of the PidFd consumes the Child to avoid pid reuse
170+
/// races. Use [`pidfd`] and [`BorrowedFd::try_clone_to_owned`] if
171+
/// you don't want to disassemble the Child yet.
172+
///
168173
/// Even if requested, a pidfd may not be available due to an older
169174
/// version of Linux being in use, or if some other error occurred.
170175
///
171176
/// [`Command`]: process::Command
172177
/// [`create_pidfd`]: CommandExt::create_pidfd
178+
/// [`pidfd`]: ChildExt::pidfd
173179
/// [`Child`]: process::Child
174-
fn take_pidfd(&mut self) -> Result<PidFd>;
180+
fn into_pidfd(self) -> crate::result::Result<PidFd, Self>
181+
where
182+
Self: Sized;
175183
}
176184

177185
/// Os-specific extensions for [`Command`]
@@ -182,7 +190,7 @@ pub trait CommandExt: Sealed {
182190
/// spawned by this [`Command`].
183191
/// By default, no pidfd will be created.
184192
///
185-
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
193+
/// The pidfd can be retrieved from the child with [`pidfd`] or [`into_pidfd`].
186194
///
187195
/// A pidfd will only be created if it is possible to do so
188196
/// in a guaranteed race-free manner. Otherwise, [`pidfd`] will return an error.
@@ -196,7 +204,7 @@ pub trait CommandExt: Sealed {
196204
/// [`Command`]: process::Command
197205
/// [`Child`]: process::Child
198206
/// [`pidfd`]: fn@ChildExt::pidfd
199-
/// [`take_pidfd`]: ChildExt::take_pidfd
207+
/// [`into_pidfd`]: ChildExt::into_pidfd
200208
fn create_pidfd(&mut self, val: bool) -> &mut process::Command;
201209
}
202210

library/std/src/sys/pal/unix/linux/pidfd/tests.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,13 @@ fn test_pidfd() {
5050
return;
5151
}
5252

53-
let mut child = Command::new("sleep")
53+
let child = Command::new("sleep")
5454
.arg("1000")
5555
.create_pidfd(true)
5656
.spawn()
5757
.expect("executing 'sleep' failed");
5858

59-
let fd = child.take_pidfd().unwrap();
60-
drop(child);
59+
let fd = child.into_pidfd().unwrap();
6160

6261
assert_matches!(fd.try_wait(), Ok(None));
6362
fd.kill().expect("kill failed");

library/std/src/sys/pal/unix/process/process_unix.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1098,12 +1098,12 @@ mod linux_child_ext {
10981098
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
10991099
}
11001100

1101-
fn take_pidfd(&mut self) -> io::Result<os::PidFd> {
1101+
fn into_pidfd(mut self) -> Result<os::PidFd, Self> {
11021102
self.handle
11031103
.pidfd
11041104
.take()
11051105
.map(|fd| <os::PidFd as FromInner<imp::PidFd>>::from_inner(fd))
1106-
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
1106+
.ok_or_else(|| self)
11071107
}
11081108
}
11091109
}

0 commit comments

Comments
 (0)