Make Linux descriptor cleanup fork-safe (#47654)

## Why

Linux descriptor cleanup used allocating directory enumeration after `fork`. The child needs allocation-free cleanup while preserving Rust's spawn-error channel until `exec` succeeds.

## What changed

- Mark unrelated descriptors close-on-exec with `close_range(CLOSE_RANGE_CLOEXEC)`, leaving stdio and explicitly preserved descriptors' flags untouched.
- Fall back to stack-backed `getdents64` enumeration of the child's descriptor table when `close_range` fails.
- Document best-effort cleanup: failures allow launch to proceed and may leave unrelated descriptors inherited.

## Testing

Add Linux tests for descriptor inheritance, unsorted and duplicate allowlists, unchanged parent descriptors, fallback cleanup above a lowered descriptor limit, preserved exec-error reporting, and successful launch when both cleanup paths fail. Retain MCP coverage for descriptor exclusion in servers and descendants.

GitOrigin-RevId: 13abb95f82557324c7b40feb36bd6f806d60674e
This commit is contained in:
Yuzhu Shen
2026-09-23 20:20:06 +00:00
committed by copyberry
parent 5bfb04b247
commit b086ad5493
7 changed files with 423 additions and 79 deletions
@@ -10,7 +10,7 @@
//! Both paths return [`StdioServerTransport`], so `RmcpClient` can hand the
//! resulting byte stream to rmcp without knowing where the process lives. The
//! executor-specific byte adaptation lives in `executor_process_transport`.
//! Unix local servers inherit only their explicit transport stdio.
//! Unix local servers use the stdio-only descriptor policy.
use std::collections::HashMap;
use std::ffi::OsString;
@@ -282,8 +282,8 @@ impl LocalStdioServerLauncher {
let mut command = Command::new(&resolved_program);
command.current_dir(&cwd).envs(&envs).args(&args);
command.process_mode(ProcessMode::NewGroup);
// MCP uses only stdio; unrelated orchestrator descriptors must not
// propagate into the server or commands it launches.
// MCP uses only stdio; select Explicit to exclude unrelated
// orchestrator descriptors from the server and commands it launches.
// Descriptor allowlisting is Unix-only. Windows can still inherit unrelated
// handles and needs a handle allowlist in the shared spawn backend.
#[cfg(unix)]
@@ -3,7 +3,6 @@
#![cfg(unix)]
use std::collections::HashMap;
use std::ffi::OsString;
use std::fs;
use std::os::fd::AsRawFd;
@@ -11,7 +10,6 @@ use std::os::fd::FromRawFd;
use std::os::fd::OwnedFd;
use std::os::unix::fs::PermissionsExt;
use std::os::unix::fs::symlink;
use std::os::unix::net::UnixStream;
use std::process::Command;
use std::sync::Arc;
use std::time::Duration;
@@ -32,28 +30,23 @@ use serde_json::json;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn local_stdio_excludes_inheritable_fds_from_server_and_descendant() -> Result<()> {
let temporary = tempfile::tempdir()?;
// macOS temporary paths can contain symlinks, while getcwd resolves them.
let cwd = fs::canonicalize(temporary.path())?;
let script = temporary.path().join("server.py");
fs::write(
&script,
r#"import errno, json, os, signal, subprocess, sys
signal.alarm(30)
sentinels = json.loads(sys.argv[2])
sentinel = int(sys.argv[2])
def probe():
inherited = []
for fd in sentinels:
try:
os.fstat(fd)
inherited.append(True)
except OSError as error:
if error.errno != errno.EBADF:
raise
inherited.append(False)
for fd in (0, 1, 2):
os.fstat(fd)
print("descriptor fixture stderr", file=sys.stderr, flush=True)
return inherited
try:
os.fstat(sentinel)
return True
except OSError as error:
if error.errno != errno.EBADF:
raise
return False
if sys.argv[1] == "probe":
print(json.dumps(probe()), flush=True)
sys.exit(0)
@@ -62,56 +55,39 @@ for line in sys.stdin:
method = message.get("method")
if method == "initialize":
result = {"protocolVersion": "2025-06-18", "capabilities": {"tools": {}}, "serverInfo": {"name": "descriptor-fixture", "version": "1"}}
elif method == "tools/list":
result = {"tools": [{"name": "probe", "inputSchema": {"type": "object"}}]}
elif method == "tools/call":
# Python normally closes extra descriptors itself. Disable that cleanup
# so the descendant actually exercises the MCP launch boundary.
child = subprocess.run([sys.executable, __file__, "probe", sys.argv[2]], close_fds=False, capture_output=True, text=True, check=True, timeout=5)
result = {"content": [], "structuredContent": {
"server": probe(), "descendant": json.loads(child.stdout),
"descendantStderr": child.stderr,
"cwd": os.getcwd(), "env": os.environ["MCP_DESCRIPTOR_TEST"],
"argument": sys.argv[3], "request": message["params"]["arguments"]}}
"descendantStderr": child.stderr}}
else:
continue
print(json.dumps({"jsonrpc": "2.0", "id": message["id"], "result": result}), flush=True)
"#,
)?;
let (pipe_reader, _pipe_writer) = std::io::pipe()?;
let (socket, _socket_peer) = UnixStream::pair()?;
let file = tempfile::tempfile()?;
let mut sentinels = Vec::new();
for fd in [
pipe_reader.as_raw_fd(),
socket.as_raw_fd(),
file.as_raw_fd(),
] {
// F_DUPFD creates inheritable descriptors. Reserve unused high numbers
// without overwriting a descriptor owned by the test runner/runtime.
let duplicate = unsafe { libc::fcntl(fd, libc::F_DUPFD, 200) };
if duplicate < 0 {
return Err(std::io::Error::last_os_error().into());
}
// SAFETY: fcntl returned a new descriptor owned only by this test.
sentinels.push(unsafe { OwnedFd::from_raw_fd(duplicate) });
// F_DUPFD reserves an unused inheritable descriptor without overwriting
// a descriptor owned by the test runner/runtime.
let raw = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_DUPFD, 200) };
if raw < 0 {
return Err(std::io::Error::last_os_error().into());
}
let sentinel_fds: Vec<_> = sentinels.iter().map(AsRawFd::as_raw_fd).collect();
let sentinel_args = serde_json::to_string(&sentinel_fds)?;
// SAFETY: fcntl returned a new descriptor owned only by this test.
let sentinel = unsafe { OwnedFd::from_raw_fd(raw) };
let sentinel_arg = raw.to_string();
let python = which::which("python3")?;
// Qualify the oracle: the same executable must see all sentinels when
// Qualify the oracle: the same executable must see the sentinel when
// spawned without the local MCP descriptor policy.
let control = Command::new(&python)
.arg(&script)
.arg("probe")
.arg(&sentinel_args)
.arg(&sentinel_arg)
.output()?;
assert!(control.status.success());
assert_eq!(
serde_json::from_slice::<serde_json::Value>(&control.stdout)?,
json!([true, true, true])
);
assert!(serde_json::from_slice::<bool>(&control.stdout)?);
symlink(&python, temporary.path().join("python-fixture"))?;
let wrapper = temporary.path().join("executable-text");
@@ -131,18 +107,14 @@ for line in sys.stdin:
args.extend([
script.clone().into_os_string(),
"server".into(),
sentinel_args.clone().into(),
"argument with spaces".into(),
sentinel_arg.clone().into(),
]);
let client = RmcpClient::new_stdio_client(
program,
args,
Some(HashMap::from([(
OsString::from("MCP_DESCRIPTOR_TEST"),
OsString::from("environment preserved"),
)])),
/*env*/ None,
&[],
Some(cwd.to_string_lossy().into_owned()),
Some(temporary.path().to_string_lossy().into_owned()),
Arc::new(LocalStdioServerLauncher::new(std::env::current_dir()?)),
)
.await?;
@@ -166,15 +138,10 @@ for line in sys.stdin:
}),
)
.await?;
let tools = client
.list_tools(/*params*/ None, Some(Duration::from_secs(/*secs*/ 5)))
.await?;
assert_eq!(tools.tools.len(), 1);
assert_eq!(tools.tools[0].name, "probe");
let result = client
.call_tool(
"probe".to_string(),
Some(json!({"message": "stdio round trip"})),
/*arguments*/ None,
/*meta*/ None,
Some(Duration::from_secs(/*secs*/ 5)),
)
@@ -183,24 +150,18 @@ for line in sys.stdin:
assert_eq!(
result.structured_content,
Some(json!({
"server": [false, false, false],
"descendant": [false, false, false],
"descendantStderr": "descriptor fixture stderr\n",
"cwd": cwd,
"env": "environment preserved",
"argument": "argument with spaces",
"request": {"message": "stdio round trip"}
"server": false,
"descendant": false,
"descendantStderr": "descriptor fixture stderr\n"
})),
"{launch} launch"
);
}
// Cleanup happens only in the spawned process; the parent still owns its
// original inheritable descriptors after the MCP client shuts down.
let flags: Vec<_> = sentinel_fds
.iter()
.map(|fd| unsafe { libc::fcntl(*fd, libc::F_GETFD) })
.collect();
assert_eq!(flags, vec![0, 0, 0]);
// Cleanup must not close or change the parent's descriptor.
assert_eq!(
unsafe { libc::fcntl(sentinel.as_raw_fd(), libc::F_GETFD) },
0
);
Ok(())
}
+5 -3
View File
@@ -38,6 +38,7 @@ pub enum ProcessMode {
#[derive(Clone, Copy, PartialEq, Eq)]
pub enum DescriptorPolicy {
Inherit,
/// Exclude unrelated descriptors, allowing launch if best-effort cleanup fails.
Explicit,
}
@@ -286,9 +287,10 @@ impl Command {
#[cfg(not(target_os = "linux"))]
let parent_pid: Option<i32> = None;
if new_session || explicit_fds || !targets.is_empty() || parent_pid.is_some() {
// SAFETY: Keep the existing Unix pre-exec setup in the fallback
// backend. The caller keeps the selected descriptors open, and
// this callback only changes the child's descriptor table.
// 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
// allocation-after-fork risk documented on close_inherited_fds_except.
unsafe {
self.inner.pre_exec(move || {
if new_session {
+2
View File
@@ -6,6 +6,8 @@ pub use child_command::Command;
pub use child_command::DescriptorPolicy;
pub use child_command::ProcessMode;
pub use child_command::SpawnFallback;
#[cfg(target_os = "linux")]
mod linux_fds;
pub mod pipe;
mod process;
pub mod process_group;
+129
View File
@@ -0,0 +1,129 @@
//! Linux descriptor cleanup after fork, using only stack storage and syscalls.
//! Stdio and explicitly preserved FDs remain usable. CLOEXEC keeps Rust's
//! spawn-error channel alive until exec succeeds. Cleanup is best-effort:
//! failures do not prevent launch and may leave unrelated descriptors inherited.
use std::ffi::CStr;
use std::io;
use std::os::fd::RawFd;
pub(crate) fn close_inherited_fds_except(preserved_fds: &[RawFd]) {
// Mark rather than close: std::process still needs its CLOEXEC error
// pipe if exec fails. Do not alter flags on explicitly preserved FDs.
let mut first = 3_u32;
loop {
// The keep-list need not be sorted. Walking its gaps leaves each kept
// FD's flags untouched, without allocating a sorted copy after fork.
let next = preserved_fds
.iter()
.copied()
.filter_map(|fd| u32::try_from(fd).ok())
.filter(|&fd| fd >= first)
.min();
let last = next.map_or(u32::MAX, |fd| fd - 1);
if first <= last
// SAFETY: close_range has no pointers and changes only this child's FDs.
&& unsafe {
libc::syscall(
libc::SYS_close_range,
first,
last,
libc::CLOSE_RANGE_CLOEXEC,
)
} != 0
{
break;
}
match next {
Some(fd) => first = fd + 1,
None => return,
}
}
// Older kernels (or seccomp policies) may reject close_range. Read
// this child's descriptor table without libc's allocating DIR API.
// Preserve best-effort launch behavior if the fallback also fails. Logging
// here could allocate or lock after fork; writing to child stderr may block.
close_from_proc(preserved_fds);
}
fn mark_cloexec(fd: RawFd, preserved_fds: &[RawFd]) {
if fd <= libc::STDERR_FILENO || preserved_fds.contains(&fd) {
return;
}
// SAFETY: fcntl operates on the child's descriptors without allocating.
let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
if flags == -1 {
return;
}
if flags & libc::FD_CLOEXEC == 0 {
// SAFETY: only adds CLOEXEC; existing flags and the descriptor remain intact.
unsafe { libc::fcntl(fd, libc::F_SETFD, flags | libc::FD_CLOEXEC) };
}
}
fn close_from_proc(preserved_fds: &[RawFd]) {
use std::os::fd::AsRawFd;
use std::os::fd::FromRawFd;
use std::os::fd::OwnedFd;
// Open after fork: a proc directory opened in the parent would enumerate
// the parent's changing descriptor table instead of this child's snapshot.
// SAFETY: the path is NUL-terminated and open does not allocate.
let raw = unsafe {
libc::open(
c"/proc/self/fd".as_ptr(),
libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
)
};
if raw == -1 {
return;
}
// SAFETY: open returned a new owned descriptor; dropping it only calls close.
let directory = unsafe { OwnedFd::from_raw_fd(raw) };
let mut buffer = [0_u8; 4096];
loop {
// SAFETY: getdents64 writes at most buffer.len() bytes into stack storage.
let count = unsafe {
libc::syscall(
libc::SYS_getdents64,
directory.as_raw_fd(),
buffer.as_mut_ptr(),
buffer.len(),
)
};
if count == -1 {
if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) {
continue;
}
return;
}
if count == 0 {
return;
}
let mut entries = &buffer[..count as usize];
// Linux getdents64 records have two 64-bit fields, a u16 record length,
// one type byte, then the NUL-terminated name (independent of libc ABI).
while !entries.is_empty() {
if entries.len() < 20 {
return;
}
let length = u16::from_ne_bytes([entries[16], entries[17]]) as usize;
if length < 20 || length > entries.len() {
return;
}
if let Ok(name) = CStr::from_bytes_until_nul(&entries[19..length])
&& let Ok(name) = name.to_str()
&& let Ok(fd) = name.parse::<RawFd>()
{
// A failure on one descriptor must not prevent cleanup of
// the remaining descriptors.
mark_cloexec(fd, preserved_fds);
}
entries = &entries[length..];
}
}
}
#[cfg(test)]
#[path = "linux_fds_tests.rs"]
mod tests;
+246
View File
@@ -0,0 +1,246 @@
//! Child-visible descriptor inheritance, fallback compatibility, and launch errors.
use std::io;
use std::io::Write;
use std::os::fd::AsRawFd;
use std::os::fd::FromRawFd;
use std::os::fd::OwnedFd;
use std::os::unix::process::CommandExt;
use std::os::unix::process::ExitStatusExt;
use std::process::Command;
use std::process::ExitStatus;
use std::process::Output;
use std::process::Stdio;
use anyhow::Context;
use pretty_assertions::assert_eq;
// Called only from pre_exec: reject one syscall in the child without changing
// the test runner's permissions or allocating memory after fork.
fn deny_syscall(syscall_number: libc::c_long) -> io::Result<()> {
let mut filter = [
libc::sock_filter {
code: (libc::BPF_LD | libc::BPF_W | libc::BPF_ABS) as _,
jt: 0,
jf: 0,
k: std::mem::offset_of!(libc::seccomp_data, nr) as _,
},
libc::sock_filter {
code: (libc::BPF_JMP | libc::BPF_JEQ | libc::BPF_K) as _,
jt: 0,
jf: 1,
k: syscall_number as _,
},
libc::sock_filter {
code: (libc::BPF_RET | libc::BPF_K) as _,
jt: 0,
jf: 0,
k: libc::SECCOMP_RET_ERRNO | libc::EPERM as u32,
},
libc::sock_filter {
code: (libc::BPF_RET | libc::BPF_K) as _,
jt: 0,
jf: 0,
k: libc::SECCOMP_RET_ALLOW,
},
];
let program = libc::sock_fprog {
len: filter.len() as _,
filter: filter.as_mut_ptr(),
};
// SAFETY: The kernel copies the stack-owned filter before prctl returns.
unsafe {
if libc::prctl(
libc::PR_SET_NO_NEW_PRIVS,
1_usize,
0_usize,
0_usize,
0_usize,
) == -1
|| libc::prctl(
libc::PR_SET_SECCOMP,
libc::SECCOMP_MODE_FILTER as usize,
&raw const program,
0_usize,
0_usize,
) == -1
{
return Err(io::Error::last_os_error());
}
// Qualify failure injection. Invalid arguments make the probe harmless
// for close_range and getdents64 even if the filter is ineffective.
if libc::syscall(syscall_number, -1_isize, 0_usize, 0_usize) != -1
|| io::Error::last_os_error().raw_os_error() != Some(libc::EPERM)
{
return Err(io::Error::from_raw_os_error(libc::EIO));
}
}
Ok(())
}
fn output_with_input(command: &mut Command) -> io::Result<Output> {
let mut child = command
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()?;
child
.stdin
.take()
.expect("piped stdin")
.write_all(b"stdio payload\n")?;
child.wait_with_output()
}
#[test]
fn cleanup_excludes_unrelated_fds_and_preserves_explicit_fds_and_stdio() -> anyhow::Result<()> {
let python = crate::tests::find_python().context("descriptor tests require Python")?;
let file = tempfile::tempfile()?;
// Model the occupied descriptor table of a parallel test runner.
let _occupied: Vec<_> = (0..64)
.map(|_| file.try_clone())
.collect::<io::Result<_>>()?;
// SAFETY: Reserve a low descriptor without replacing another owner.
let raw = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_DUPFD_CLOEXEC, 3) };
anyhow::ensure!(raw != -1, "{}", io::Error::last_os_error());
// SAFETY: fcntl returned a new descriptor owned by this test.
let reservation = unsafe { OwnedFd::from_raw_fd(raw) };
let reserved_fd = reservation.as_raw_fd();
// Keep every sentinel above the limit even when the parent's low slots fill.
let minimum = (reserved_fd + 1).max(/*other*/ 200);
let mut descriptors = Vec::new();
for operation in [libc::F_DUPFD, libc::F_DUPFD, libc::F_DUPFD_CLOEXEC] {
// SAFETY: fcntl reserves a new high descriptor without replacing any existing owner.
let fd = unsafe { libc::fcntl(file.as_raw_fd(), operation, minimum) };
anyhow::ensure!(fd != -1, "{}", io::Error::last_os_error());
// SAFETY: The duplicate is newly owned by this test.
descriptors.push(unsafe { OwnedFd::from_raw_fd(fd) });
}
let fds: Vec<_> = descriptors.iter().map(AsRawFd::as_raw_fd).collect();
let command = || {
let mut command = Command::new(&python);
command
.arg("-c")
.arg(
r#"import errno, os, sys
inherited = []
for fd in map(int, sys.argv[1:]):
try:
os.fstat(fd)
inherited.append(True)
except OSError as error:
if error.errno != errno.EBADF: raise
inherited.append(False)
print(inherited)
print(sys.stdin.read(), end="")
print("stderr preserved", file=sys.stderr)
"#,
)
.args(fds.iter().map(ToString::to_string));
command
};
let expected = |inherited| Output {
status: ExitStatus::from_raw(/*raw*/ 0),
stdout: format!("{inherited}\nstdio payload\n").into_bytes(),
stderr: b"stderr preserved\n".to_vec(),
};
// Qualify the oracle: the executable itself must not sanitize inherited FDs.
assert_eq!(
output_with_input(&mut command())?,
expected("[True, True, False]")
);
for (name, force_fallback) in [("default", false), ("fallback above fd limit", true)] {
for preserved in [&[][..], &[fds[2], fds[1], fds[2]][..]] {
// Unsorted and repeated entries must preserve the same inheritance.
let allowlist = preserved.to_vec();
let mut command = command();
// SAFETY: The hook uses only preallocated data and fork-safe syscalls.
unsafe {
command.pre_exec(move || {
if force_fallback {
deny_syscall(libc::SYS_close_range)?;
// Leave a free slot for procfs below the lowered limit,
// even if parallel tests filled every other low slot.
let mut limit = libc::rlimit {
rlim_cur: 0,
rlim_max: 0,
};
if libc::close(reserved_fd) == -1
|| libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit) == -1
{
return Err(io::Error::last_os_error());
}
limit.rlim_cur = reserved_fd as libc::rlim_t + 1;
if libc::setrlimit(libc::RLIMIT_NOFILE, &limit) == -1 {
return Err(io::Error::last_os_error());
}
}
crate::pty::close_inherited_fds_except(&allowlist);
Ok(())
});
}
let inheritance = if preserved.is_empty() {
"[False, False, False]"
} else {
"[False, True, False]"
};
assert_eq!(
output_with_input(&mut command)
.with_context(|| format!("{name}, allowlist {preserved:?}"))?,
expected(inheritance),
"{name}, allowlist {preserved:?}"
);
}
}
// The child must not mutate the parent's descriptors or inheritance flags.
let flags: Vec<_> = fds
.iter()
.map(|&fd| unsafe { libc::fcntl(fd, libc::F_GETFD) })
.collect();
assert_eq!(flags, [0, 0, libc::FD_CLOEXEC]);
Ok(())
}
#[test]
fn cleanup_fallback_preserves_exec_failure_reporting() -> anyhow::Result<()> {
let temporary = tempfile::tempdir()?;
let mut command = Command::new(temporary.path().join("does-not-exist"));
// SAFETY: The hook uses only stack storage and fork-safe syscalls in the child.
unsafe {
command.pre_exec(|| {
deny_syscall(libc::SYS_close_range)?;
crate::pty::close_inherited_fds_except(&[]);
Ok(())
});
}
let error = match command.spawn() {
Err(error) => error,
Ok(mut child) => {
child.wait()?;
anyhow::bail!("exec failure reported as success");
}
};
assert_eq!(error.raw_os_error(), Some(libc::ENOENT));
Ok(())
}
#[tokio::test]
async fn cleanup_failures_do_not_prevent_explicit_launch() -> anyhow::Result<()> {
let mut command = crate::Command::new("/bin/sh");
command
.args(["-c", "exit 42"])
.descriptor_policy(crate::DescriptorPolicy::Explicit);
// SAFETY: The hook only installs child-local syscall filters using stack
// storage. Command::spawn registers its real cleanup hook after this hook.
unsafe {
command.inner.pre_exec(|| {
deny_syscall(libc::SYS_close_range)?;
deny_syscall(libc::SYS_getdents64)
});
}
assert_eq!(
command.spawn()?.wait().await?,
ExitStatus::from_raw(/*raw*/ 42 << 8)
);
Ok(())
}
+6 -2
View File
@@ -29,6 +29,8 @@ use tokio::sync::mpsc;
use tokio::sync::oneshot;
use tokio::task::JoinHandle;
#[cfg(target_os = "linux")]
pub(crate) use crate::linux_fds::close_inherited_fds_except;
use crate::process::ChildTerminator;
use crate::process::ProcessHandle;
use crate::process::ProcessSignal;
@@ -492,6 +494,7 @@ fn set_cloexec(fd: RawFd) -> std::io::Result<()> {
}
// macOS needs a fork-safe sweep because recvmsg cannot set close-on-exec.
// Cleanup is best-effort; enumeration and close failures do not prevent launch.
#[cfg(target_os = "macos")]
pub fn close_inherited_fds_except(preserved_fds: &[RawFd]) {
let mut descriptors = [libc::proc_fdinfo {
@@ -561,8 +564,9 @@ pub fn close_inherited_fds_except(preserved_fds: &[RawFd]) {
}
}
// Other Unix platforms keep their existing fd cleanup.
#[cfg(all(unix, not(target_os = "macos")))]
// 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"))))]
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();