refactor(duplex): own the dup'd descriptors, so a failed redirect cannot leak one

`StdioDuplex::take` dup'd stdin and stdout, redirected both to /dev/null, and
only then wrapped the raw descriptors in `File`. Any of the four `?`s between
the first `dup` and the wrap returned without closing what it already held —
a descriptor leak on the error path.

`dup_fd` now hands back an `OwnedFd`. `dup` returns a fresh descriptor nothing
else holds, which is exactly that type's contract, and ownership from the
moment it exists is what makes the early returns safe. `take` drops two
`unsafe` blocks in the process: `File::from(OwnedFd)` is the safe conversion.

Immaterial in production — `take` is called once in tty7-server's main and the
`?` there exits the process — but it is unsafe-adjacent code where the correct
version is also the shorter one.

Exercised by the 55 `stdio_conformance` tests, which drive
`tty7-server --stdio` through this constructor. 2934 pass.
This commit is contained in:
l0ng-ai
2026-08-15 18:11:17 +08:00
parent 768aca01f1
commit fd100f0e95
+12 -6
View File
@@ -66,15 +66,16 @@ pub struct StdioDuplex {
#[cfg(unix)]
impl StdioDuplex {
pub fn take() -> io::Result<StdioDuplex> {
use std::os::fd::FromRawFd as _;
// Each descriptor is owned from the moment it exists, so the four `?`s
// below cannot strand one: a `redirect_to_null` that fails after both
// `dup`s used to return without closing either.
let stdin_fd = dup_fd(libc::STDIN_FILENO)?;
let stdout_fd = dup_fd(libc::STDOUT_FILENO)?;
redirect_to_null(libc::STDIN_FILENO)?;
redirect_to_null(libc::STDOUT_FILENO)?;
let read = unsafe { std::fs::File::from_raw_fd(stdin_fd) };
let write = unsafe { std::fs::File::from_raw_fd(stdout_fd) };
let read = std::fs::File::from(stdin_fd);
let write = std::fs::File::from(stdout_fd);
Ok(StdioDuplex {
read,
write: StdioWriter {
@@ -147,13 +148,18 @@ impl LinkShutdown for StdioWriter {
}
}
/// Hands back an *owned* descriptor rather than a raw one, so that a caller
/// which gives up between one `dup` and the next drops it instead of leaking
/// it. `dup` returns a fresh descriptor that nothing else holds, which is
/// exactly the contract `OwnedFd` wants.
#[cfg(unix)]
fn dup_fd(fd: libc::c_int) -> io::Result<libc::c_int> {
fn dup_fd(fd: libc::c_int) -> io::Result<std::os::fd::OwnedFd> {
use std::os::fd::FromRawFd as _;
let new = unsafe { libc::dup(fd) };
if new < 0 {
return Err(io::Error::last_os_error());
}
Ok(new)
Ok(unsafe { std::os::fd::OwnedFd::from_raw_fd(new) })
}
#[cfg(unix)]