context: give spawned children a clean fd table
pkh's process carries inheritable descriptors (async runtime, gpgme sockets, the log tee) that std passes to every child. Tools with fd-number assumptions then break on the shifted table: mmdebstrap's apt hook is `cat >&N`, and /bin/sh (dash) rejects any two-digit N with "Bad fd number" — a fresh-chroot bootstrap died with "mmdebstrap failed to run" whenever the hook pipe landed above fd 9, and succeeded from a clean shell where it lands lower. Sweep fds >= 3 pre-exec in the local driver's spawns: it is the one funnel every context spawn goes through (schroot and unshare chain through their parent, ssh execs remotely over libssh2). Verified end to end: the sid bootstrap test fails on the unfixed code under a polluted fd table and passes with the sweep under the same table.
This commit is contained in:
@@ -4,6 +4,7 @@ use super::api::{ContextDriver, LineSink, Stream};
|
|||||||
use super::capture::pump;
|
use super::capture::pump;
|
||||||
use std::io;
|
use std::io;
|
||||||
use std::os::unix::fs::symlink;
|
use std::os::unix::fs::symlink;
|
||||||
|
use std::os::unix::process::CommandExt;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::process::{Command, Stdio};
|
use std::process::{Command, Stdio};
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
@@ -11,6 +12,30 @@ use std::time::SystemTime;
|
|||||||
|
|
||||||
pub struct LocalDriver;
|
pub struct LocalDriver;
|
||||||
|
|
||||||
|
/// Close every file descriptor above stdio in the spawned child
|
||||||
|
///
|
||||||
|
/// pkh's process legitimately carries descriptors that stay open for its
|
||||||
|
/// lifetime (the async runtime, gpgme sockets, the log tee), and std passes
|
||||||
|
/// any inheritable one to children. Tools with fd-number assumptions then
|
||||||
|
/// see their low-numbered allocations land on two-digit fds: mmdebstrap's
|
||||||
|
/// apt hook is `cat >&N`, which /bin/sh (dash) rejects for N >= 10 with
|
||||||
|
/// "Bad fd number" — the fresh-chroot bootstrap died exactly there. A
|
||||||
|
/// clean child table keeps those numbers single-digit.
|
||||||
|
fn sanitize_fds(cmd: &mut Command) {
|
||||||
|
unsafe {
|
||||||
|
cmd.pre_exec(|| {
|
||||||
|
// One syscall when the kernel has close_range; the loop
|
||||||
|
// covers the rest (and is what the syscall does anyway).
|
||||||
|
if libc::syscall(libc::SYS_close_range, 3, libc::c_uint::MAX, 0u32) != 0 {
|
||||||
|
for fd in 3..1024 {
|
||||||
|
libc::close(fd);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl ContextDriver for LocalDriver {
|
impl ContextDriver for LocalDriver {
|
||||||
fn ensure_available(&self, src: &Path, dest_root: &str) -> io::Result<PathBuf> {
|
fn ensure_available(&self, src: &Path, dest_root: &str) -> io::Result<PathBuf> {
|
||||||
let dest_root_path = Path::new(dest_root);
|
let dest_root_path = Path::new(dest_root);
|
||||||
@@ -77,6 +102,7 @@ impl ContextDriver for LocalDriver {
|
|||||||
if let Some(dir) = cwd {
|
if let Some(dir) = cwd {
|
||||||
cmd.current_dir(dir);
|
cmd.current_dir(dir);
|
||||||
}
|
}
|
||||||
|
sanitize_fds(&mut cmd);
|
||||||
cmd.status()
|
cmd.status()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -99,6 +125,7 @@ impl ContextDriver for LocalDriver {
|
|||||||
cmd.stdout(Stdio::piped())
|
cmd.stdout(Stdio::piped())
|
||||||
.stderr(Stdio::piped())
|
.stderr(Stdio::piped())
|
||||||
.stdin(Stdio::null());
|
.stdin(Stdio::null());
|
||||||
|
sanitize_fds(&mut cmd);
|
||||||
|
|
||||||
let mut child = cmd.spawn()?;
|
let mut child = cmd.spawn()?;
|
||||||
let stdout = child.stdout.take();
|
let stdout = child.stdout.take();
|
||||||
@@ -137,6 +164,7 @@ impl ContextDriver for LocalDriver {
|
|||||||
if let Some(dir) = cwd {
|
if let Some(dir) = cwd {
|
||||||
cmd.current_dir(dir);
|
cmd.current_dir(dir);
|
||||||
}
|
}
|
||||||
|
sanitize_fds(&mut cmd);
|
||||||
cmd.output()
|
cmd.output()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -227,4 +255,34 @@ mod tests {
|
|||||||
std::fs::remove_dir(name).unwrap();
|
std::fs::remove_dir(name).unwrap();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Inherited descriptors must not leak into spawned children: the
|
||||||
|
/// tools' low-numbered fd assumptions break on a shifted table
|
||||||
|
/// (mmdebstrap's dash-executed `cat >&N` apt hook).
|
||||||
|
#[test]
|
||||||
|
fn spawned_children_get_a_clean_fd_table() {
|
||||||
|
// Raw open(): std marks its own descriptors close-on-exec, which
|
||||||
|
// would make the test pass vacuously.
|
||||||
|
let mut strays = Vec::new();
|
||||||
|
for _ in 0..8 {
|
||||||
|
let path = std::ffi::CString::new("/dev/null").unwrap();
|
||||||
|
let fd = unsafe { libc::open(path.as_ptr(), libc::O_RDWR) };
|
||||||
|
assert!(fd >= 0);
|
||||||
|
strays.push(fd);
|
||||||
|
}
|
||||||
|
let mut cmd = Command::new("sh");
|
||||||
|
cmd.arg("-c").arg("ls /proc/self/fd");
|
||||||
|
sanitize_fds(&mut cmd);
|
||||||
|
let out = cmd.output().unwrap();
|
||||||
|
for fd in strays {
|
||||||
|
unsafe { libc::close(fd) };
|
||||||
|
}
|
||||||
|
|
||||||
|
// stdio (3) + the directory ls itself opened (1)
|
||||||
|
let table = String::from_utf8_lossy(&out.stdout);
|
||||||
|
assert!(
|
||||||
|
table.lines().count() <= 5,
|
||||||
|
"child fd table should be clean, got:\n{table}"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user