Skip to content

Commit

Permalink
Replace CmsgSpace with a macro
Browse files Browse the repository at this point in the history 
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 nix-rust#994
  • Loading branch information
@asomers
asomers committed Feb 12, 2019
1 parent 24513ef commit 9f69c7d
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 43 deletions.
106 changes: 88 additions & 18 deletions src/sys/socket/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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>,
Expand Down Expand Up @@ -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(
Expand All @@ -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")
/// };
Expand All @@ -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",
Expand Down Expand Up @@ -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 = {
Expand Down
55 changes: 30 additions & 25 deletions test/sys/test_socket.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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() {
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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() {
Expand All @@ -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())
Expand All @@ -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;

Expand All @@ -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");
}
Expand All @@ -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};
Expand Down Expand Up @@ -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_::*;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 9f69c7d

Please sign in to comment.