iHore/vLLM
8
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[Bugfix] Eliminate frontend ZMQ port TOCTOU with inherited listeners (#54113)

Browse Source 
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Bugen Zhao
2026-10-08 06:25:27 +00:00
committed by GitHub
co-authored by OpenAI Codex Claude Opus 5.5
parent fb2ac82458
commit f226af0e11
18 changed files with 790 additions and 585 deletions
Download Patch File Download Diff File Expand all files Collapse all files
rust/src/engine-core-client/Cargo.toml
+2
View File
@@ -18,6 +18,7 @@ futures.workspace = true
half.workspace = true
hex.workspace = true
itertools.workspace = true
libc.workspace = true
parking_lot.workspace = true
rmp-serde.workspace = true
rmpv.workspace = true
@@ -27,6 +28,7 @@ serde_json.workspace = true
serde_repr.workspace = true
serde_tuple.workspace = true
serde_with.workspace = true
socket2.workspace = true
task-local.workspace = true
tempfile = { workspace = true, optional = true }
thiserror.workspace = true
rust/src/engine-core-client/src/client.rs
+12 -13
View File
@@ -53,16 +53,15 @@ pub enum TransportMode {
local_output_address: Option<String>,
},
/// The Python supervisor has already chosen the frontend transport
/// addresses, and the Rust process only needs to bind them and wait for
/// engine registration frames.
/// The Python supervisor has already bound the frontend transport
/// listeners. Rust adopts them and waits for engine registration frames.
/// `EngineCoreClient::connect` consumes each descriptor exactly once;
/// copied integer values do not create additional descriptor ownership.
Bootstrapped {
/// Input ROUTER socket address that engines will connect to for
/// requests.
input_address: String,
/// Output PULL socket address that engines will connect to for
/// responses.
output_address: String,
/// Raw input ROUTER listener descriptor inherited from the supervisor.
input_listener_fd: i32,
/// Raw output PULL listener descriptor inherited from the supervisor.
output_listener_fd: i32,
/// First data-parallel engine rank expected to register on this
/// transport.
engine_start_index: u32,
@@ -336,8 +335,8 @@ impl EngineCoreClient {
}
TransportMode::Bootstrapped {
input_address,
output_address,
input_listener_fd,
output_listener_fd,
engine_start_index,
engine_count,
ready_timeout,
@@ -348,8 +347,8 @@ impl EngineCoreClient {
}
transport::connect_bootstrapped(
input_address,
output_address,
*input_listener_fd,
*output_listener_fd,
*engine_start_index,
*engine_count,
*ready_timeout,
rust/src/engine-core-client/src/tests/client.rs
+27 -11
View File
@@ -4,6 +4,8 @@
use std::collections::{BTreeMap, BTreeSet};
use std::convert::TryFrom;
use std::io::Cursor;
use std::os::fd::IntoRawFd;
use std::os::unix::net::UnixListener;
use std::path::PathBuf;
use std::process::Command;
use std::sync::Once;
@@ -311,10 +313,17 @@ fn bootstrapped_test_config(
client_index: u32,
coordinator_mode: Option<CoordinatorMode>,
) -> EngineCoreClientConfig {
fn listener(address: &str) -> i32 {
let path = address.strip_prefix("ipc://").expect("bootstrapped tests use IPC listeners");
let _ = std::fs::remove_file(path);
let listener = UnixListener::bind(path).expect("bind inherited test listener");
listener.into_raw_fd()
}
EngineCoreClientConfig {
transport_mode: TransportMode::Bootstrapped {
input_address,
output_address,
input_listener_fd: listener(&input_address),
output_listener_fd: listener(&output_address),
engine_start_index: 0,
engine_count,
data_parallel_size: engine_count,
@@ -362,15 +371,22 @@ fn bootstrapped_test_config_with_start_index(
#[test]
fn client_config_validates_bootstrapped_dp_range() {
let mut config = bootstrapped_test_config_with_start_index(
"ipc://unused-input".to_string(),
"ipc://unused-output".to_string(),
1,
2,
Duration::from_secs(1),
0,
None,
);
// Validation never consumes the descriptors, so placeholders avoid opening
// listeners that nothing would close.
let mut config = EngineCoreClientConfig {
transport_mode: TransportMode::Bootstrapped {
input_listener_fd: -1,
output_listener_fd: -1,
engine_start_index: 1,
engine_count: 2,
data_parallel_size: 3,
ready_timeout: Duration::from_secs(1),
},
coordinator_mode: None,
model_name: "test-model".to_string(),
client_index: 0,
engine_stats_enabled: true,
};
config.validate().expect("frontend may own a subset of global DP ranks");
let TransportMode::Bootstrapped {
rust/src/engine-core-client/src/transport.rs
+63 -5
View File
@@ -4,6 +4,8 @@
use std::collections::{BTreeMap, BTreeSet};
use std::fmt::Debug;
use std::ops::Deref;
use std::os::fd::{FromRawFd, OwnedFd, RawFd};
use std::os::unix::net::UnixListener as StdUnixListener;
use std::time::Duration;
use bytes::Bytes;
@@ -327,8 +329,8 @@ pub async fn connect_handshake(
/// identities are synthesized from contiguous rank order instead of being
/// discovered through a Rust-owned handshake.
pub async fn connect_bootstrapped(
input_address: &str,
output_address: &str,
input_listener_fd: RawFd,
output_listener_fd: RawFd,
engine_start_index: u32,
engine_count: usize,
ready_timeout: Duration,
@@ -354,10 +356,16 @@ pub async fn connect_bootstrapped(
}
let mut input_socket = RouterSocket::new();
let input_address = input_socket.bind(input_address).await?.to_string();
let input_address = input_socket
.bind_listener(adopt_listener(input_listener_fd)?)
.await?
.to_string();
let mut output_socket = PullSocket::new();
let output_address = output_socket.bind(output_address).await?.to_string();
let output_address = output_socket
.bind_listener(adopt_listener(output_listener_fd)?)
.await?
.to_string();
let engines = wait_for_input_registrations(
&mut input_socket,
@@ -385,6 +393,34 @@ pub async fn connect_bootstrapped(
})
}
fn adopt_listener(fd: RawFd) -> Result<zeromq::Listener> {
if fd < 0 || unsafe { libc::fcntl(fd, libc::F_GETFD) } < 0 {
return Err(Error::InvalidClientConfig {
message: format!("inherited ZMQ listener fd {fd} is not open"),
});
}
// SAFETY: Python's Popen(pass_fds) gives the frontend its own descriptor.
// The bootstrapped startup path calls this function once per descriptor,
// transferring ownership to the listener constructed below.
let fd = unsafe { OwnedFd::from_raw_fd(fd) };
let socket: socket2::Socket = fd.into();
let domain = socket.domain()?;
if domain == socket2::Domain::IPV4 || domain == socket2::Domain::IPV6 {
let listener: std::net::TcpListener = socket.into();
return Ok(listener.into());
}
if domain == socket2::Domain::UNIX {
let listener: StdUnixListener = socket.into();
return Ok(listener.into());
}
Err(Error::InvalidClientConfig {
message: format!("unsupported inherited listener domain {domain:?}"),
})
}
/// Bind new input and output sockets.
async fn bind_local_sockets(
local_host: &str,
@@ -603,7 +639,12 @@ pub async fn run_output_loop(
#[cfg(test)]
mod tests {
use super::bind_local_sockets;
use std::os::fd::IntoRawFd;
use zeromq::RouterSocket;
use zeromq::prelude::Socket;
use super::{adopt_listener, bind_local_sockets};
#[tokio::test]
async fn bind_local_sockets_resolves_zero_port_bindings() {
@@ -614,4 +655,21 @@ mod tests {
assert!(output_address.starts_with("tcp://127.0.0.1:"));
assert_ne!(input_address, output_address);
}
#[tokio::test]
async fn inherited_tcp_listener_is_adopted_without_rebinding() {
let listener = std::net::TcpListener::bind(("127.0.0.1", 0)).expect("bind listener");
let expected = listener.local_addr().expect("listener address");
let mut router = RouterSocket::new();
let endpoint = router
.bind_listener(
adopt_listener(listener.into_raw_fd()).expect("adopt inherited listener fd"),
)
.await
.expect("bind inherited listener");
assert_eq!(endpoint.to_string(), format!("tcp://{expected}"));
assert!(router.close().await.is_empty());
}
}