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

Author: asomers

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::<u8>::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<T> {
 impl<T> CmsgSpace<T> {
     /// Create a CmsgSpace<T>. 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<T> CmsgBuffer for CmsgSpace<T> {
+    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<T> as *mut u8,
+                                      mem::size_of::<Self>())
+        }
+    }
+}
+
+impl CmsgBuffer for Vec<u8> {
+    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<TimeVal> = 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<T>>,
-                      flags: MsgFlags) -> Result<RecvMsg<'a>>
+pub fn recvmsg<'a>(fd: RawFd, iov: &[IoVec<&mut [u8]>],
+                   cmsg_buffer: Option<&'a mut CmsgBuffer>,
+                   flags: MsgFlags) -> Result<RecvMsg<'a>>
 {
     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 = {

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,20 +187,24 @@ 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;
 
     let (send, receive) = UnixDatagram::pair().unwrap();
     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<libc::ucred> = 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<RawFd>)>::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<T>(mut space: ::nix::sys::socket::CmsgSpace<T>) {
+fn test_impl_scm_credentials_and_rights(mut space: Vec<u8>) {
     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::<libc::in_pktinfo>::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<libc::in_addr>)>::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::<libc::in6_pktinfo>::new();
+        let mut space = cmsg_space!(libc::in6_pktinfo);
         let msg = recvmsg(
             receive,
             &iovec,