Skip to content

Commit 19a625d

Browse files
thomaseizingergretchenfrage
authored andcommitted
fix(quinn-udp): move cmsg-len check to Iterator
In #2154, we added a piece of logic to `cmsg_nxt_hdr` that - within the `apple-fast-datapath` code - inspects the `cmsg_len` property and checks if we can actually fit another `cmsghdr` into it. This however only makes sense when we are _reading_ a buffer of cmsgs that has been initialised by the kernel. When we are _sending_ datagrams, the cmsg buffer is initialised with all zeros and for every cmsg we want to attach to a datagram, we advance the buffer using CMSG_NXTHDR. In this case, all the fields within `cmsghdr` are 0 initialised and it doesn't make sense to check their length. In fact, doing so results in a false-positive panic triggered by `Encoder::push` because it thinks that the cmsg buffer is too short to accomodate the message it wants to write. To fix this issue, we move the check to the `Iterator` implementation, guarded by the `apple_fast` cfg. This allows us to use the `cmsg_nxt_hdr` function for both reading and writing cmsgs.
1 parent 3f94660 commit 19a625d

File tree

3 files changed

+35
-12
lines changed

3 files changed

+35
-12
lines changed

quinn-udp/src/cmsg/mod.rs

+11
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,17 @@ impl<'a, M: MsgHdr> Iterator for Iter<'a, M> {
112112
fn next(&mut self) -> Option<Self::Item> {
113113
let current = self.cmsg.take()?;
114114
self.cmsg = unsafe { self.hdr.cmsg_nxt_hdr(current).as_ref() };
115+
116+
#[cfg(apple_fast)]
117+
{
118+
// On MacOS < 14 CMSG_NXTHDR might continuously return a zeroed cmsg. In
119+
// such case, return `None` instead, thus indicating the end of
120+
// the cmsghdr chain.
121+
if current.len() < mem::size_of::<M::ControlMessage>() {
122+
return None;
123+
}
124+
}
125+
115126
Some(current)
116127
}
117128
}

quinn-udp/src/cmsg/unix.rs

+1-12
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,7 @@ impl MsgHdr for crate::imp::msghdr_x {
4343

4444
fn cmsg_nxt_hdr(&self, cmsg: &Self::ControlMessage) -> *mut Self::ControlMessage {
4545
let selfp = self as *const _ as *mut libc::msghdr;
46-
let next = unsafe { libc::CMSG_NXTHDR(selfp, cmsg) };
47-
48-
// On MacOS < 14 CMSG_NXTHDR might continuously return a zeroed cmsg. In
49-
// such case, return a null pointer instead, thus indicating the end of
50-
// the cmsghdr chain.
51-
if unsafe { next.as_ref() }
52-
.is_some_and(|n| (n.cmsg_len as usize) < std::mem::size_of::<libc::cmsghdr>())
53-
{
54-
return std::ptr::null_mut();
55-
}
56-
57-
next
46+
unsafe { libc::CMSG_NXTHDR(selfp, cmsg) }
5847
}
5948

6049
fn set_control_len(&mut self, len: usize) {

quinn-udp/tests/tests.rs

+23
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,29 @@ fn basic() {
3131
);
3232
}
3333

34+
#[test]
35+
fn basic_src_ip() {
36+
let send = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
37+
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
38+
.unwrap();
39+
let recv = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
40+
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
41+
.unwrap();
42+
let src_ip = send.local_addr().unwrap().ip();
43+
let dst_addr = recv.local_addr().unwrap();
44+
test_send_recv(
45+
&send.into(),
46+
&recv.into(),
47+
Transmit {
48+
destination: dst_addr,
49+
ecn: None,
50+
contents: b"hello",
51+
segment_size: None,
52+
src_ip: Some(src_ip),
53+
},
54+
);
55+
}
56+
3457
#[test]
3558
fn ecn_v6() {
3659
let send = Socket::from(UdpSocket::bind((Ipv6Addr::LOCALHOST, 0)).unwrap());

0 commit comments

Comments
 (0)