diff --git a/src/net/send_recv/msg.rs b/src/net/send_recv/msg.rs index 234e9933a..6c8de947e 100644 --- a/src/net/send_recv/msg.rs +++ b/src/net/send_recv/msg.rs @@ -344,7 +344,7 @@ impl<'buf, 'slice, 'fd> SendAncillaryBuffer<'buf, 'slice, 'fd> { self.length = new_length; // Get the last header in the buffer. - let last_header = leap!(messages::Messages::new(buffer).last()); + let (last_header, _) = leap!(messages::Messages::new(buffer).last()); // Set the header fields. last_header.cmsg_len = unsafe { c::CMSG_LEN(source_len) } as _; @@ -533,34 +533,48 @@ impl<'buf> AncillaryDrain<'buf> { } } + /// `space` is the number of bytes of buffer space at and after `msg`. fn advance( read_and_length: &mut Option<(&'buf mut usize, &'buf mut usize)>, msg: &c::cmsghdr, + space: usize, ) -> Option> { + // Clamp the message length to the buffer. When `recvmsg` truncates + // control data to fit, Linux reduces `cmsg_len` to match what it + // wrote, but macOS leaves `cmsg_len` holding the untruncated length, + // so it can run past the end of the buffer. + let msg_len = (msg.cmsg_len as usize).min(space); + // Advance the `read` pointer. if let Some((read, length)) = read_and_length { - let msg_len = msg.cmsg_len as usize; **read += msg_len; **length -= msg_len; } - Self::cvt_msg(msg) + Self::cvt_msg(msg, msg_len) } /// A closure that converts a message into a [`RecvAncillaryMessage`]. - fn cvt_msg(msg: &c::cmsghdr) -> Option> { + fn cvt_msg(msg: &c::cmsghdr, msg_len: usize) -> Option> { unsafe { - // Get a pointer to the payload. + // Get a pointer to the payload. Use `msg_len` rather than + // `msg.cmsg_len`, as the message may have been truncated to fit + // in the buffer. If there isn't even a whole header, there's no + // message to report. let payload = c::CMSG_DATA(msg); - let payload_len = msg.cmsg_len as usize - c::CMSG_LEN(0) as usize; - - // Get a mutable slice of the payload. - let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len); + let payload_len = msg_len.checked_sub(c::CMSG_LEN(0) as usize)?; // Determine what type it is. let (level, msg_type) = (msg.cmsg_level, msg.cmsg_type); match (level as _, msg_type as _) { (c::SOL_SOCKET, c::SCM_RIGHTS) => { + // Truncation can leave a partial file descriptor at the + // end of the payload; round down to whole descriptors. + let payload_len = payload_len - payload_len % size_of::(); + + // Get a mutable slice of the payload. + let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len); + // Create an iterator that reads out the file descriptors. let fds = AncillaryIter::new(payload); @@ -569,7 +583,7 @@ impl<'buf> AncillaryDrain<'buf> { #[cfg(linux_kernel)] (c::SOL_SOCKET, c::SCM_CREDENTIALS) => { if payload_len >= size_of::() { - let ucred = payload.as_ptr().cast::().read_unaligned(); + let ucred = payload.cast::().read_unaligned(); Some(RecvAncillaryMessage::ScmCredentials(ucred)) } else { None @@ -586,7 +600,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> { fn next(&mut self) -> Option { self.messages - .find_map(|ev| Self::advance(&mut self.read_and_length, ev)) + .find_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space)) } fn size_hint(&self) -> (usize, Option) { @@ -600,13 +614,13 @@ impl<'buf> Iterator for AncillaryDrain<'buf> { F: FnMut(B, Self::Item) -> B, { self.messages - .filter_map(|ev| Self::advance(&mut self.read_and_length, ev)) + .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space)) .fold(init, f) } fn count(mut self) -> usize { self.messages - .filter_map(|ev| Self::advance(&mut self.read_and_length, ev)) + .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space)) .count() } @@ -615,7 +629,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> { Self: Sized, { self.messages - .filter_map(|ev| Self::advance(&mut self.read_and_length, ev)) + .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space)) .last() } @@ -624,7 +638,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> { Self: Sized, { self.messages - .filter_map(|ev| Self::advance(&mut self.read_and_length, ev)) + .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space)) .collect() } } @@ -951,13 +965,21 @@ mod messages { } impl<'a> Iterator for Messages<'a> { - type Item = &'a mut c::cmsghdr; + /// A message header, along with the number of bytes of buffer space + /// at and after it, which is an upper bound on the size of the + /// message. + type Item = (&'a mut c::cmsghdr, usize); #[inline] fn next(&mut self) -> Option { // Get the current header. let header = self.header?; + // Compute the number of bytes of buffer space at and after this + // header. + let end = (self.msghdr.msg_control as usize) + (self.msghdr.msg_controllen as usize); + let space = end.saturating_sub(header.as_ptr() as usize); + // Get the next header. self.header = NonNull::new(unsafe { c::CMSG_NXTHDR(&self.msghdr, header.as_ptr()) }); @@ -967,7 +989,7 @@ mod messages { } // SAFETY: The lifetime of `header` is tied to this. - Some(unsafe { &mut *header.as_ptr() }) + Some((unsafe { &mut *header.as_ptr() }, space)) } fn size_hint(&self) -> (usize, Option) { diff --git a/tests/net/cmsg.rs b/tests/net/cmsg.rs index 1c7bdb46a..44501a047 100644 --- a/tests/net/cmsg.rs +++ b/tests/net/cmsg.rs @@ -33,3 +33,101 @@ fn test_buffer_sizes() { assert!(cmsg_space!(ScmRights(1)) * 2 >= cmsg_space!(ScmRights(1), ScmRights(1))); assert!(cmsg_space!(ScmRights(1), ScmRights(0)) >= cmsg_space!(ScmRights(1))); } + +/// Test that receiving more `SCM_RIGHTS` file descriptors than fit in the +/// ancillary buffer doesn't panic and doesn't produce descriptors from +/// outside the buffer. +/// +/// Linux truncates the control data and adjusts `cmsg_len` to match, but +/// macOS truncates the data and leaves `cmsg_len` holding the untruncated +/// length, so the parsing code has to be prepared for a `cmsg_len` that runs +/// past the end of the buffer. +/// +/// Platforms report the truncation with `CTRUNC`, but not all of the +/// environments rustix's CI runs in do, so this doesn't check for it. +#[test] +fn test_truncated_scm_rights() { + use rustix::fd::{AsFd, OwnedFd}; + use rustix::io::{IoSlice, IoSliceMut}; + use rustix::net::{ + recvmsg, sendmsg, socket, socketpair, AddressFamily, RecvAncillaryBuffer, + RecvAncillaryMessage, RecvFlags, SendAncillaryBuffer, SendAncillaryMessage, SendFlags, + SocketFlags, SocketType, + }; + use std::mem::MaybeUninit; + + /// The number of file descriptors to send. + const NUM_FDS: usize = 16; + /// The number of file descriptors the receive buffer has room for. + const NUM_SLOTS: usize = 5; + + crate::init(); + + let (send_sock, recv_sock) = socketpair( + AddressFamily::UNIX, + SocketType::STREAM, + SocketFlags::empty(), + None, + ) + .unwrap(); + + // Make some file descriptors to send. + let fds: Vec = (0..NUM_FDS) + .map(|_| socket(AddressFamily::UNIX, SocketType::STREAM, None).unwrap()) + .collect(); + let borrowed: Vec<_> = fds.iter().map(AsFd::as_fd).collect(); + + let mut space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_FDS))]; + let mut cmsg_buffer = SendAncillaryBuffer::new(space.as_mut_slice()); + assert!(cmsg_buffer.push(SendAncillaryMessage::ScmRights(&borrowed))); + + sendmsg( + &send_sock, + &[IoSlice::new(b"hello")], + &mut cmsg_buffer, + SendFlags::empty(), + ) + .unwrap(); + + // Receive into a buffer with room for only `NUM_SLOTS` file descriptors. + let mut cmsg_space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_SLOTS))]; + let mut cmsg_buffer = RecvAncillaryBuffer::new(cmsg_space.as_mut_slice()); + + let mut buffer = [0_u8; 5]; + let result = recvmsg( + &recv_sock, + &mut [IoSliceMut::new(&mut buffer)], + &mut cmsg_buffer, + RecvFlags::empty(), + ) + .unwrap(); + + assert_eq!(result.bytes, 5); + assert_eq!(&buffer, b"hello"); + + // Draining the buffer shouldn't panic. + let mut received = Vec::new(); + for msg in cmsg_buffer.drain() { + match msg { + RecvAncillaryMessage::ScmRights(rights) => received.extend(rights), + _ => panic!("unexpected ancillary message"), + } + } + + // Platforms deliver differing amounts of a truncated control message — + // Linux fills the buffer, FreeBSD delivers none of it — so don't assume + // how many descriptors come back. Do require that the message was + // truncated, and that every descriptor that did come back is one of the + // sockets that was sent, rather than something read from past the end of + // the buffer. + assert!(received.len() < NUM_FDS); + for fd in &received { + assert_eq!( + rustix::net::sockopt::socket_type(fd).unwrap(), + SocketType::STREAM + ); + } + + // Dropping the buffer drains it again; that shouldn't panic either. + drop(cmsg_buffer); +}