From 8177dd6f31a9ed5da576441cb5681c901623f14a Mon Sep 17 00:00:00 2001 From: Max Leonard Inden Date: Mon, 5 May 2025 15:24:22 +0000 Subject: [PATCH] Bug 1963796 - update Rust quinn-udp crate to v0.5.12 r=kershaw,supply-chain-reviewers Differential Revision: https://phabricator.services.mozilla.com/D247581 --- Cargo.lock | 4 +- supply-chain/audits.toml | 5 + .../rust/quinn-udp/.cargo-checksum.json | 2 +- third_party/rust/quinn-udp/Cargo.lock | 26 +-- third_party/rust/quinn-udp/Cargo.toml | 2 +- third_party/rust/quinn-udp/src/cmsg/mod.rs | 11 + third_party/rust/quinn-udp/src/cmsg/unix.rs | 13 +- third_party/rust/quinn-udp/src/unix.rs | 219 +++++++++--------- third_party/rust/quinn-udp/tests/tests.rs | 23 ++ 9 files changed, 167 insertions(+), 138 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a5e3eef84861..8725e5f58809 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5405,9 +5405,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" [[package]] name = "quinn-udp" -version = "0.5.11" +version = "0.5.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "541d0f57c6ec747a90738a52741d3221f7960e8ac2f0ff4b1a63680e033b4ab5" +checksum = "ee4e529991f949c5e25755532370b8af5d114acae52326361d68d47af64aa842" dependencies = [ "cfg_aliases", "libc", diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml index 59c86db8c7f4..61b0187b0427 100644 --- a/supply-chain/audits.toml +++ b/supply-chain/audits.toml @@ -4348,6 +4348,11 @@ who = "Max Leonard Inden " criteria = "safe-to-deploy" delta = "0.5.10 -> 0.5.11" +[[audits.quinn-udp]] +who = "Max Leonard Inden " +criteria = "safe-to-deploy" +delta = "0.5.11 -> 0.5.12" + [[audits.quote]] who = "Nika Layzell " criteria = "safe-to-deploy" diff --git a/third_party/rust/quinn-udp/.cargo-checksum.json b/third_party/rust/quinn-udp/.cargo-checksum.json index 13c8ebe538e1..2ada89f12d0c 100644 --- a/third_party/rust/quinn-udp/.cargo-checksum.json +++ b/third_party/rust/quinn-udp/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.lock":"88cdec3b9b3cb594afa1667a630573da210dfb67edb677c86e6437428d4191eb","Cargo.toml":"2ae44e1d3aa82ee7b91aa4e2b7cea54ac52113944c586b15d515d631a0cf7c80","LICENSE-APACHE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","LICENSE-MIT":"4b2d0aca6789fa39e03d6738e869ea0988cceba210ca34ebb59c15c463e93a04","benches/throughput.rs":"86cb85d2ae07169da8c279861c53b7a055168aaaa91155576c633b8724748db6","build.rs":"1d7ecadda4a26fb0eba598789eef9b99a1b4febba9bcb61a34f0c92b1d1bbaeb","src/cmsg/mod.rs":"63d6ea7126341fededdaef14260a7eed715ad3f507d4da586dbab814f581a54d","src/cmsg/unix.rs":"b13d36e757eef3038303e9429573ca29c31aff406592514c304bba73e1bff22f","src/cmsg/windows.rs":"6fb936ec4a283efc5796872e777441e3039c40589073865644a8ef7936af4f4b","src/fallback.rs":"1e59ea16c6e1487bbb6aa759e349000431474aa245960512cb3b5117a1ed9e21","src/lib.rs":"77d48436bbfcccaea8dc3f061acc874ef5089148bf1700fc7a61b1b3d1b575e1","src/unix.rs":"61ebd3e7e4bbff8197258484bd61b9cacef29a5e47b870dd4f2fa9a9e25f1949","src/windows.rs":"43da25457cb17c61369c3ba2c1d98f0ff758c5ea3207ae22969cca1f620b54af","tests/tests.rs":"2b16e546866567b13a6e17d94e84a6b9cdf53e00f1d8ee8f59d080389590e5b0"},"package":"541d0f57c6ec747a90738a52741d3221f7960e8ac2f0ff4b1a63680e033b4ab5"} \ No newline at end of file +{"files":{"Cargo.lock":"b6db9f61ff4fdb22fb4a928df627f66d2a12b699476b244ea5260e010d8c2ae1","Cargo.toml":"397318dc0e80f559c5f570a71e5497fd2a5ab1b4daab1f365d094f1612198968","LICENSE-APACHE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","LICENSE-MIT":"4b2d0aca6789fa39e03d6738e869ea0988cceba210ca34ebb59c15c463e93a04","benches/throughput.rs":"86cb85d2ae07169da8c279861c53b7a055168aaaa91155576c633b8724748db6","build.rs":"1d7ecadda4a26fb0eba598789eef9b99a1b4febba9bcb61a34f0c92b1d1bbaeb","src/cmsg/mod.rs":"ccf970026c8578b1c4661fbe106093dfb62b084a231ecbb4c62eaa10df5822fe","src/cmsg/unix.rs":"7917bce2f3c8e844eca2e4cfea82669b2a31cf311321dc42532626db4ee42de8","src/cmsg/windows.rs":"6fb936ec4a283efc5796872e777441e3039c40589073865644a8ef7936af4f4b","src/fallback.rs":"1e59ea16c6e1487bbb6aa759e349000431474aa245960512cb3b5117a1ed9e21","src/lib.rs":"77d48436bbfcccaea8dc3f061acc874ef5089148bf1700fc7a61b1b3d1b575e1","src/unix.rs":"ae3cc0de15c0ec03b4aaa108a69406ee62d3b57abf5226ccd8f8e66b85c95d2d","src/windows.rs":"43da25457cb17c61369c3ba2c1d98f0ff758c5ea3207ae22969cca1f620b54af","tests/tests.rs":"bd4ee24b0e1ccab9beb444541b472bc1e815e2aba19d75572a379b6e1533449c"},"package":"ee4e529991f949c5e25755532370b8af5d114acae52326361d68d47af64aa842"} \ No newline at end of file diff --git a/third_party/rust/quinn-udp/Cargo.lock b/third_party/rust/quinn-udp/Cargo.lock index 1fc01283db09..143f2e7912a4 100644 --- a/third_party/rust/quinn-udp/Cargo.lock +++ b/third_party/rust/quinn-udp/Cargo.lock @@ -106,18 +106,18 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.32" +version = "4.5.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6088f3ae8c3608d19260cd7445411865a485688711b78b5be70d78cd96136f83" +checksum = "d8aa86934b44c19c50f87cc2790e19f54f7a67aedb64101c2e1a2e5ecfb73944" dependencies = [ "clap_builder", ] [[package]] name = "clap_builder" -version = "4.5.32" +version = "4.5.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22a7ef7f676155edfb82daa97f99441f3ebf4a58d5e32f295a56259f1b6facc8" +checksum = "2414dbb2dd0695280da6ea9261e327479e9d37b0630f6b53ba2a11c60c679fd9" dependencies = [ "anstyle", "clap_lex", @@ -306,9 +306,9 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "miniz_oxide" -version = "0.8.5" +version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e3e04debbb59698c15bacbb6d93584a8c0ca9cc3213cb423d31f760d8843ce5" +checksum = "ff70ce3e48ae43fa075863cef62e8b43b71a4f2382229920e0df362592919430" dependencies = [ "adler2", ] @@ -344,9 +344,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.21.1" +version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d75b0bedcc4fe52caa0e03d9f1151a323e4aa5e2d78ba3580400cd3c9e2bc4bc" +checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" [[package]] name = "oorandom" @@ -377,7 +377,7 @@ dependencies = [ [[package]] name = "quinn-udp" -version = "0.5.11" +version = "0.5.12" dependencies = [ "cfg_aliases", "criterion", @@ -483,9 +483,9 @@ dependencies = [ [[package]] name = "socket2" -version = "0.5.8" +version = "0.5.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c970269d99b64e60ec3bd6ad27270092a5394c4e309314b18ae3fe575695fbe8" +checksum = "4f5fd57c80058a56cf5c777ab8a126398ece8e442983605d280a44ce79d0edef" dependencies = [ "libc", "windows-sys 0.52.0", @@ -514,9 +514,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.44.1" +version = "1.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f382da615b842244d4b8738c82ed1275e6c5dd90c459a30941cd07080b06c91a" +checksum = "e6b88822cbe49de4185e3a4cbf8321dd487cf5fe0c5c65695fef6346371e9c48" dependencies = [ "backtrace", "libc", diff --git a/third_party/rust/quinn-udp/Cargo.toml b/third_party/rust/quinn-udp/Cargo.toml index af6001757fb2..a8d2bafb8f0e 100644 --- a/third_party/rust/quinn-udp/Cargo.toml +++ b/third_party/rust/quinn-udp/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.71" name = "quinn-udp" -version = "0.5.11" +version = "0.5.12" build = "build.rs" autolib = false autobins = false diff --git a/third_party/rust/quinn-udp/src/cmsg/mod.rs b/third_party/rust/quinn-udp/src/cmsg/mod.rs index 4a1c90e2228d..3b98be509a46 100644 --- a/third_party/rust/quinn-udp/src/cmsg/mod.rs +++ b/third_party/rust/quinn-udp/src/cmsg/mod.rs @@ -112,6 +112,17 @@ impl<'a, M: MsgHdr> Iterator for Iter<'a, M> { fn next(&mut self) -> Option { let current = self.cmsg.take()?; self.cmsg = unsafe { self.hdr.cmsg_nxt_hdr(current).as_ref() }; + + #[cfg(apple_fast)] + { + // On MacOS < 14 CMSG_NXTHDR might continuously return a zeroed cmsg. In + // such case, return `None` instead, thus indicating the end of + // the cmsghdr chain. + if current.len() < mem::size_of::() { + return None; + } + } + Some(current) } } diff --git a/third_party/rust/quinn-udp/src/cmsg/unix.rs b/third_party/rust/quinn-udp/src/cmsg/unix.rs index 69b01756e02c..93ac76ba8016 100644 --- a/third_party/rust/quinn-udp/src/cmsg/unix.rs +++ b/third_party/rust/quinn-udp/src/cmsg/unix.rs @@ -43,18 +43,7 @@ impl MsgHdr for crate::imp::msghdr_x { fn cmsg_nxt_hdr(&self, cmsg: &Self::ControlMessage) -> *mut Self::ControlMessage { let selfp = self as *const _ as *mut libc::msghdr; - let next = unsafe { libc::CMSG_NXTHDR(selfp, cmsg) }; - - // On MacOS < 14 CMSG_NXTHDR might continuously return a zeroed cmsg. In - // such case, return a null pointer instead, thus indicating the end of - // the cmsghdr chain. - if unsafe { next.as_ref() } - .is_some_and(|n| (n.cmsg_len as usize) < std::mem::size_of::()) - { - return std::ptr::null_mut(); - } - - next + unsafe { libc::CMSG_NXTHDR(selfp, cmsg) } } fn set_control_len(&mut self, len: usize) { diff --git a/third_party/rust/quinn-udp/src/unix.rs b/third_party/rust/quinn-udp/src/unix.rs index d6a0345dcfef..c89279691172 100644 --- a/third_party/rust/quinn-udp/src/unix.rs +++ b/third_party/rust/quinn-udp/src/unix.rs @@ -208,6 +208,9 @@ impl UdpSocketState { match send(self, socket.0, transmit) { Ok(()) => Ok(()), Err(e) if e.kind() == io::ErrorKind::WouldBlock => Err(e), + // - EMSGSIZE is expected for MTU probes. Future work might be able to avoid + // these by automatically clamping the MTUD upper bound to the interface MTU. + Err(e) if e.raw_os_error() == Some(libc::EMSGSIZE) => Ok(()), Err(e) => { log_sendmsg_error(&self.last_send_error, e, transmit); @@ -328,57 +331,53 @@ fn send( loop { let n = unsafe { libc::sendmsg(io.as_raw_fd(), &msg_hdr, 0) }; - if n == -1 { - let e = io::Error::last_os_error(); - match e.kind() { - io::ErrorKind::Interrupted => { - // Retry the transmission + + if n >= 0 { + return Ok(()); + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry the transmission + io::ErrorKind::Interrupted => continue, + io::ErrorKind::WouldBlock => return Err(e), + _ => { + // Some network adapters and drivers do not support GSO. Unfortunately, Linux + // offers no easy way for us to detect this short of an EIO or sometimes EINVAL + // when we try to actually send datagrams using it. + #[cfg(any(target_os = "linux", target_os = "android"))] + if let Some(libc::EIO) | Some(libc::EINVAL) = e.raw_os_error() { + // Prevent new transmits from being scheduled using GSO. Existing GSO transmits + // may already be in the pipeline, so we need to tolerate additional failures. + if state.max_gso_segments() > 1 { + crate::log::info!( + "`libc::sendmsg` failed with {e}; halting segmentation offload" + ); + state + .max_gso_segments + .store(1, std::sync::atomic::Ordering::Relaxed); + } + } + + // Some arguments to `sendmsg` are not supported. Switch to + // fallback mode and retry if we haven't already. + if e.raw_os_error() == Some(libc::EINVAL) && !state.sendmsg_einval() { + state.set_sendmsg_einval(); + prepare_msg( + transmit, + &dst_addr, + &mut msg_hdr, + &mut iovec, + &mut cmsgs, + encode_src_ip, + state.sendmsg_einval(), + ); continue; } - io::ErrorKind::WouldBlock => return Err(e), - _ => { - // Some network adapters and drivers do not support GSO. Unfortunately, Linux - // offers no easy way for us to detect this short of an EIO or sometimes EINVAL - // when we try to actually send datagrams using it. - #[cfg(any(target_os = "linux", target_os = "android"))] - if let Some(libc::EIO) | Some(libc::EINVAL) = e.raw_os_error() { - // Prevent new transmits from being scheduled using GSO. Existing GSO transmits - // may already be in the pipeline, so we need to tolerate additional failures. - if state.max_gso_segments() > 1 { - crate::log::info!( - "`libc::sendmsg` failed with {e}; halting segmentation offload" - ); - state - .max_gso_segments - .store(1, std::sync::atomic::Ordering::Relaxed); - } - } - // Some arguments to `sendmsg` are not supported. Switch to - // fallback mode and retry if we haven't already. - if e.raw_os_error() == Some(libc::EINVAL) && !state.sendmsg_einval() { - state.set_sendmsg_einval(); - prepare_msg( - transmit, - &dst_addr, - &mut msg_hdr, - &mut iovec, - &mut cmsgs, - encode_src_ip, - state.sendmsg_einval(), - ); - continue; - } - - // - EMSGSIZE is expected for MTU probes. Future work might be able to avoid - // these by automatically clamping the MTUD upper bound to the interface MTU. - if e.raw_os_error() != Some(libc::EMSGSIZE) { - return Err(e); - } - } + return Err(e); } } - return Ok(()); } } @@ -417,24 +416,17 @@ fn send(state: &UdpSocketState, io: SockRef<'_>, transmit: &Transmit<'_>) -> io: } loop { let n = unsafe { sendmsg_x(io.as_raw_fd(), hdrs.as_ptr(), cnt as u32, 0) }; - if n == -1 { - let e = io::Error::last_os_error(); - match e.kind() { - io::ErrorKind::Interrupted => { - // Retry the transmission - continue; - } - io::ErrorKind::WouldBlock => return Err(e), - _ => { - // - EMSGSIZE is expected for MTU probes. Future work might be able to avoid - // these by automatically clamping the MTUD upper bound to the interface MTU. - if e.raw_os_error() != Some(libc::EMSGSIZE) { - return Err(e); - } - } - } + + if n >= 0 { + return Ok(()); + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry the transmission + io::ErrorKind::Interrupted => continue, + _ => return Err(e), } - return Ok(()); } } @@ -455,24 +447,17 @@ fn send(state: &UdpSocketState, io: SockRef<'_>, transmit: &Transmit<'_>) -> io: ); loop { let n = unsafe { libc::sendmsg(io.as_raw_fd(), &hdr, 0) }; - if n == -1 { - let e = io::Error::last_os_error(); - match e.kind() { - io::ErrorKind::Interrupted => { - // Retry the transmission - continue; - } - io::ErrorKind::WouldBlock => return Err(e), - _ => { - // - EMSGSIZE is expected for MTU probes. Future work might be able to avoid - // these by automatically clamping the MTUD upper bound to the interface MTU. - if e.raw_os_error() != Some(libc::EMSGSIZE) { - return Err(e); - } - } - } + + if n >= 0 { + return Ok(()); + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry the transmission + io::ErrorKind::Interrupted => continue, + _ => return Err(e), } - return Ok(()); } } @@ -500,14 +485,17 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) -> ptr::null_mut::(), ) }; - if n == -1 { - let e = io::Error::last_os_error(); - if e.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(e); + + if n >= 0 { + break n; + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry receiving + io::ErrorKind::Interrupted => continue, + _ => return Err(e), } - break n; }; for i in 0..(msg_count as usize) { meta[i] = decode_recv(&names[i], &hdrs[i].msg_hdr, hdrs[i].msg_len as usize); @@ -518,7 +506,13 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) -> #[cfg(apple_fast)] fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) -> io::Result { let mut names = [MaybeUninit::::uninit(); BATCH_SIZE]; - let mut ctrls = [cmsg::Aligned(MaybeUninit::<[u8; CMSG_LEN]>::uninit()); BATCH_SIZE]; + // MacOS 10.15 `recvmsg_x` does not override the `msghdr_x` + // `msg_controllen`. Thus, after the call to `recvmsg_x`, one does not know + // which control messages have been written to. To prevent reading + // uninitialized memory, do not use `MaybeUninit` for `ctrls`, instead + // initialize `ctrls` with `0`s. A control message of all `0`s is + // automatically skipped by `libc::CMSG_NXTHDR`. + let mut ctrls = [cmsg::Aligned([0u8; CMSG_LEN]); BATCH_SIZE]; let mut hdrs = unsafe { mem::zeroed::<[msghdr_x; BATCH_SIZE]>() }; let max_msg_count = bufs.len().min(BATCH_SIZE); for i in 0..max_msg_count { @@ -526,15 +520,16 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) -> } let msg_count = loop { let n = unsafe { recvmsg_x(io.as_raw_fd(), hdrs.as_mut_ptr(), max_msg_count as _, 0) }; - match n { - -1 => { - let e = io::Error::last_os_error(); - if e.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(e); - } - n => break n, + + if n >= 0 { + break n; + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry receiving + io::ErrorKind::Interrupted => continue, + _ => return Err(e), } }; for i in 0..(msg_count as usize) { @@ -551,17 +546,21 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) -> prepare_recv(&mut bufs[0], &mut name, &mut ctrl, &mut hdr); let n = loop { let n = unsafe { libc::recvmsg(io.as_raw_fd(), &mut hdr, 0) }; - if n == -1 { - let e = io::Error::last_os_error(); - if e.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(e); - } + if hdr.msg_flags & libc::MSG_TRUNC != 0 { continue; } - break n; + + if n >= 0 { + break n; + } + + let e = io::Error::last_os_error(); + match e.kind() { + // Retry receiving + io::ErrorKind::Interrupted => continue, + _ => return Err(e), + } }; meta[0] = decode_recv(&name, &hdr, n as usize); Ok(1) @@ -613,11 +612,13 @@ fn prepare_msg( encoder.push(libc::IPPROTO_IPV6, libc::IPV6_TCLASS, ecn); } - // Only set the segment size if it is different from the size of the contents. - // Some network drivers don't like being told to do GSO even if there is effectively only a single segment. + // Only set the segment size if it is less than the size of the contents. + // Some network drivers don't like being told to do GSO even if there is effectively only a single segment (i.e. `segment_size == transmit.contents.len()`) + // Additionally, a `segment_size` that is greater than the content also means there is effectively only a single segment. + // This case is actually quite common when splitting up a prepared GSO batch again after GSO has been disabled because the last datagram in a GSO batch is allowed to be smaller than the segment size. if let Some(segment_size) = transmit .segment_size - .filter(|segment_size| *segment_size != transmit.contents.len()) + .filter(|segment_size| *segment_size < transmit.contents.len()) { gso::set_segment_size(&mut encoder, segment_size as u16); } @@ -681,7 +682,7 @@ fn prepare_recv( fn prepare_recv( buf: &mut IoSliceMut, name: &mut MaybeUninit, - ctrl: &mut cmsg::Aligned>, + ctrl: &mut cmsg::Aligned<[u8; CMSG_LEN]>, hdr: &mut msghdr_x, ) { hdr.msg_name = name.as_mut_ptr() as _; diff --git a/third_party/rust/quinn-udp/tests/tests.rs b/third_party/rust/quinn-udp/tests/tests.rs index 96f8415b4afe..7d6de0655329 100644 --- a/third_party/rust/quinn-udp/tests/tests.rs +++ b/third_party/rust/quinn-udp/tests/tests.rs @@ -31,6 +31,29 @@ fn basic() { ); } +#[test] +fn basic_src_ip() { + let send = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0)) + .or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0))) + .unwrap(); + let recv = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0)) + .or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0))) + .unwrap(); + let src_ip = send.local_addr().unwrap().ip(); + let dst_addr = recv.local_addr().unwrap(); + test_send_recv( + &send.into(), + &recv.into(), + Transmit { + destination: dst_addr, + ecn: None, + contents: b"hello", + segment_size: None, + src_ip: Some(src_ip), + }, + ); +} + #[test] fn ecn_v6() { let send = Socket::from(UdpSocket::bind((Ipv6Addr::LOCALHOST, 0)).unwrap());