David Uhden Collado wrote:
Kirill A. Korinsky wrote:
On Tue, 20 Oct 2026 13:12:00 +0200,
David Uhden Collado <[email protected]> wrote:
I agree that this should ultimately be fixed upstream. The concern
about post-fork allocation remains valid, but the replacement needs
a better worst-case bound.
Please, open an issue in upstream when.
OpenBSD is not an officially supported platform, so they will probably
ignore it, and it will remain that way in the port.
I've replaced the previous implementation with a different approach
using OpenBSD's sysctl(KERN_FILE_BYPID).
The new implementation:
- Allocates the descriptor buffer before fork().
- Takes a fresh descriptor snapshot in the child.
- Closes only descriptors that are neither preserved nor CLOEXEC.
- Preserves Rust's CLOEXEC pipe used to report exec() errors.
- Returns an error if the snapshot or cleanup fails.
The kernel still scans the descriptor table internally, but this
avoids the previous sequence of fcntl() calls for unused descriptors.
Prepare OpenBSD descriptor cleanup in the parent so the pre-exec callback
can close unneeded inherited descriptors without allocating after fork.
Propagate cleanup errors to abort spawning if descriptors cannot be safely
closed.
Index: codex-rs/utils/pty/src/child_command.rs
--- codex-rs/utils/pty/src/child_command.rs.orig
+++ codex-rs/utils/pty/src/child_command.rs
@@ -300,9 +300,15 @@
#[cfg(not(target_os = "linux"))]
let parent_pid: Option<i32> = None;
if new_session || explicit_fds || !targets.is_empty() ||
parent_pid.is_some() {
+ #[cfg(target_os = "openbsd")]
+ let mut fd_cleanup = if explicit_fds {
+ Some(crate::pty::OpenBsdFdCleanup::new()?)
+ } else {
+ None
+ };
// SAFETY: The caller keeps the selected descriptors open.
Session
- // and parent-death setup use system calls; Linux and macOS
cleanup
- // avoid allocation after fork. Other Unix targets have an
+ // and parent-death setup use system calls; Linux, macOS and
OpenBSD
+ // cleanup avoid allocation after fork. Other Unix targets
have an
// allocation-after-fork risk documented on
close_inherited_fds_except.
unsafe {
self.inner.pre_exec(move || {
@@ -312,6 +318,11 @@
if let Some(parent_pid) = parent_pid {
crate::process_group::set_parent_death_signal(parent_pid)?;
}
+ #[cfg(target_os = "openbsd")]
+ if let Some(cleanup) = fd_cleanup.as_mut() {
+ cleanup.close_inherited_fds_except(&targets)?;
+ }
+ #[cfg(not(target_os = "openbsd"))]
if explicit_fds {
crate::pty::close_inherited_fds_except(&targets);
}
Use OpenBSD KERN_FILE snapshots to close inherited descriptors, including
those above a lowered resource limit, while preserving standard streams,
explicitly passed descriptors and CLOEXEC descriptors such as Rust's exec
error pipe. Allocate storage before fork and block signal handlers while
reading and consuming the child's snapshot; abort spawning on cleanup errors.
Index: codex-rs/utils/pty/src/pty.rs
--- codex-rs/utils/pty/src/pty.rs.orig
+++ codex-rs/utils/pty/src/pty.rs
@@ -397,6 +397,8 @@
}
let inherited_fds = inherited_fds.to_vec();
// The child sees the same terminal on stdin, stdout, and stderr.
+ #[cfg(target_os = "openbsd")]
+ let mut fd_cleanup = OpenBsdFdCleanup::new()?;
unsafe {
command
.stdin(Stdio::from(slave.try_clone()?))
@@ -404,6 +406,9 @@
.stderr(Stdio::from(slave.try_clone()?))
.pre_exec(move || {
configure_child_terminal()?;
+ #[cfg(target_os = "openbsd")]
+ fd_cleanup.close_inherited_fds_except(&inherited_fds)?;
+ #[cfg(not(target_os = "openbsd"))]
close_inherited_fds_except(&inherited_fds);
make_fds_inheritable(&inherited_fds)?;
Ok(())
@@ -606,9 +611,126 @@
}
}
+// Allocate in the parent; take the descriptor snapshot only in the child so it
+// includes std::process's private CLOEXEC error pipe and concurrent parent
opens.
+// KERN_FILE requires an unpledged process.
+#[cfg(target_os = "openbsd")]
+pub(crate) struct OpenBsdFdCleanup {
+ mib: [libc::c_int; 6],
+ descriptors: Vec<std::mem::MaybeUninit<libc::kinfo_file>>,
+}
+
+#[cfg(target_os = "openbsd")]
+impl OpenBsdFdCleanup {
+ pub(crate) fn new() -> std::io::Result<Self> {
+ let mut mib = [
+ libc::CTL_KERN,
+ libc::KERN_FILE,
+ libc::KERN_FILE_BYPID,
+ unsafe { libc::getpid() },
+ std::mem::size_of::<libc::kinfo_file>() as libc::c_int,
+ libc::c_int::MAX,
+ ];
+ let mut bytes = 0;
+ // SAFETY: A null output buffer requests the size, including kernel
slop.
+ if unsafe {
+ libc::sysctl(
+ mib.as_ptr(),
+ 6,
+ std::ptr::null_mut(),
+ &mut bytes,
+ std::ptr::null_mut(),
+ 0,
+ )
+ } == -1 {
+ return Err(std::io::Error::last_os_error());
+ }
+ let mut limit: libc::rlimit = unsafe { std::mem::zeroed() };
+ if unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit) } == -1 {
+ return Err(std::io::Error::last_os_error());
+ }
+ // Allow concurrent opens up to the current limit, plus
cwd/root/text/trace.
+ // The snapshot size also covers already-open descriptors above a
lowered
+ // limit. Neither value is used as a bound on descriptor numbers.
+ let limit_count = usize::try_from(limit.rlim_cur)
+ .ok()
+ .and_then(|count| count.checked_add(4))
+ .ok_or_else(||
std::io::Error::from_raw_os_error(libc::EOVERFLOW))?;
+ let count = (bytes /
std::mem::size_of::<libc::kinfo_file>()).max(limit_count);
+ mib[5] = libc::c_int::try_from(count)
+ .map_err(|_| std::io::Error::from_raw_os_error(libc::EOVERFLOW))?;
+ let mut descriptors = Vec::new();
+ descriptors
+ .try_reserve_exact(count)
+ .map_err(|_| std::io::Error::from_raw_os_error(libc::ENOMEM))?;
+ descriptors.resize_with(count, std::mem::MaybeUninit::uninit);
+ Ok(Self { mib, descriptors })
+ }
+
+ pub(crate) fn close_inherited_fds_except(
+ &mut self,
+ preserved_fds: &[RawFd],
+ ) -> std::io::Result<()> {
+ // SAFETY: Only syscalls and preallocated storage are used after fork.
+ // Block handlers while taking and consuming the child's snapshot.
+ unsafe {
+ let mut all_signals: libc::sigset_t = std::mem::zeroed();
+ let mut old_mask: libc::sigset_t = std::mem::zeroed();
+ if libc::sigfillset(&mut all_signals) == -1
+ || libc::sigprocmask(libc::SIG_BLOCK, &all_signals, &mut
old_mask) == -1
+ {
+ return Err(std::io::Error::last_os_error());
+ }
+ let result = (|| {
+ self.mib[3] = libc::getpid();
+ let capacity =
std::mem::size_of_val(self.descriptors.as_slice());
+ let mut bytes = capacity;
+ // ENOMEM means an incomplete snapshot: fail the spawn rather
+ // than leak descriptors if the table grew since preparation.
+ if libc::sysctl(
+ self.mib.as_ptr(),
+ 6,
+ self.descriptors.as_mut_ptr().cast(),
+ &mut bytes,
+ std::ptr::null_mut(),
+ 0,
+ ) == -1
+ {
+ return Err(std::io::Error::last_os_error());
+ }
+ let entry_size = std::mem::size_of::<libc::kinfo_file>();
+ if bytes > capacity || bytes % entry_size != 0 {
+ return Err(std::io::Error::from_raw_os_error(libc::EIO));
+ }
+ for entry in &self.descriptors[..bytes / entry_size] {
+ let entry = entry.assume_init_ref();
+ let fd = entry.fd_fd;
+ // Negative entries describe cwd/root/text, not
descriptors.
+ // UF_EXCLOSE in fd_ofileflags is OpenBSD's FD_CLOEXEC bit.
+ if fd > libc::STDERR_FILENO
+ && !preserved_fds.contains(&fd)
+ && entry.fd_ofileflags & libc::FD_CLOEXEC as u32 == 0
+ && libc::close(fd) == -1
+ {
+ return Err(std::io::Error::last_os_error());
+ }
+ }
+ Ok(())
+ })();
+ if libc::sigprocmask(libc::SIG_SETMASK, &old_mask,
std::ptr::null_mut()) == -1 {
+ return Err(std::io::Error::last_os_error());
+ }
+ result
+ }
+ }
+}
+
// Other Unix platforms use best-effort /dev/fd cleanup.
// Directory enumeration allocates, so this path is not guaranteed fork-safe.
-#[cfg(all(unix, not(any(target_os = "linux", target_os = "macos"))))]
+#[cfg(all(
+ unix,
+ not(any(target_os = "linux", target_os = "macos", target_os = "openbsd"))
+))]
pub(crate) fn close_inherited_fds_except(preserved_fds: &[RawFd]) {
if let Ok(dir) = std::fs::read_dir("/dev/fd") {
let mut fds = Vec::new();
