mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-06 03:28:24 +00:00
Set the UDP listen socket's reuse flags after its bind, not before
`UdpRawSocket::open` set SO_REUSEPORT and SO_REUSEADDR before binding, on the belief -- asserted in the comment there -- that the flags must precede the bind for the per-peer connected sockets to join the port later. That is true of the joining socket and false of the holder: a holder flagged after its own bind still admits the join. Before the bind, the flags mean something the listen socket never wanted. With outbound_only the bind is 0.0.0.0:0, and a flagged port-zero bind lets the kernel hand back a port another flagged socket already holds. On a configured port it is worse and needs no special posture: a second daemon binding the same address succeeded silently and shared the port, with the kernel splitting inbound datagrams across the two recv loops on the source 4-tuple, where it should have failed to start with EADDRINUSE. Move both calls after the bind and rewrite the comment that recorded the wrong belief. The connected-UDP fast path is unaffected: open_connected_fd is the joiner and keeps its flags before its own bind, which is where they belong. Cover the configured-port case with a test that opens a second socket on a port the first still holds and requires the bind to fail. It fails on the old ordering, where the second open succeeds. The port-zero duplicate is probabilistic and needs hundreds of sockets to show up, so it is left to the measurements in the issue rather than to a flaky test. (cherry picked from commit 7034841da5877f57c233845122bd507467249c14)
This commit is contained in:
@@ -3386,8 +3386,8 @@ async fn app_owned_udp_fd_seam_stays_silent_without_a_udp_transport() {
|
||||
}
|
||||
|
||||
/// A UDP transport that never bound has no fd to hand out. The bind address is
|
||||
/// deliberately unparseable — a busy port would not do it, since
|
||||
/// `UdpRawSocket::open` sets `SO_REUSEADDR`/`SO_REUSEPORT` before binding.
|
||||
/// deliberately unparseable, so the failure is in parsing and cannot depend on
|
||||
/// what else happens to hold a port while the suite runs.
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn app_owned_udp_fd_seam_stays_silent_when_the_udp_transport_fails_to_start() {
|
||||
|
||||
@@ -76,6 +76,29 @@ mod tests {
|
||||
assert!(send_buf > 0, "send buffer should be non-zero");
|
||||
}
|
||||
|
||||
/// The listen socket's reuse flags go on after its own bind, so they
|
||||
/// never license the kernel to hand this socket a port someone else
|
||||
/// holds. The visible consequence is that a second bind of an occupied
|
||||
/// port fails loudly instead of silently sharing it and splitting the
|
||||
/// inbound datagrams between the two recv loops.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn a_second_open_of_an_occupied_port_fails_instead_of_sharing_it() {
|
||||
let holder = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536)
|
||||
.expect("failed to bind the holding socket");
|
||||
let addr = holder.local_addr();
|
||||
|
||||
// `UdpRawSocket` is not `Debug`, so this cannot be `expect_err`.
|
||||
let Err(err) = UdpRawSocket::open(addr, 65536, 65536) else {
|
||||
panic!("the port is already held, so the second bind must fail");
|
||||
};
|
||||
|
||||
assert!(
|
||||
err.to_string().contains("bind failed"),
|
||||
"the failure must come from bind, not from a later step: {err}",
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_async_udp_socket_send_recv() {
|
||||
let sock1 = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536)
|
||||
|
||||
@@ -57,19 +57,27 @@ impl UdpRawSocket {
|
||||
sock.set_nonblocking(true)
|
||||
.map_err(|e| TransportError::StartFailed(format!("set nonblocking failed: {}", e)))?;
|
||||
|
||||
// SO_REUSEPORT lets per-peer `ConnectedPeerSocket`s bind
|
||||
// to the same wildcard port the listen socket holds. Must
|
||||
// be set BEFORE bind. Without this, the connected-UDP
|
||||
// activation handler fails with EADDRINUSE on Linux and
|
||||
// every outbound packet falls back to the wildcard listen
|
||||
// socket — losing the kernel 5-tuple cache benefit and
|
||||
// most of the multihop forwarding throughput gain.
|
||||
let _ = sock.set_reuse_port(true);
|
||||
let _ = sock.set_reuse_address(true);
|
||||
|
||||
sock.bind(&bind_addr.into())
|
||||
.map_err(|e| TransportError::StartFailed(format!("bind failed: {}", e)))?;
|
||||
|
||||
// SO_REUSEPORT lets per-peer `ConnectedPeerSocket`s bind to the same
|
||||
// port this listen socket holds; without it the connected-UDP
|
||||
// activation handler fails with EADDRINUSE on Linux and every
|
||||
// outbound packet falls back to the wildcard listen socket, losing
|
||||
// the kernel 5-tuple cache benefit and most of the multihop
|
||||
// forwarding throughput gain.
|
||||
//
|
||||
// Set AFTER bind, deliberately. The flags mean different things
|
||||
// either side of it: before bind, "the kernel may give me a port
|
||||
// another socket already holds", which for the `outbound_only`
|
||||
// `0.0.0.0:0` bind means duplicate ephemeral ports, and for a
|
||||
// configured port means a second daemon silently shares it instead
|
||||
// of failing to start; after bind, "another socket may later join my
|
||||
// port", which is the only half the fast path needs. A joiner still
|
||||
// binds successfully against a holder flagged after its own bind.
|
||||
let _ = sock.set_reuse_port(true);
|
||||
let _ = sock.set_reuse_address(true);
|
||||
|
||||
// Set socket buffer sizes
|
||||
sock.set_recv_buffer_size(recv_buf_size)
|
||||
.map_err(|e| TransportError::StartFailed(format!("set recv buffer: {}", e)))?;
|
||||
|
||||
Reference in New Issue
Block a user