From ed1c90d6bc9871d9881e8fcb05370752b6a5d25b Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Mon, 14 Jan 2019 23:06:21 -0700 Subject: [PATCH 1/6] Replace hand-rolled cmsg logic with libc's cmsg(3) functions. Our hand-rolled logic had subtle alignment bugs that caused test_scm_rights to fail on OpenBSD (and probably could cause problems on other platforms too). Using cmsg(3) is much cleaner, shorter, and more portable. No user-visible changes. --- src/sys/socket/mod.rs | 488 ++++++++++++++++++++---------------------- 1 file changed, 227 insertions(+), 261 deletions(-) diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index 64d2fc12a0..9527d0b349 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -3,7 +3,7 @@ //! [Further reading](http://man7.org/linux/man-pages/man7/socket.7.html) use {Error, Result}; use errno::Errno; -use libc::{self, c_void, c_int, socklen_t, size_t}; +use libc::{self, c_void, c_int, iovec, socklen_t, size_t}; use std::{fmt, mem, ptr, slice}; use std::os::unix::io::RawFd; use sys::time::TimeVal; @@ -32,6 +32,11 @@ pub use self::addr::{ pub use ::sys::socket::addr::netlink::NetlinkAddr; pub use libc::{ + CMSG_FIRSTHDR, + CMSG_NXTHDR, + CMSG_DATA, + CMSG_SPACE, + CMSG_LEN, cmsghdr, msghdr, sa_family_t, @@ -303,39 +308,6 @@ impl fmt::Debug for Ipv6MembershipRequest { } } -/// Copy the in-memory representation of `src` into the byte slice `dst`. -/// -/// Returns the remainder of `dst`. -/// -/// Panics when `dst` is too small for `src` (more precisely, panics if -/// `mem::size_of_val(src) >= dst.len()`). -/// -/// Unsafe because it transmutes `src` to raw bytes, which is only safe for some -/// types `T`. Refer to the [Rustonomicon] for details. -/// -/// [Rustonomicon]: https://doc.rust-lang.org/nomicon/transmutes.html -unsafe fn copy_bytes<'a, T: ?Sized>(src: &T, dst: &'a mut [u8]) -> &'a mut [u8] { - let srclen = mem::size_of_val(src); - ptr::copy_nonoverlapping( - src as *const T as *const u8, - dst[..srclen].as_mut_ptr(), - srclen - ); - - &mut dst[srclen..] -} - -/// Fills `dst` with `len` zero bytes and returns the remainder of the slice. -/// -/// Panics when `len >= dst.len()`. -fn pad_bytes(len: usize, dst: &mut [u8]) -> &mut [u8] { - for pad in &mut dst[..len] { - *pad = 0; - } - - &mut dst[len..] -} - cfg_if! { // Darwin and DragonFly BSD always align struct cmsghdr to 32-bit only. if #[cfg(any(target_os = "dragonfly", target_os = "ios", target_os = "macos"))] { @@ -375,13 +347,12 @@ impl CmsgSpace { } } -#[derive(Debug)] +#[allow(missing_debug_implementations)] // msghdr isn't Debug pub struct RecvMsg<'a> { - // The number of bytes received. - pub bytes: usize, - cmsg_buffer: &'a [u8], + cmsghdr: Option<&'a cmsghdr>, pub address: Option, pub flags: MsgFlags, + mhdr: msghdr, } impl<'a> RecvMsg<'a> { @@ -389,45 +360,37 @@ impl<'a> RecvMsg<'a> { /// msghdr. pub fn cmsgs(&self) -> CmsgIterator { CmsgIterator { - buf: self.cmsg_buffer, + cmsghdr: self.cmsghdr, + mhdr: &self.mhdr } } } -#[derive(Debug)] +#[allow(missing_debug_implementations)] // msghdr isn't Debug pub struct CmsgIterator<'a> { /// Control message buffer to decode from. Must adhere to cmsg alignment. - buf: &'a [u8], + cmsghdr: Option<&'a cmsghdr>, + mhdr: &'a msghdr } impl<'a> Iterator for CmsgIterator<'a> { type Item = ControlMessage<'a>; - // The implementation loosely follows CMSG_FIRSTHDR / CMSG_NXTHDR, - // although we handle the invariants in slightly different places to - // get a better iterator interface. fn next(&mut self) -> Option> { - if self.buf.len() == 0 { - // The iterator assumes that `self.buf` always contains exactly the - // bytes we need, so we're at the end when the buffer is empty. - return None; - } - - // Safe if: `self.buf` is `cmsghdr`-aligned. - let cmsg: &'a cmsghdr = unsafe { - &*(self.buf[..mem::size_of::()].as_ptr() as *const cmsghdr) - }; - - let cmsg_len = cmsg.cmsg_len as usize; - - // Advance our internal pointer. - let cmsg_data = &self.buf[cmsg_align(mem::size_of::())..cmsg_len]; - self.buf = &self.buf[cmsg_align(cmsg_len)..]; - - // Safe if: `cmsg_data` contains the expected (amount of) content. This - // is verified by the kernel. - unsafe { - Some(ControlMessage::decode_from(cmsg, cmsg_data)) + match self.cmsghdr { + None => None, // No more messages + Some(hdr) => { + // Get the data. + // Safe if cmsghdr points to valid data returned by recvmsg(2) + let cm = unsafe { Some(ControlMessage::decode_from(hdr))}; + // Advance the internal pointer. Safe if mhdr and cmsghdr point + // to valid data returned by recvmsg(2) + self.cmsghdr = unsafe { + let p = CMSG_NXTHDR(self.mhdr as *const _, hdr as *const _); + p.as_ref() + }; + cm + } } } } @@ -562,33 +525,96 @@ pub enum ControlMessage<'a> { #[allow(missing_debug_implementations)] pub struct UnknownCmsg<'a>(&'a cmsghdr, &'a [u8]); -// Round `len` up to meet the platform's required alignment for -// `cmsghdr`s and trailing `cmsghdr` data. This should match the -// behaviour of CMSG_ALIGN from the Linux headers and do the correct -// thing on other platforms that don't usually provide CMSG_ALIGN. -#[inline] -fn cmsg_align(len: usize) -> usize { - let align_bytes = mem::size_of::() - 1; - (len + align_bytes) & !align_bytes -} - impl<'a> ControlMessage<'a> { /// The value of CMSG_SPACE on this message. + /// Safe because CMSG_SPACE is always safe fn space(&self) -> usize { - cmsg_align(self.len()) + unsafe{CMSG_SPACE(self.len() as libc::c_uint) as usize} } /// The value of CMSG_LEN on this message. + /// Safe because CMSG_LEN is always safe + #[cfg(any(target_os = "android", + all(target_os = "linux", not(target_env = "musl"))))] + fn cmsg_len(&self) -> usize { + unsafe{CMSG_LEN(self.len() as libc::c_uint) as usize} + } + + #[cfg(not(any(target_os = "android", + all(target_os = "linux", not(target_env = "musl")))))] + fn cmsg_len(&self) -> libc::c_uint { + unsafe{CMSG_LEN(self.len() as libc::c_uint)} + } + + /// Return a reference to the payload data as a byte pointer + fn data(&self) -> *const u8 { + match self { + &ControlMessage::ScmRights(fds) => { + fds as *const [RawFd] as *const u8 + }, + #[cfg(any(target_os = "android", target_os = "linux"))] + &ControlMessage::ScmCredentials(creds) => { + creds as *const libc::ucred as *const u8 + } + &ControlMessage::ScmTimestamp(t) => { + t as *const TimeVal as *const u8 + }, + #[cfg(any( + target_os = "android", + target_os = "ios", + target_os = "linux", + target_os = "macos" + ))] + &ControlMessage::Ipv4PacketInfo(pktinfo) => { + pktinfo as *const libc::in_pktinfo as *const u8 + }, + #[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "ios", + target_os = "linux", + target_os = "macos" + ))] + &ControlMessage::Ipv6PacketInfo(pktinfo) => { + pktinfo as *const libc::in6_pktinfo as *const u8 + }, + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + &ControlMessage::Ipv4RecvIf(dl) => { + dl as *const libc::sockaddr_dl as *const u8 + }, + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + &ControlMessage::Ipv4RecvDstAddr(in_addr) => { + in_addr as *const libc::in_addr as *const u8 + }, + &ControlMessage::Unknown(UnknownCmsg(_, bytes)) => { + bytes as *const _ as *const u8 + } + } + } + + /// The size of the payload, excluding its cmsghdr fn len(&self) -> usize { - cmsg_align(mem::size_of::()) + match *self { - ControlMessage::ScmRights(fds) => { + match self { + &ControlMessage::ScmRights(fds) => { mem::size_of_val(fds) }, #[cfg(any(target_os = "android", target_os = "linux"))] - ControlMessage::ScmCredentials(creds) => { + &ControlMessage::ScmCredentials(creds) => { mem::size_of_val(creds) } - ControlMessage::ScmTimestamp(t) => { + &ControlMessage::ScmTimestamp(t) => { mem::size_of_val(t) }, #[cfg(any( @@ -597,7 +623,7 @@ impl<'a> ControlMessage<'a> { target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv4PacketInfo(pktinfo) => { + &ControlMessage::Ipv4PacketInfo(pktinfo) => { mem::size_of_val(pktinfo) }, #[cfg(any( @@ -607,7 +633,7 @@ impl<'a> ControlMessage<'a> { target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv6PacketInfo(pktinfo) => { + &ControlMessage::Ipv6PacketInfo(pktinfo) => { mem::size_of_val(pktinfo) }, #[cfg(any( @@ -617,7 +643,7 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvIf(dl) => { + &ControlMessage::Ipv4RecvIf(dl) => { mem::size_of_val(dl) }, #[cfg(any( @@ -627,10 +653,10 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvDstAddr(inaddr) => { + &ControlMessage::Ipv4RecvDstAddr(inaddr) => { mem::size_of_val(inaddr) }, - ControlMessage::Unknown(UnknownCmsg(_, bytes)) => { + &ControlMessage::Unknown(UnknownCmsg(_, bytes)) => { mem::size_of_val(bytes) } } @@ -638,18 +664,18 @@ impl<'a> ControlMessage<'a> { /// Returns the value to put into the `cmsg_level` field of the header. fn cmsg_level(&self) -> libc::c_int { - match *self { - ControlMessage::ScmRights(_) => libc::SOL_SOCKET, + match self { + &ControlMessage::ScmRights(_) => libc::SOL_SOCKET, #[cfg(any(target_os = "android", target_os = "linux"))] - ControlMessage::ScmCredentials(_) => libc::SOL_SOCKET, - ControlMessage::ScmTimestamp(_) => libc::SOL_SOCKET, + &ControlMessage::ScmCredentials(_) => libc::SOL_SOCKET, + &ControlMessage::ScmTimestamp(_) => libc::SOL_SOCKET, #[cfg(any( target_os = "android", target_os = "ios", target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv4PacketInfo(_) => libc::IPPROTO_IP, + &ControlMessage::Ipv4PacketInfo(_) => libc::IPPROTO_IP, #[cfg(any( target_os = "android", target_os = "freebsd", @@ -657,7 +683,7 @@ impl<'a> ControlMessage<'a> { target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv6PacketInfo(_) => libc::IPPROTO_IPV6, + &ControlMessage::Ipv6PacketInfo(_) => libc::IPPROTO_IPV6, #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -665,7 +691,7 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvIf(_) => libc::IPPROTO_IP, + &ControlMessage::Ipv4RecvIf(_) => libc::IPPROTO_IP, #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -673,25 +699,25 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvDstAddr(_) => libc::IPPROTO_IP, - ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_level, + &ControlMessage::Ipv4RecvDstAddr(_) => libc::IPPROTO_IP, + &ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_level, } } /// Returns the value to put into the `cmsg_type` field of the header. fn cmsg_type(&self) -> libc::c_int { - match *self { - ControlMessage::ScmRights(_) => libc::SCM_RIGHTS, + match self { + &ControlMessage::ScmRights(_) => libc::SCM_RIGHTS, #[cfg(any(target_os = "android", target_os = "linux"))] - ControlMessage::ScmCredentials(_) => libc::SCM_CREDENTIALS, - ControlMessage::ScmTimestamp(_) => libc::SCM_TIMESTAMP, + &ControlMessage::ScmCredentials(_) => libc::SCM_CREDENTIALS, + &ControlMessage::ScmTimestamp(_) => libc::SCM_TIMESTAMP, #[cfg(any( target_os = "android", target_os = "ios", target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv4PacketInfo(_) => libc::IP_PKTINFO, + &ControlMessage::Ipv4PacketInfo(_) => libc::IP_PKTINFO, #[cfg(any( target_os = "android", target_os = "freebsd", @@ -699,7 +725,7 @@ impl<'a> ControlMessage<'a> { target_os = "linux", target_os = "macos" ))] - ControlMessage::Ipv6PacketInfo(_) => libc::IPV6_PKTINFO, + &ControlMessage::Ipv6PacketInfo(_) => libc::IPV6_PKTINFO, #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -707,7 +733,7 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvIf(_) => libc::IP_RECVIF, + &ControlMessage::Ipv4RecvIf(_) => libc::IP_RECVIF, #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -715,93 +741,23 @@ impl<'a> ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - ControlMessage::Ipv4RecvDstAddr(_) => libc::IP_RECVDSTADDR, - ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_type, + &ControlMessage::Ipv4RecvDstAddr(_) => libc::IP_RECVDSTADDR, + &ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_type, } } - // Unsafe: start and end of buffer must be cmsg_align'd. Updates - // the provided slice; panics if the buffer is too small. - unsafe fn encode_into(&self, buf: &mut [u8]) { - let final_buf = if let ControlMessage::Unknown(ref cmsg) = *self { - let &UnknownCmsg(orig_cmsg, bytes) = cmsg; - - let buf = copy_bytes(orig_cmsg, buf); - - let padlen = cmsg_align(mem::size_of_val(&orig_cmsg)) - - mem::size_of_val(&orig_cmsg); - let buf = pad_bytes(padlen, buf); - - copy_bytes(bytes, buf) - } else { - let cmsg = cmsghdr { - cmsg_len: self.len() as _, - cmsg_level: self.cmsg_level(), - cmsg_type: self.cmsg_type(), - ..mem::zeroed() // zero out platform-dependent padding fields - }; - let buf = copy_bytes(&cmsg, buf); - - let padlen = cmsg_align(mem::size_of_val(&cmsg)) - - mem::size_of_val(&cmsg); - let buf = pad_bytes(padlen, buf); - - match *self { - ControlMessage::ScmRights(fds) => { - copy_bytes(fds, buf) - }, - #[cfg(any(target_os = "android", target_os = "linux"))] - ControlMessage::ScmCredentials(creds) => { - copy_bytes(creds, buf) - }, - ControlMessage::ScmTimestamp(t) => { - copy_bytes(t, buf) - }, - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - ControlMessage::Ipv4PacketInfo(pktinfo) => { - copy_bytes(pktinfo, buf) - }, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - ControlMessage::Ipv6PacketInfo(pktinfo) => { - copy_bytes(pktinfo, buf) - } - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - ControlMessage::Ipv4RecvIf(dl) => { - copy_bytes(dl, buf) - }, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - ControlMessage::Ipv4RecvDstAddr(inaddr) => { - copy_bytes(inaddr, buf) - }, - ControlMessage::Unknown(_) => unreachable!(), - } - }; - - let padlen = self.space() - self.len(); - pad_bytes(padlen, final_buf); + // Unsafe: cmsg must point to a valid cmsghdr with enough space to + // encode self. + unsafe fn encode_into(&self, cmsg: *mut cmsghdr) { + (*cmsg).cmsg_level = self.cmsg_level(); + (*cmsg).cmsg_type = self.cmsg_type(); + (*cmsg).cmsg_len = self.cmsg_len(); + let data = self.data(); + ptr::copy_nonoverlapping( + data, + CMSG_DATA(cmsg), + self.len() + ); } /// Decodes a `ControlMessage` from raw bytes. @@ -810,22 +766,23 @@ impl<'a> ControlMessage<'a> { /// specified in the header. Normally, the kernel ensures that this is the /// case. "Correct" in this case includes correct length, alignment and /// actual content. - unsafe fn decode_from(header: &'a cmsghdr, data: &'a [u8]) -> ControlMessage<'a> { + unsafe fn decode_from(header: &'a cmsghdr) -> ControlMessage<'a> + { + let p = CMSG_DATA(header); + let len = header as *const _ as usize + header.cmsg_len as usize + - p as usize; match (header.cmsg_level, header.cmsg_type) { (libc::SOL_SOCKET, libc::SCM_RIGHTS) => { ControlMessage::ScmRights( - slice::from_raw_parts(data.as_ptr() as *const _, - data.len() / mem::size_of::())) + slice::from_raw_parts(p as *const RawFd, + len / mem::size_of::())) }, #[cfg(any(target_os = "android", target_os = "linux"))] (libc::SOL_SOCKET, libc::SCM_CREDENTIALS) => { - ControlMessage::ScmCredentials( - &*(data.as_ptr() as *const _) - ) + ControlMessage::ScmCredentials(&*(p as *const _)) } (libc::SOL_SOCKET, libc::SCM_TIMESTAMP) => { - ControlMessage::ScmTimestamp( - &*(data.as_ptr() as *const _)) + ControlMessage::ScmTimestamp(&*(p as *const _)) }, #[cfg(any( target_os = "android", @@ -835,8 +792,7 @@ impl<'a> ControlMessage<'a> { target_os = "macos" ))] (libc::IPPROTO_IPV6, libc::IPV6_PKTINFO) => { - ControlMessage::Ipv6PacketInfo( - &*(data.as_ptr() as *const _)) + ControlMessage::Ipv6PacketInfo(&*(p as *const _)) } #[cfg(any( target_os = "android", @@ -845,8 +801,7 @@ impl<'a> ControlMessage<'a> { target_os = "macos" ))] (libc::IPPROTO_IP, libc::IP_PKTINFO) => { - ControlMessage::Ipv4PacketInfo( - &*(data.as_ptr() as *const _)) + ControlMessage::Ipv4PacketInfo(&*(p as *const _)) } #[cfg(any( target_os = "freebsd", @@ -856,8 +811,7 @@ impl<'a> ControlMessage<'a> { target_os = "openbsd", ))] (libc::IPPROTO_IP, libc::IP_RECVIF) => { - ControlMessage::Ipv4RecvIf( - &*(data.as_ptr() as *const _)) + ControlMessage::Ipv4RecvIf(&*(p as *const _)) } #[cfg(any( target_os = "freebsd", @@ -867,11 +821,11 @@ impl<'a> ControlMessage<'a> { target_os = "openbsd", ))] (libc::IPPROTO_IP, libc::IP_RECVDSTADDR) => { - ControlMessage::Ipv4RecvDstAddr( - &*(data.as_ptr() as *const _)) + ControlMessage::Ipv4RecvDstAddr(&*(p as *const _)) } (_, _) => { + let data = slice::from_raw_parts(p, len); ControlMessage::Unknown(UnknownCmsg(header, data)) } } @@ -884,51 +838,60 @@ impl<'a> ControlMessage<'a> { /// as with sendto. /// /// Allocates if cmsgs is nonempty. -pub fn sendmsg<'a>(fd: RawFd, iov: &[IoVec<&'a [u8]>], cmsgs: &[ControlMessage<'a>], flags: MsgFlags, addr: Option<&'a SockAddr>) -> Result { - let mut capacity = 0; - for cmsg in cmsgs { - capacity += cmsg.space(); - } - // Note that the resulting vector claims to have length == capacity, - // so it's presently uninitialized. - let mut cmsg_buffer = unsafe { - let mut vec = Vec::::with_capacity(capacity); - vec.set_len(capacity); - vec - }; - { - let mut ofs = 0; - for cmsg in cmsgs { - let ptr = &mut cmsg_buffer[ofs..]; - unsafe { - cmsg.encode_into(ptr); - } - ofs += cmsg.space(); - } - } +pub fn sendmsg(fd: RawFd, iov: &[IoVec<&[u8]>], cmsgs: &[ControlMessage], + flags: MsgFlags, addr: Option<&SockAddr>) -> Result +{ + let capacity = cmsgs.iter().map(|c| c.space()).sum(); + + // First size the buffer needed to hold the cmsgs. It must be zeroed, + // because subsequent code will not clear the padding bytes. + let cmsg_buffer = vec![0u8; capacity]; + // Next encode the sending address, if provided let (name, namelen) = match addr { - Some(addr) => { let (x, y) = unsafe { addr.as_ffi_pair() }; (x as *const _, y) } + Some(addr) => { + let (x, y) = unsafe { addr.as_ffi_pair() }; + (x as *const _, y) + }, None => (ptr::null(), 0), }; + // The message header must be initialized before the individual cmsgs. let cmsg_ptr = if capacity > 0 { - cmsg_buffer.as_ptr() as *const c_void + cmsg_buffer.as_ptr() as *mut c_void } else { - ptr::null() + ptr::null_mut() }; - let mhdr = unsafe { - let mut mhdr: msghdr = mem::uninitialized(); - mhdr.msg_name = name as *mut _; - mhdr.msg_namelen = namelen; - mhdr.msg_iov = iov.as_ptr() as *mut _; - mhdr.msg_iovlen = iov.len() as _; - mhdr.msg_control = cmsg_ptr as *mut _; - mhdr.msg_controllen = capacity as _; - mhdr.msg_flags = 0; + let mhdr = { + // Musl's msghdr has private fields, so this is the only way to + // initialize it. + let mut mhdr: msghdr = unsafe{mem::uninitialized()}; + mhdr.msg_name = name as *mut _; + mhdr.msg_namelen = namelen; + // transmute iov into a mutable pointer. sendmsg doesn't really mutate + // the buffer, but the standard says that it takes a mutable pointer + mhdr.msg_iov = iov.as_ptr() as *mut _; + mhdr.msg_iovlen = iov.len() as _; + mhdr.msg_control = cmsg_ptr; + mhdr.msg_controllen = capacity as _; + mhdr.msg_flags = 0; mhdr }; + + // Encode each cmsg. This must happen after initializing the header because + // CMSG_NEXT_HDR and friends read the msg_control and msg_controllen fields. + // CMSG_FIRSTHDR is always safe + let mut pmhdr: *mut cmsghdr = unsafe{CMSG_FIRSTHDR(&mhdr as *const msghdr)}; + for cmsg in cmsgs { + assert_ne!(pmhdr, ptr::null_mut()); + // Safe because we know that pmhdr is valid, and we initialized it with + // sufficient space + unsafe { cmsg.encode_into(pmhdr) }; + // Safe because mhdr is valid + pmhdr = unsafe{CMSG_NXTHDR(&mhdr as *const msghdr, pmhdr)}; + } + let ret = unsafe { libc::sendmsg(fd, &mhdr, flags.bits()) }; Errno::result(ret).map(|r| r as usize) @@ -937,45 +900,48 @@ pub fn sendmsg<'a>(fd: RawFd, iov: &[IoVec<&'a [u8]>], cmsgs: &[ControlMessage<' /// Receive message in scatter-gather vectors from a socket, and /// optionally receive ancillary data into the provided buffer. /// If no ancillary data is desired, use () as the type parameter. -pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], cmsg_buffer: Option<&'a mut CmsgSpace>, flags: MsgFlags) -> Result> { +pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], + cmsg_buffer: Option<&'a mut CmsgSpace>, + flags: MsgFlags) -> Result> +{ let mut address: sockaddr_storage = unsafe { mem::uninitialized() }; let (msg_control, msg_controllen) = match cmsg_buffer { Some(cmsg_buffer) => (cmsg_buffer as *mut _, mem::size_of_val(cmsg_buffer)), None => (ptr::null_mut(), 0), }; - let mut mhdr = unsafe { - let mut mhdr: msghdr = mem::uninitialized(); - mhdr.msg_name = &mut address as *mut _ as *mut _; - mhdr.msg_namelen = mem::size_of::() as socklen_t; - mhdr.msg_iov = iov.as_ptr() as *mut _; - mhdr.msg_iovlen = iov.len() as _; - mhdr.msg_control = msg_control as *mut _; - mhdr.msg_controllen = msg_controllen as _; - mhdr.msg_flags = 0; + let mut mhdr = { + // Musl's msghdr has private fields, so this is the only way to + // initialize it. + let mut mhdr: msghdr = unsafe{mem::uninitialized()}; + mhdr.msg_name = &mut address as *mut sockaddr_storage as *mut c_void; + mhdr.msg_namelen = mem::size_of::() as socklen_t; + mhdr.msg_iov = iov.as_ptr() as *mut iovec; + mhdr.msg_iovlen = iov.len() as _; + mhdr.msg_control = msg_control as *mut c_void; + mhdr.msg_controllen = msg_controllen as _; + mhdr.msg_flags = 0; mhdr }; - let ret = unsafe { libc::recvmsg(fd, &mut mhdr, flags.bits()) }; - - let cmsg_buffer = if msg_controllen > 0 { - // got control message(s) - debug_assert!(!mhdr.msg_control.is_null()); - unsafe { - // Safe: The pointer is not null and the length is correct as part of `recvmsg`s - // contract. - slice::from_raw_parts(mhdr.msg_control as *const u8, - mhdr.msg_controllen as usize) - } - } else { - // No control message, create an empty buffer to avoid creating a slice from a null pointer - &[] + + let _ret = unsafe { libc::recvmsg(fd, &mut mhdr, flags.bits()) }; + + let cmsghdr = unsafe { + if mhdr.msg_controllen > 0 { + // got control message(s) + debug_assert!(!mhdr.msg_control.is_null()); + debug_assert!(msg_controllen >= mhdr.msg_controllen as usize); + CMSG_FIRSTHDR(&mhdr as *const msghdr) + } else { + ptr::null() + }.as_ref() }; Ok(unsafe { RecvMsg { - bytes: Errno::result(ret)? as usize, - cmsg_buffer, + cmsghdr, address: sockaddr_storage_to_addr(&address, mhdr.msg_namelen as usize).ok(), flags: MsgFlags::from_bits_truncate(mhdr.msg_flags), + mhdr, } }) } From 368b9fe9436734f7fccffa2fdf3e723ee20fb836 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Tue, 15 Jan 2019 09:23:33 -0700 Subject: [PATCH 2/6] Fix error handling of RecvMsg There were two problems: 1) It would always return Ok, even on error 2) It could panic if there was an error, because sockaddr_storage_to_addr would be called on uninitialized memory. --- src/sys/socket/mod.rs | 45 ++++++++++++++++++++++++----------------- test/sys/test_socket.rs | 15 ++++++++++++++ 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index 9527d0b349..ffb68157b9 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -900,6 +900,9 @@ pub fn sendmsg(fd: RawFd, iov: &[IoVec<&[u8]>], cmsgs: &[ControlMessage], /// Receive message in scatter-gather vectors from a socket, and /// optionally receive ancillary data into the provided buffer. /// If no ancillary data is desired, use () as the type parameter. +/// +/// # References +/// [recvmsg(2)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/recvmsg.html) pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], cmsg_buffer: Option<&'a mut CmsgSpace>, flags: MsgFlags) -> Result> @@ -923,26 +926,30 @@ pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], mhdr }; - let _ret = unsafe { libc::recvmsg(fd, &mut mhdr, flags.bits()) }; - - let cmsghdr = unsafe { - if mhdr.msg_controllen > 0 { - // got control message(s) - debug_assert!(!mhdr.msg_control.is_null()); - debug_assert!(msg_controllen >= mhdr.msg_controllen as usize); - CMSG_FIRSTHDR(&mhdr as *const msghdr) - } else { - ptr::null() - }.as_ref() - }; + let ret = unsafe { libc::recvmsg(fd, &mut mhdr, flags.bits()) }; + + Errno::result(ret).map(|_| { + let cmsghdr = unsafe { + if mhdr.msg_controllen > 0 { + // got control message(s) + debug_assert!(!mhdr.msg_control.is_null()); + debug_assert!(msg_controllen >= mhdr.msg_controllen as usize); + CMSG_FIRSTHDR(&mhdr as *const msghdr) + } else { + ptr::null() + }.as_ref() + }; - Ok(unsafe { RecvMsg { - cmsghdr, - address: sockaddr_storage_to_addr(&address, - mhdr.msg_namelen as usize).ok(), - flags: MsgFlags::from_bits_truncate(mhdr.msg_flags), - mhdr, - } }) + let address = unsafe { + sockaddr_storage_to_addr(&address, mhdr.msg_namelen as usize).ok() + }; + RecvMsg { + cmsghdr, + address, + flags: MsgFlags::from_bits_truncate(mhdr.msg_flags), + mhdr, + } + }) } diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index 2cc3475f75..cc8c6f8961 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -117,6 +117,21 @@ pub fn test_socketpair() { assert_eq!(&buf[..], b"hello"); } +// Test error handling of our recvmsg wrapper +#[test] +pub fn test_recvmsg_ebadf() { + use nix::Error; + use nix::errno::Errno; + use nix::sys::socket::{MsgFlags, recvmsg}; + use nix::sys::uio::IoVec; + + let mut buf = [0u8; 5]; + let iov = [IoVec::from_mut_slice(&mut buf[..])]; + let fd = -1; // Bad file descriptor + let r = recvmsg::<()>(fd, &iov, None, MsgFlags::empty()); + assert_eq!(r.err().unwrap(), Error::Sys(Errno::EBADF)); +} + #[test] pub fn test_scm_rights() { use nix::sys::uio::IoVec; From e0f612df5a232e058e4188502ab11b533cc78608 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Tue, 12 Feb 2019 11:21:49 -0700 Subject: [PATCH 3/6] Replace CmsgSpace with a macro CmsgSpace had three problems: 1) It would oversize buffers that expect multiple control messages 2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually undersize a buffer for a single control message. 3) It could do bad things on drop, if you instantiate it with a type that implements Drop (which none of the currently supported ControlMessage types do). Fixes #994 --- src/sys/socket/mod.rs | 106 +++++++++++++++++++++++++++++++++------- test/sys/test_socket.rs | 55 +++++++++++---------- 2 files changed, 118 insertions(+), 43 deletions(-) diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index ffb68157b9..de6644bb96 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -3,7 +3,8 @@ //! [Further reading](http://man7.org/linux/man-pages/man7/socket.7.html) use {Error, Result}; use errno::Errno; -use libc::{self, c_void, c_int, iovec, socklen_t, size_t}; +use libc::{self, c_void, c_int, iovec, socklen_t, size_t, + CMSG_FIRSTHDR, CMSG_NXTHDR, CMSG_DATA, CMSG_LEN}; use std::{fmt, mem, ptr, slice}; use std::os::unix::io::RawFd; use sys::time::TimeVal; @@ -32,11 +33,6 @@ pub use self::addr::{ pub use ::sys::socket::addr::netlink::NetlinkAddr; pub use libc::{ - CMSG_FIRSTHDR, - CMSG_NXTHDR, - CMSG_DATA, - CMSG_SPACE, - CMSG_LEN, cmsghdr, msghdr, sa_family_t, @@ -47,6 +43,10 @@ pub use libc::{ sockaddr_un, }; +// Needed by the cmsg_space macro +#[doc(hidden)] +pub use libc::{c_uint, CMSG_SPACE}; + /// These constants are used to specify the communication semantics /// when creating a socket with [`socket()`](fn.socket.html) #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -317,6 +317,55 @@ cfg_if! { } } +/// A type that can be used to store ancillary data received by +/// [`recvmsg`](fn.recvmsg.html) +pub trait CmsgBuffer { + fn as_bytes_mut(&mut self) -> &mut [u8]; +} + +/// Create a buffer large enough for storing some control messages as returned +/// by [`recvmsg`](fn.recvmsg.html). +/// +/// # Examples +/// +/// ``` +/// # #[macro_use] extern crate nix; +/// # use nix::sys::time::TimeVal; +/// # use std::os::unix::io::RawFd; +/// # fn main() { +/// // Create a buffer for a `ControlMessage::ScmTimestamp` message +/// let _ = cmsg_space!(TimeVal); +/// // Create a buffer big enough for a `ControlMessage::ScmRights` message +/// // with two file descriptors +/// let _ = cmsg_space!([RawFd; 2]); +/// // Create a buffer big enough for a `ControlMessage::ScmRights` message +/// // and a `ControlMessage::ScmTimestamp` message +/// let _ = cmsg_space!(RawFd, TimeVal); +/// # } +/// ``` +// Unfortunately, CMSG_SPACE isn't a const_fn, or else we could return a +// stack-allocated array. +#[macro_export] +macro_rules! cmsg_space { + ( $( $x:ty ),* ) => { + { + use nix::sys::socket::{c_uint, CMSG_SPACE}; + use std::mem; + let mut space = 0; + $( + // CMSG_SPACE is always safe + space += unsafe { + CMSG_SPACE(mem::size_of::<$x>() as c_uint) + } as usize; + )* + let mut v = Vec::::with_capacity(space); + // safe because any bit pattern is a valid u8 + unsafe {v.set_len(space)}; + v + } + } +} + /// A structure used to make room in a cmsghdr passed to recvmsg. The /// size and alignment match that of a cmsghdr followed by a T, but the /// fields are not accessible, as the actual types will change on a call @@ -341,12 +390,29 @@ pub struct CmsgSpace { impl CmsgSpace { /// Create a CmsgSpace. The structure is used only for space, so /// the fields are uninitialized. + #[deprecated( since="0.14.0", note="Use the cmsg_space! macro instead")] pub fn new() -> Self { // Safe because the fields themselves aren't accessible. unsafe { mem::uninitialized() } } } +impl CmsgBuffer for CmsgSpace { + fn as_bytes_mut(&mut self) -> &mut [u8] { + // Safe because nothing ever attempts to access CmsgSpace's fields + unsafe { + slice::from_raw_parts_mut(self as *mut CmsgSpace as *mut u8, + mem::size_of::()) + } + } +} + +impl CmsgBuffer for Vec { + fn as_bytes_mut(&mut self) -> &mut [u8] { + &mut self[..] + } +} + #[allow(missing_debug_implementations)] // msghdr isn't Debug pub struct RecvMsg<'a> { cmsghdr: Option<&'a cmsghdr>, @@ -437,11 +503,12 @@ pub enum ControlMessage<'a> { // https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=222039 #[cfg_attr(not(all(target_os = "freebsd", target_arch = "x86")), doc = " ```")] #[cfg_attr(all(target_os = "freebsd", target_arch = "x86"), doc = " ```no_run")] - /// use nix::sys::socket::*; - /// use nix::sys::uio::IoVec; - /// use nix::sys::time::*; - /// use std::time::*; - /// + /// # #[macro_use] extern crate nix; + /// # use nix::sys::socket::*; + /// # use nix::sys::uio::IoVec; + /// # use nix::sys::time::*; + /// # use std::time::*; + /// # fn main() { /// // Set up /// let message = "Ohayō!".as_bytes(); /// let in_socket = socket( @@ -462,11 +529,11 @@ pub enum ControlMessage<'a> { /// assert_eq!(message.len(), l); /// // Receive the message /// let mut buffer = vec![0u8; message.len()]; - /// let mut cmsgspace: CmsgSpace = CmsgSpace::new(); + /// let mut cmsgspace = cmsg_space!(TimeVal); /// let iov = [IoVec::from_mut_slice(&mut buffer)]; /// let r = recvmsg(in_socket, &iov, Some(&mut cmsgspace), flags).unwrap(); /// let rtime = match r.cmsgs().next() { - /// Some(ControlMessage::ScmTimestamp(&rtime)) => rtime, + /// Some(ControlMessage::ScmTimestamp(rtime)) => rtime, /// Some(_) => panic!("Unexpected control message"), /// None => panic!("No control message") /// }; @@ -480,9 +547,9 @@ pub enum ControlMessage<'a> { /// assert!(rduration <= time1.duration_since(UNIX_EPOCH).unwrap()); /// // Close socket /// nix::unistd::close(in_socket).unwrap(); + /// # } /// ``` ScmTimestamp(&'a TimeVal), - #[cfg(any( target_os = "android", target_os = "ios", @@ -903,13 +970,16 @@ pub fn sendmsg(fd: RawFd, iov: &[IoVec<&[u8]>], cmsgs: &[ControlMessage], /// /// # References /// [recvmsg(2)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/recvmsg.html) -pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], - cmsg_buffer: Option<&'a mut CmsgSpace>, - flags: MsgFlags) -> Result> +pub fn recvmsg<'a>(fd: RawFd, iov: &[IoVec<&mut [u8]>], + cmsg_buffer: Option<&'a mut CmsgBuffer>, + flags: MsgFlags) -> Result> { let mut address: sockaddr_storage = unsafe { mem::uninitialized() }; let (msg_control, msg_controllen) = match cmsg_buffer { - Some(cmsg_buffer) => (cmsg_buffer as *mut _, mem::size_of_val(cmsg_buffer)), + Some(cmsgspace) => { + let msg_buf = cmsgspace.as_bytes_mut(); + (msg_buf.as_mut_ptr(), msg_buf.len()) + }, None => (ptr::null_mut(), 0), }; let mut mhdr = { diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index cc8c6f8961..5a40574dfd 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -128,17 +128,20 @@ pub fn test_recvmsg_ebadf() { let mut buf = [0u8; 5]; let iov = [IoVec::from_mut_slice(&mut buf[..])]; let fd = -1; // Bad file descriptor - let r = recvmsg::<()>(fd, &iov, None, MsgFlags::empty()); + let r = recvmsg(fd, &iov, None, MsgFlags::empty()); assert_eq!(r.err().unwrap(), Error::Sys(Errno::EBADF)); } +// Disable the test on emulated platforms due to a bug in QEMU versions < +// 2.12.0. https://bugs.launchpad.net/qemu/+bug/1701808 +#[cfg_attr(not(any(target_arch = "x86_64", target_arch="i686")), ignore)] #[test] pub fn test_scm_rights() { use nix::sys::uio::IoVec; use nix::unistd::{pipe, read, write, close}; use nix::sys::socket::{socketpair, sendmsg, recvmsg, AddressFamily, SockType, SockFlag, - ControlMessage, CmsgSpace, MsgFlags}; + ControlMessage, MsgFlags}; let (fd1, fd2) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) .unwrap(); @@ -157,7 +160,7 @@ pub fn test_scm_rights() { { let mut buf = [0u8; 5]; let iov = [IoVec::from_mut_slice(&mut buf[..])]; - let mut cmsgspace: CmsgSpace<[RawFd; 1]> = CmsgSpace::new(); + let mut cmsgspace = cmsg_space!([RawFd; 1]); let msg = recvmsg(fd2, &iov, Some(&mut cmsgspace), MsgFlags::empty()).unwrap(); for cmsg in msg.cmsgs() { @@ -184,12 +187,16 @@ pub fn test_scm_rights() { } /// Tests that passing multiple fds using a single `ControlMessage` works. +// Disable the test on emulated platforms due to a bug in QEMU versions < +// 2.12.0. https://bugs.launchpad.net/qemu/+bug/1701808 +#[cfg_attr(not(any(target_arch = "x86_64", target_arch="i686")), ignore)] #[test] fn test_scm_rights_single_cmsg_multiple_fds() { use std::os::unix::net::UnixDatagram; use std::os::unix::io::{RawFd, AsRawFd}; use std::thread; - use nix::sys::socket::{CmsgSpace, ControlMessage, MsgFlags, sendmsg, recvmsg}; + use nix::sys::socket::{ControlMessage, MsgFlags, + sendmsg, recvmsg}; use nix::sys::uio::IoVec; use libc; @@ -197,7 +204,7 @@ fn test_scm_rights_single_cmsg_multiple_fds() { let thread = thread::spawn(move || { let mut buf = [0u8; 8]; let iovec = [IoVec::from_mut_slice(&mut buf)]; - let mut space = CmsgSpace::<[RawFd; 2]>::new(); + let mut space = cmsg_space!([RawFd; 2]); let msg = recvmsg( receive.as_raw_fd(), &iovec, @@ -237,8 +244,7 @@ pub fn test_sendmsg_empty_cmsgs() { use nix::sys::uio::IoVec; use nix::unistd::close; use nix::sys::socket::{socketpair, sendmsg, recvmsg, - AddressFamily, SockType, SockFlag, - CmsgSpace, MsgFlags}; + AddressFamily, SockType, SockFlag, MsgFlags}; let (fd1, fd2) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) .unwrap(); @@ -252,7 +258,7 @@ pub fn test_sendmsg_empty_cmsgs() { { let mut buf = [0u8; 5]; let iov = [IoVec::from_mut_slice(&mut buf[..])]; - let mut cmsgspace: CmsgSpace<[RawFd; 1]> = CmsgSpace::new(); + let mut cmsgspace = cmsg_space!([RawFd; 1]); let msg = recvmsg(fd2, &iov, Some(&mut cmsgspace), MsgFlags::empty()).unwrap(); for _ in msg.cmsgs() { @@ -271,7 +277,7 @@ fn test_scm_credentials() { use nix::unistd::{close, getpid, getuid, getgid}; use nix::sys::socket::{socketpair, sendmsg, recvmsg, setsockopt, AddressFamily, SockType, SockFlag, - ControlMessage, CmsgSpace, MsgFlags}; + ControlMessage, MsgFlags}; use nix::sys::socket::sockopt::PassCred; let (send, recv) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) @@ -293,7 +299,7 @@ fn test_scm_credentials() { { let mut buf = [0u8; 5]; let iov = [IoVec::from_mut_slice(&mut buf[..])]; - let mut cmsgspace: CmsgSpace = CmsgSpace::new(); + let mut cmsgspace = cmsg_space!(libc::ucred); let msg = recvmsg(recv, &iov, Some(&mut cmsgspace), MsgFlags::empty()).unwrap(); let mut received_cred = None; @@ -303,7 +309,7 @@ fn test_scm_credentials() { assert_eq!(cred.pid, getpid().as_raw()); assert_eq!(cred.uid, getuid().as_raw()); assert_eq!(cred.gid, getgid().as_raw()); - received_cred = Some(*cred); + received_cred = Some(cred); } else { panic!("unexpected cmsg"); } @@ -322,27 +328,26 @@ fn test_scm_credentials() { #[cfg_attr(not(any(target_arch = "x86_64", target_arch = "x86")), ignore)] #[test] fn test_scm_credentials_and_rights() { - use nix::sys::socket::CmsgSpace; use libc; - test_impl_scm_credentials_and_rights(CmsgSpace::<(libc::ucred, CmsgSpace)>::new()); + let space = cmsg_space!(libc::ucred, RawFd); + test_impl_scm_credentials_and_rights(space); } -/// Ensure that passing a `CmsgSpace` with too much space for the received -/// messages still works. +/// Ensure that passing a an oversized control message buffer to recvmsg +/// still works. #[cfg(any(target_os = "android", target_os = "linux"))] // qemu's handling of multiple cmsgs is bugged, ignore tests on non-x86 // see https://bugs.launchpad.net/qemu/+bug/1781280 #[cfg_attr(not(any(target_arch = "x86_64", target_arch = "x86")), ignore)] #[test] fn test_too_large_cmsgspace() { - use nix::sys::socket::CmsgSpace; - - test_impl_scm_credentials_and_rights(CmsgSpace::<[u8; 1024]>::new()); + let space = vec![0u8; 1024]; + test_impl_scm_credentials_and_rights(space); } #[cfg(any(target_os = "android", target_os = "linux"))] -fn test_impl_scm_credentials_and_rights(mut space: ::nix::sys::socket::CmsgSpace) { +fn test_impl_scm_credentials_and_rights(mut space: Vec) { use libc; use nix::sys::uio::IoVec; use nix::unistd::{pipe, read, write, close, getpid, getuid, getgid}; @@ -537,7 +542,7 @@ pub fn test_recv_ipv4pktinfo() { use nix::sys::socket::sockopt::Ipv4PacketInfo; use nix::sys::socket::{bind, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket}; - use nix::sys::socket::{recvmsg, sendmsg, CmsgSpace, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; use nix::sys::uio::IoVec; use nix::net::if_::*; @@ -573,7 +578,7 @@ pub fn test_recv_ipv4pktinfo() { { let mut buf = [0u8; 8]; let iovec = [IoVec::from_mut_slice(&mut buf)]; - let mut space = CmsgSpace::::new(); + let mut space = cmsg_space!(libc::in_pktinfo); let msg = recvmsg( receive, &iovec, @@ -627,7 +632,7 @@ pub fn test_recvif() { use nix::sys::socket::sockopt::{Ipv4RecvIf, Ipv4RecvDstAddr}; use nix::sys::socket::{bind, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket, SockAddr}; - use nix::sys::socket::{recvmsg, sendmsg, CmsgSpace, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; use nix::sys::uio::IoVec; let lo_ifaddr = loopback_address(AddressFamily::Inet); @@ -663,7 +668,7 @@ pub fn test_recvif() { { let mut buf = [0u8; 8]; let iovec = [IoVec::from_mut_slice(&mut buf)]; - let mut space = CmsgSpace::<(libc::sockaddr_dl, CmsgSpace)>::new(); + let mut space = cmsg_space!(libc::sockaddr_dl, libc::in_addr); let msg = recvmsg( receive, &iovec, @@ -737,7 +742,7 @@ pub fn test_recv_ipv6pktinfo() { use nix::sys::socket::sockopt::Ipv6RecvPacketInfo; use nix::sys::socket::{bind, AddressFamily, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket}; - use nix::sys::socket::{recvmsg, sendmsg, CmsgSpace, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; use nix::sys::uio::IoVec; let lo_ifaddr = loopback_address(AddressFamily::Inet6); @@ -772,7 +777,7 @@ pub fn test_recv_ipv6pktinfo() { { let mut buf = [0u8; 8]; let iovec = [IoVec::from_mut_slice(&mut buf)]; - let mut space = CmsgSpace::::new(); + let mut space = cmsg_space!(libc::in6_pktinfo); let msg = recvmsg( receive, &iovec, From 15ed5a539e4014da51c8ee8593ab6f46fba2d00c Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Tue, 12 Feb 2019 11:22:18 -0700 Subject: [PATCH 4/6] Fix misaligned references when using recvmsg with control messages On some platforms the alignment of cmsg_data could be less than the alignment of the messages that it contains. That led to unaligned reads on those platforms. This change fixes the issue by always copying the message contents into aligned objects. The change is not 100% backwards compatible when using recvmsg. Users may have to replace code like this: ```rust if let ControlMessage::ScmRights(&fds) = cmsg { ``` with this: ```rust if let ControlMessageOwned::ScmRights(fds) = cmsg { ``` Fixes #999 --- src/sys/socket/mod.rs | 425 +++++++++++++++------------------------- src/sys/time.rs | 7 + test/sys/test_socket.rs | 34 ++-- 3 files changed, 178 insertions(+), 288 deletions(-) diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index de6644bb96..771ce8bfcb 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -333,13 +333,13 @@ pub trait CmsgBuffer { /// # use nix::sys::time::TimeVal; /// # use std::os::unix::io::RawFd; /// # fn main() { -/// // Create a buffer for a `ControlMessage::ScmTimestamp` message +/// // Create a buffer for a `ControlMessageOwned::ScmTimestamp` message /// let _ = cmsg_space!(TimeVal); -/// // Create a buffer big enough for a `ControlMessage::ScmRights` message +/// // Create a buffer big enough for a `ControlMessageOwned::ScmRights` message /// // with two file descriptors /// let _ = cmsg_space!([RawFd; 2]); -/// // Create a buffer big enough for a `ControlMessage::ScmRights` message -/// // and a `ControlMessage::ScmTimestamp` message +/// // Create a buffer big enough for a `ControlMessageOwned::ScmRights` message +/// // and a `ControlMessageOwned::ScmTimestamp` message /// let _ = cmsg_space!(RawFd, TimeVal); /// # } /// ``` @@ -440,15 +440,15 @@ pub struct CmsgIterator<'a> { } impl<'a> Iterator for CmsgIterator<'a> { - type Item = ControlMessage<'a>; + type Item = ControlMessageOwned; - fn next(&mut self) -> Option> { + fn next(&mut self) -> Option { match self.cmsghdr { None => None, // No more messages Some(hdr) => { // Get the data. // Safe if cmsghdr points to valid data returned by recvmsg(2) - let cm = unsafe { Some(ControlMessage::decode_from(hdr))}; + let cm = unsafe { Some(ControlMessageOwned::decode_from(hdr))}; // Advance the internal pointer. Safe if mhdr and cmsghdr point // to valid data returned by recvmsg(2) self.cmsghdr = unsafe { @@ -461,36 +461,25 @@ impl<'a> Iterator for CmsgIterator<'a> { } } -/// A type-safe wrapper around a single control message. More types may -/// be added to this enum; do not exhaustively pattern-match it. +/// A type-safe wrapper around a single control message, as used with +/// [`recvmsg`](#fn.recvmsg). +/// /// [Further reading](http://man7.org/linux/man-pages/man3/cmsg.3.html) +// Nix version 0.13.0 and earlier used ControlMessage for both recvmsg and +// sendmsg. However, on some platforms the messages returned by recvmsg may be +// unaligned. ControlMessageOwned takes those messages by copy, obviating any +// alignment issues. +// +// See https://github.com/nix-rust/nix/issues/999 #[allow(missing_debug_implementations)] -pub enum ControlMessage<'a> { - /// A message of type `SCM_RIGHTS`, containing an array of file - /// descriptors passed between processes. - /// - /// See the description in the "Ancillary messages" section of the - /// [unix(7) man page](http://man7.org/linux/man-pages/man7/unix.7.html). - /// - /// Using multiple `ScmRights` messages for a single `sendmsg` call isn't recommended since it - /// causes platform-dependent behaviour: It might swallow all but the first `ScmRights` message - /// or fail with `EINVAL`. Instead, you can put all fds to be passed into a single `ScmRights` - /// message. - ScmRights(&'a [RawFd]), - /// A message of type `SCM_CREDENTIALS`, containing the pid, uid and gid of - /// a process connected to the socket. - /// - /// This is similar to the socket option `SO_PEERCRED`, but requires a - /// process to explicitly send its credentials. A process running as root is - /// allowed to specify any credentials, while credentials sent by other - /// processes are verified by the kernel. - /// - /// For further information, please refer to the - /// [`unix(7)`](http://man7.org/linux/man-pages/man7/unix.7.html) man page. - // FIXME: When `#[repr(transparent)]` is stable, use it on `UnixCredentials` - // and put that in here instead of a raw ucred. +pub enum ControlMessageOwned { + /// Received version of + /// [`ControlMessage::ScmRights`][#enum.ControlMessage.html#variant.ScmRights] + ScmRights(Vec), + /// Received version of + /// [`ControlMessage::ScmCredentials`][#enum.ControlMessage.html#variant.ScmCredentials] #[cfg(any(target_os = "android", target_os = "linux"))] - ScmCredentials(&'a libc::ucred), + ScmCredentials(libc::ucred), /// A message of type `SCM_TIMESTAMP`, containing the time the /// packet was received by the kernel. /// @@ -533,7 +522,7 @@ pub enum ControlMessage<'a> { /// let iov = [IoVec::from_mut_slice(&mut buffer)]; /// let r = recvmsg(in_socket, &iov, Some(&mut cmsgspace), flags).unwrap(); /// let rtime = match r.cmsgs().next() { - /// Some(ControlMessage::ScmTimestamp(rtime)) => rtime, + /// Some(ControlMessageOwned::ScmTimestamp(rtime)) => rtime, /// Some(_) => panic!("Unexpected control message"), /// None => panic!("No control message") /// }; @@ -549,14 +538,14 @@ pub enum ControlMessage<'a> { /// nix::unistd::close(in_socket).unwrap(); /// # } /// ``` - ScmTimestamp(&'a TimeVal), + ScmTimestamp(TimeVal), #[cfg(any( target_os = "android", target_os = "ios", target_os = "linux", target_os = "macos" ))] - Ipv4PacketInfo(&'a libc::in_pktinfo), + Ipv4PacketInfo(libc::in_pktinfo), #[cfg(any( target_os = "android", target_os = "freebsd", @@ -564,7 +553,7 @@ pub enum ControlMessage<'a> { target_os = "linux", target_os = "macos" ))] - Ipv6PacketInfo(&'a libc::in6_pktinfo), + Ipv6PacketInfo(libc::in6_pktinfo), #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -572,7 +561,7 @@ pub enum ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - Ipv4RecvIf(&'a libc::sockaddr_dl), + Ipv4RecvIf(libc::sockaddr_dl), #[cfg(any( target_os = "freebsd", target_os = "ios", @@ -580,17 +569,138 @@ pub enum ControlMessage<'a> { target_os = "netbsd", target_os = "openbsd", ))] - Ipv4RecvDstAddr(&'a libc::in_addr), - + Ipv4RecvDstAddr(libc::in_addr), /// Catch-all variant for unimplemented cmsg types. #[doc(hidden)] - Unknown(UnknownCmsg<'a>), + Unknown(UnknownCmsg), +} + +impl ControlMessageOwned { + /// Decodes a `ControlMessageOwned` from raw bytes. + /// + /// This is only safe to call if the data is correct for the message type + /// specified in the header. Normally, the kernel ensures that this is the + /// case. "Correct" in this case includes correct length, alignment and + /// actual content. + /// + /// Returns `None` if the data may be unaligned. In that case use + /// `ControlMessageOwned::decode_from`. + unsafe fn decode_from(header: &cmsghdr) -> ControlMessageOwned + { + let p = CMSG_DATA(header); + let len = header as *const _ as usize + header.cmsg_len as usize + - p as usize; + match (header.cmsg_level, header.cmsg_type) { + (libc::SOL_SOCKET, libc::SCM_RIGHTS) => { + let n = len / mem::size_of::(); + let mut fds = Vec::with_capacity(n); + for i in 0..n { + let fdp = (p as *const RawFd).offset(i as isize); + fds.push(ptr::read_unaligned(fdp)); + } + let cmo = ControlMessageOwned::ScmRights(fds); + cmo + }, + #[cfg(any(target_os = "android", target_os = "linux"))] + (libc::SOL_SOCKET, libc::SCM_CREDENTIALS) => { + let cred: libc::ucred = ptr::read_unaligned(p as *const _); + ControlMessageOwned::ScmCredentials(cred) + } + (libc::SOL_SOCKET, libc::SCM_TIMESTAMP) => { + let tv: libc::timeval = ptr::read_unaligned(p as *const _); + ControlMessageOwned::ScmTimestamp(TimeVal::from(tv)) + }, + #[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "ios", + target_os = "linux", + target_os = "macos" + ))] + (libc::IPPROTO_IPV6, libc::IPV6_PKTINFO) => { + let info = ptr::read_unaligned(p as *const libc::in6_pktinfo); + ControlMessageOwned::Ipv6PacketInfo(info) + } + #[cfg(any( + target_os = "android", + target_os = "ios", + target_os = "linux", + target_os = "macos" + ))] + (libc::IPPROTO_IP, libc::IP_PKTINFO) => { + let info = ptr::read_unaligned(p as *const libc::in_pktinfo); + ControlMessageOwned::Ipv4PacketInfo(info) + } + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + (libc::IPPROTO_IP, libc::IP_RECVIF) => { + let dl = ptr::read_unaligned(p as *const libc::sockaddr_dl); + ControlMessageOwned::Ipv4RecvIf(dl) + }, + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + (libc::IPPROTO_IP, libc::IP_RECVDSTADDR) => { + let dl = ptr::read_unaligned(p as *const libc::in_addr); + ControlMessageOwned::Ipv4RecvDstAddr(dl) + }, + (_, _) => { + let sl = slice::from_raw_parts(p, len); + let ucmsg = UnknownCmsg(*header, Vec::::from(&sl[..])); + ControlMessageOwned::Unknown(ucmsg) + } + } + } +} + +/// A type-safe zero-copy wrapper around a single control message, as used wih +/// [`sendmsg`](#fn.sendmsg). More types may be added to this enum; do not +/// exhaustively pattern-match it. +/// +/// [Further reading](http://man7.org/linux/man-pages/man3/cmsg.3.html) +#[allow(missing_debug_implementations)] +pub enum ControlMessage<'a> { + /// A message of type `SCM_RIGHTS`, containing an array of file + /// descriptors passed between processes. + /// + /// See the description in the "Ancillary messages" section of the + /// [unix(7) man page](http://man7.org/linux/man-pages/man7/unix.7.html). + /// + /// Using multiple `ScmRights` messages for a single `sendmsg` call isn't + /// recommended since it causes platform-dependent behaviour: It might + /// swallow all but the first `ScmRights` message or fail with `EINVAL`. + /// Instead, you can put all fds to be passed into a single `ScmRights` + /// message. + ScmRights(&'a [RawFd]), + /// A message of type `SCM_CREDENTIALS`, containing the pid, uid and gid of + /// a process connected to the socket. + /// + /// This is similar to the socket option `SO_PEERCRED`, but requires a + /// process to explicitly send its credentials. A process running as root is + /// allowed to specify any credentials, while credentials sent by other + /// processes are verified by the kernel. + /// + /// For further information, please refer to the + /// [`unix(7)`](http://man7.org/linux/man-pages/man7/unix.7.html) man page. + // FIXME: When `#[repr(transparent)]` is stable, use it on `UnixCredentials` + // and put that in here instead of a raw ucred. + #[cfg(any(target_os = "android", target_os = "linux"))] + ScmCredentials(&'a libc::ucred), } // An opaque structure used to prevent cmsghdr from being a public type #[doc(hidden)] #[allow(missing_debug_implementations)] -pub struct UnknownCmsg<'a>(&'a cmsghdr, &'a [u8]); +pub struct UnknownCmsg(cmsghdr, Vec); impl<'a> ControlMessage<'a> { /// The value of CMSG_SPACE on this message. @@ -617,57 +727,12 @@ impl<'a> ControlMessage<'a> { fn data(&self) -> *const u8 { match self { &ControlMessage::ScmRights(fds) => { - fds as *const [RawFd] as *const u8 + fds as *const _ as *const u8 }, #[cfg(any(target_os = "android", target_os = "linux"))] &ControlMessage::ScmCredentials(creds) => { creds as *const libc::ucred as *const u8 } - &ControlMessage::ScmTimestamp(t) => { - t as *const TimeVal as *const u8 - }, - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv4PacketInfo(pktinfo) => { - pktinfo as *const libc::in_pktinfo as *const u8 - }, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv6PacketInfo(pktinfo) => { - pktinfo as *const libc::in6_pktinfo as *const u8 - }, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvIf(dl) => { - dl as *const libc::sockaddr_dl as *const u8 - }, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvDstAddr(in_addr) => { - in_addr as *const libc::in_addr as *const u8 - }, - &ControlMessage::Unknown(UnknownCmsg(_, bytes)) => { - bytes as *const _ as *const u8 - } } } @@ -681,51 +746,6 @@ impl<'a> ControlMessage<'a> { &ControlMessage::ScmCredentials(creds) => { mem::size_of_val(creds) } - &ControlMessage::ScmTimestamp(t) => { - mem::size_of_val(t) - }, - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv4PacketInfo(pktinfo) => { - mem::size_of_val(pktinfo) - }, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv6PacketInfo(pktinfo) => { - mem::size_of_val(pktinfo) - }, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvIf(dl) => { - mem::size_of_val(dl) - }, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvDstAddr(inaddr) => { - mem::size_of_val(inaddr) - }, - &ControlMessage::Unknown(UnknownCmsg(_, bytes)) => { - mem::size_of_val(bytes) - } } } @@ -735,39 +755,6 @@ impl<'a> ControlMessage<'a> { &ControlMessage::ScmRights(_) => libc::SOL_SOCKET, #[cfg(any(target_os = "android", target_os = "linux"))] &ControlMessage::ScmCredentials(_) => libc::SOL_SOCKET, - &ControlMessage::ScmTimestamp(_) => libc::SOL_SOCKET, - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv4PacketInfo(_) => libc::IPPROTO_IP, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv6PacketInfo(_) => libc::IPPROTO_IPV6, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvIf(_) => libc::IPPROTO_IP, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvDstAddr(_) => libc::IPPROTO_IP, - &ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_level, } } @@ -777,39 +764,6 @@ impl<'a> ControlMessage<'a> { &ControlMessage::ScmRights(_) => libc::SCM_RIGHTS, #[cfg(any(target_os = "android", target_os = "linux"))] &ControlMessage::ScmCredentials(_) => libc::SCM_CREDENTIALS, - &ControlMessage::ScmTimestamp(_) => libc::SCM_TIMESTAMP, - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv4PacketInfo(_) => libc::IP_PKTINFO, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - &ControlMessage::Ipv6PacketInfo(_) => libc::IPV6_PKTINFO, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvIf(_) => libc::IP_RECVIF, - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - &ControlMessage::Ipv4RecvDstAddr(_) => libc::IP_RECVDSTADDR, - &ControlMessage::Unknown(ref cmsg) => cmsg.0.cmsg_type, } } @@ -826,77 +780,6 @@ impl<'a> ControlMessage<'a> { self.len() ); } - - /// Decodes a `ControlMessage` from raw bytes. - /// - /// This is only safe to call if the data is correct for the message type - /// specified in the header. Normally, the kernel ensures that this is the - /// case. "Correct" in this case includes correct length, alignment and - /// actual content. - unsafe fn decode_from(header: &'a cmsghdr) -> ControlMessage<'a> - { - let p = CMSG_DATA(header); - let len = header as *const _ as usize + header.cmsg_len as usize - - p as usize; - match (header.cmsg_level, header.cmsg_type) { - (libc::SOL_SOCKET, libc::SCM_RIGHTS) => { - ControlMessage::ScmRights( - slice::from_raw_parts(p as *const RawFd, - len / mem::size_of::())) - }, - #[cfg(any(target_os = "android", target_os = "linux"))] - (libc::SOL_SOCKET, libc::SCM_CREDENTIALS) => { - ControlMessage::ScmCredentials(&*(p as *const _)) - } - (libc::SOL_SOCKET, libc::SCM_TIMESTAMP) => { - ControlMessage::ScmTimestamp(&*(p as *const _)) - }, - #[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - (libc::IPPROTO_IPV6, libc::IPV6_PKTINFO) => { - ControlMessage::Ipv6PacketInfo(&*(p as *const _)) - } - #[cfg(any( - target_os = "android", - target_os = "ios", - target_os = "linux", - target_os = "macos" - ))] - (libc::IPPROTO_IP, libc::IP_PKTINFO) => { - ControlMessage::Ipv4PacketInfo(&*(p as *const _)) - } - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - (libc::IPPROTO_IP, libc::IP_RECVIF) => { - ControlMessage::Ipv4RecvIf(&*(p as *const _)) - } - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - ))] - (libc::IPPROTO_IP, libc::IP_RECVDSTADDR) => { - ControlMessage::Ipv4RecvDstAddr(&*(p as *const _)) - } - - (_, _) => { - let data = slice::from_raw_parts(p, len); - ControlMessage::Unknown(UnknownCmsg(header, data)) - } - } - } } diff --git a/src/sys/time.rs b/src/sys/time.rs index 9671f5310a..4bd3b7808d 100644 --- a/src/sys/time.rs +++ b/src/sys/time.rs @@ -1,4 +1,5 @@ use std::{cmp, fmt, ops}; +use std::convert::From; use libc::{c_long, timespec, timeval}; pub use libc::{time_t, suseconds_t}; @@ -467,6 +468,12 @@ impl fmt::Display for TimeVal { } } +impl From for TimeVal { + fn from(tv: timeval) -> Self { + TimeVal(tv) + } +} + #[inline] fn div_mod_floor_64(this: i64, other: i64) -> (i64, i64) { (div_floor_64(this, other), mod_floor_64(this, other)) diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index 5a40574dfd..5d55c87dc5 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -141,7 +141,7 @@ pub fn test_scm_rights() { use nix::unistd::{pipe, read, write, close}; use nix::sys::socket::{socketpair, sendmsg, recvmsg, AddressFamily, SockType, SockFlag, - ControlMessage, MsgFlags}; + ControlMessage, ControlMessageOwned, MsgFlags}; let (fd1, fd2) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) .unwrap(); @@ -164,7 +164,7 @@ pub fn test_scm_rights() { let msg = recvmsg(fd2, &iov, Some(&mut cmsgspace), MsgFlags::empty()).unwrap(); for cmsg in msg.cmsgs() { - if let ControlMessage::ScmRights(fd) = cmsg { + if let ControlMessageOwned::ScmRights(fd) = cmsg { assert_eq!(received_r, None); assert_eq!(fd.len(), 1); received_r = Some(fd[0]); @@ -195,7 +195,7 @@ fn test_scm_rights_single_cmsg_multiple_fds() { use std::os::unix::net::UnixDatagram; use std::os::unix::io::{RawFd, AsRawFd}; use std::thread; - use nix::sys::socket::{ControlMessage, MsgFlags, + use nix::sys::socket::{ControlMessage, ControlMessageOwned, MsgFlags, sendmsg, recvmsg}; use nix::sys::uio::IoVec; use libc; @@ -215,7 +215,7 @@ fn test_scm_rights_single_cmsg_multiple_fds() { let mut cmsgs = msg.cmsgs(); match cmsgs.next() { - Some(ControlMessage::ScmRights(fds)) => { + Some(ControlMessageOwned::ScmRights(fds)) => { assert_eq!(fds.len(), 2, "unexpected fd count (expected 2 fds, got {})", fds.len()); @@ -277,7 +277,7 @@ fn test_scm_credentials() { use nix::unistd::{close, getpid, getuid, getgid}; use nix::sys::socket::{socketpair, sendmsg, recvmsg, setsockopt, AddressFamily, SockType, SockFlag, - ControlMessage, MsgFlags}; + ControlMessage, ControlMessageOwned, MsgFlags}; use nix::sys::socket::sockopt::PassCred; let (send, recv) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) @@ -304,7 +304,7 @@ fn test_scm_credentials() { let mut received_cred = None; for cmsg in msg.cmsgs() { - if let ControlMessage::ScmCredentials(cred) = cmsg { + if let ControlMessageOwned::ScmCredentials(cred) = cmsg { assert!(received_cred.is_none()); assert_eq!(cred.pid, getpid().as_raw()); assert_eq!(cred.uid, getuid().as_raw()); @@ -353,7 +353,7 @@ fn test_impl_scm_credentials_and_rights(mut space: Vec) { use nix::unistd::{pipe, read, write, close, getpid, getuid, getgid}; use nix::sys::socket::{socketpair, sendmsg, recvmsg, setsockopt, AddressFamily, SockType, SockFlag, - ControlMessage, MsgFlags}; + ControlMessage, ControlMessageOwned, MsgFlags}; use nix::sys::socket::sockopt::PassCred; let (send, recv) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()) @@ -390,17 +390,17 @@ fn test_impl_scm_credentials_and_rights(mut space: Vec) { for cmsg in msg.cmsgs() { match cmsg { - ControlMessage::ScmRights(fds) => { + ControlMessageOwned::ScmRights(fds) => { assert_eq!(received_r, None, "already received fd"); assert_eq!(fds.len(), 1); received_r = Some(fds[0]); } - ControlMessage::ScmCredentials(cred) => { + ControlMessageOwned::ScmCredentials(cred) => { assert!(received_cred.is_none()); assert_eq!(cred.pid, getpid().as_raw()); assert_eq!(cred.uid, getuid().as_raw()); assert_eq!(cred.gid, getgid().as_raw()); - received_cred = Some(*cred); + received_cred = Some(cred); } _ => panic!("unexpected cmsg"), } @@ -542,7 +542,7 @@ pub fn test_recv_ipv4pktinfo() { use nix::sys::socket::sockopt::Ipv4PacketInfo; use nix::sys::socket::{bind, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket}; - use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessageOwned, MsgFlags}; use nix::sys::uio::IoVec; use nix::net::if_::*; @@ -592,7 +592,7 @@ pub fn test_recv_ipv4pktinfo() { let mut cmsgs = msg.cmsgs(); match cmsgs.next() { - Some(ControlMessage::Ipv4PacketInfo(pktinfo)) => { + Some(ControlMessageOwned::Ipv4PacketInfo(pktinfo)) => { let i = if_nametoindex(lo_name.as_bytes()).expect("if_nametoindex"); assert_eq!( pktinfo.ipi_ifindex as libc::c_uint, @@ -632,7 +632,7 @@ pub fn test_recvif() { use nix::sys::socket::sockopt::{Ipv4RecvIf, Ipv4RecvDstAddr}; use nix::sys::socket::{bind, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket, SockAddr}; - use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessageOwned, MsgFlags}; use nix::sys::uio::IoVec; let lo_ifaddr = loopback_address(AddressFamily::Inet); @@ -685,7 +685,7 @@ pub fn test_recvif() { let mut rx_recvdstaddr = false; for cmsg in msg.cmsgs() { match cmsg { - ControlMessage::Ipv4RecvIf(dl) => { + ControlMessageOwned::Ipv4RecvIf(dl) => { rx_recvif = true; let i = if_nametoindex(lo_name.as_bytes()).expect("if_nametoindex"); assert_eq!( @@ -696,7 +696,7 @@ pub fn test_recvif() { dl.sdl_index ); }, - ControlMessage::Ipv4RecvDstAddr(addr) => { + ControlMessageOwned::Ipv4RecvDstAddr(addr) => { rx_recvdstaddr = true; if let SockAddr::Inet(InetAddr::V4(a)) = lo { assert_eq!(a.sin_addr.s_addr, @@ -742,7 +742,7 @@ pub fn test_recv_ipv6pktinfo() { use nix::sys::socket::sockopt::Ipv6RecvPacketInfo; use nix::sys::socket::{bind, AddressFamily, SockFlag, SockType}; use nix::sys::socket::{getsockname, setsockopt, socket}; - use nix::sys::socket::{recvmsg, sendmsg, ControlMessage, MsgFlags}; + use nix::sys::socket::{recvmsg, sendmsg, ControlMessageOwned, MsgFlags}; use nix::sys::uio::IoVec; let lo_ifaddr = loopback_address(AddressFamily::Inet6); @@ -791,7 +791,7 @@ pub fn test_recv_ipv6pktinfo() { let mut cmsgs = msg.cmsgs(); match cmsgs.next() { - Some(ControlMessage::Ipv6PacketInfo(pktinfo)) => { + Some(ControlMessageOwned::Ipv6PacketInfo(pktinfo)) => { let i = if_nametoindex(lo_name.as_bytes()).expect("if_nametoindex"); assert_eq!( pktinfo.ipi6_ifindex, From 2966dcd52098a2f20e0bb8e7e6f9655fbd00d792 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 30 Jan 2019 13:45:39 -0700 Subject: [PATCH 5/6] Add CHANGELOG entry. --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0106966ac6..ec74eb270d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,25 @@ This project adheres to [Semantic Versioning](http://semver.org/). ([#1002](https://github.com/nix-rust/nix/pull/1002)) ### Changed - `PollFd` event flags renamed to `PollFlags` ([#1024](https://github.com/nix-rust/nix/pull/1024/)) +- `recvmsg` now returns an Iterator over `ControlMessageOwned` objects rather + than `ControlMessage` objects. This is sadly not backwards-compatible. Fix + code like this: + ```rust + if let ControlMessage::ScmRights(&fds) = cmsg { + ``` + + By replacing it with code like this: + ```rust + if let ControlMessageOwned::ScmRights(fds) = cmsg { + ``` + ([#1020](https://github.com/nix-rust/nix/pull/1020)) +- Replaced `CmsgSpace` with the `cmsg_space` macro. + ([#1020](https://github.com/nix-rust/nix/pull/1020)) + ### Fixed +- Fixed multiple bugs when using `sendmsg` and `recvmsg` with ancillary control messages + ([#1020](https://github.com/nix-rust/nix/pull/1020)) + ### Removed ## [0.13.0] - 2019-01-15 From b5c4c7a9c043afc97ced704c1edd482e12903234 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 31 Jan 2019 08:29:54 -0700 Subject: [PATCH 6/6] Enable IPv4PacketInfo and Ipv6PacketInfo on more OSes. This was an oversight from PR #1002 --- src/sys/socket/mod.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index 771ce8bfcb..8000991773 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -543,15 +543,19 @@ pub enum ControlMessageOwned { target_os = "android", target_os = "ios", target_os = "linux", - target_os = "macos" + target_os = "macos", + target_os = "netbsd", ))] Ipv4PacketInfo(libc::in_pktinfo), #[cfg(any( target_os = "android", + target_os = "dragonfly", target_os = "freebsd", target_os = "ios", target_os = "linux", - target_os = "macos" + target_os = "macos", + target_os = "openbsd", + target_os = "netbsd", ))] Ipv6PacketInfo(libc::in6_pktinfo), #[cfg(any( @@ -625,7 +629,8 @@ impl ControlMessageOwned { target_os = "android", target_os = "ios", target_os = "linux", - target_os = "macos" + target_os = "macos", + target_os = "netbsd", ))] (libc::IPPROTO_IP, libc::IP_PKTINFO) => { let info = ptr::read_unaligned(p as *const libc::in_pktinfo);