fixes UB for recvmmsg, simplifies function signature of recvmmsg and sendmmsg

Author: Jan561

CHANGELOG.md

@@ -3,6 +3,17 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](https://semver.org/).
 
+## [0.28.0] - YYYY-MM-DD
+
+### Fixed
+
+- Fixed the function signature of `recvmmsg`, potentially causing UB
+  ([#2119](https://github.com/nix-rust/nix/issues/2119))
+
+### Changed
+
+- Simplified the function signatures of `recvmmsg` and `sendmmsg`
+
 ## [0.27.1] - 2023-08-28
 
 ### Fixed

src/sys/personality.rs

@@ -80,10 +80,9 @@ pub fn get() -> Result<Persona> {
 ///
 /// Example:
 ///
-// Disable test on aarch64 until we know why it fails.
+// Disable test until we know why it fails.
 // https://github.com/nix-rust/nix/issues/2060
-#[cfg_attr(target_arch = "aarch64", doc = " ```no_run")]
-#[cfg_attr(not(target_arch = "aarch64"), doc = " ```")]
+#[doc = " ```no_run"]
 /// # use nix::sys::personality::{self, Persona};
 /// let mut pers = personality::get().unwrap();
 /// assert!(!pers.contains(Persona::ADDR_NO_RANDOMIZE));

src/sys/socket/mod.rs

@@ -1552,11 +1552,11 @@ pub fn sendmmsg<'a, XS, AS, C, I, S>(
     flags: MsgFlags
 ) -> crate::Result<MultiResults<'a, S>>
     where
-        XS: IntoIterator<Item = &'a I>,
+        XS: IntoIterator<Item = I>,
         AS: AsRef<[Option<S>]>,
-        I: AsRef<[IoSlice<'a>]> + 'a,
-        C: AsRef<[ControlMessage<'a>]> + 'a,
-        S: SockaddrLike + 'a
+        I: AsRef<[IoSlice<'a>]>,
+        C: AsRef<[ControlMessage<'a>]>,
+        S: SockaddrLike,
 {
 
     let mut count = 0;
@@ -1583,9 +1583,16 @@ pub fn sendmmsg<'a, XS, AS, C, I, S>(
             pmhdr = unsafe { CMSG_NXTHDR(p, pmhdr) };
         }
 
-        count = i+1;
+        // Doing an unchecked addition is alright here, as the only way to obtain an instance of `MultiHeaders`
+        // is through the `preallocate` function, which takes an `usize` as an argument to define its size,
+        // which also provides an upper bound for the size of this zipped iterator. Thus, `i < usize::MAX` or in
+        // other words: `count` doesn't overflow
+        count = i + 1;
     }
 
+    // SAFETY: all pointers are guaranteed to be valid for the scope of this function. `count` does represent the
+    // maximum number of messages that can be sent safely (i.e. `count` is the minimum of the sizes of `slices`,
+    // `data.items` and `addrs`)
     let sent = Errno::result(unsafe {
         libc::sendmmsg(
             fd,
@@ -1711,21 +1718,28 @@ pub fn recvmmsg<'a, XS, S, I>(
     mut timeout: Option<crate::sys::time::TimeSpec>,
 ) -> crate::Result<MultiResults<'a, S>>
 where
-    XS: IntoIterator<Item = &'a I>,
-    I: AsRef<[IoSliceMut<'a>]> + 'a,
+    XS: IntoIterator<Item = I>,
+    I: AsMut<[IoSliceMut<'a>]>,
 {
     let mut count = 0;
-    for (i, (slice, mmsghdr)) in slices.into_iter().zip(data.items.iter_mut()).enumerate() {
+    for (i, (mut slice, mmsghdr)) in slices.into_iter().zip(data.items.iter_mut()).enumerate() {
         let p = &mut mmsghdr.msg_hdr;
-        p.msg_iov = slice.as_ref().as_ptr() as *mut libc::iovec;
-        p.msg_iovlen = slice.as_ref().len() as _;
+        p.msg_iov = slice.as_mut().as_mut_ptr().cast();
+        p.msg_iovlen = slice.as_mut().len() as _;
+
+        // Doing an unchecked addition is alright here, as the only way to obtain an instance of `MultiHeaders`
+        // is through the `preallocate` function, which takes an `usize` as an argument to define its size,
+        // which also provides an upper bound for the size of this zipped iterator. Thus, `i < usize::MAX` or in
+        // other words: `count` doesn't overflow
         count = i + 1;
     }
 
     let timeout_ptr = timeout
         .as_mut()
         .map_or_else(std::ptr::null_mut, |t| t as *mut _ as *mut libc::timespec);
 
+    // SAFETY: all pointers are guaranteed to be valid for the scope of this function. `count` does represent the
+    // maximum number of messages that can be received safely (i.e. `count` is the minimum of the sizes of `slices` and `data.items`)
     let received = Errno::result(unsafe {
         libc::recvmmsg(
             fd,
@@ -1903,7 +1917,7 @@ mod test {
 
         let t = sys::time::TimeSpec::from_duration(std::time::Duration::from_secs(10));
 
-        let recv = super::recvmmsg(rsock.as_raw_fd(), &mut data, recv_iovs.iter(), flags, Some(t))?;
+        let recv = super::recvmmsg(rsock.as_raw_fd(), &mut data, recv_iovs.iter_mut(), flags, Some(t))?;
 
         for rmsg in recv {
             #[cfg(not(any(qemu, target_arch = "aarch64")))]

test/sys/test_socket.rs

@@ -565,7 +565,7 @@ mod recvfrom {
         let res: Vec<RecvMsg<SockaddrIn>> = recvmmsg(
             rsock.as_raw_fd(),
             &mut data,
-            msgs.iter(),
+            msgs.iter_mut(),
             MsgFlags::empty(),
             None,
         )
@@ -653,7 +653,7 @@ mod recvfrom {
         let res: Vec<RecvMsg<SockaddrIn>> = recvmmsg(
             rsock.as_raw_fd(),
             &mut data,
-            msgs.iter(),
+            msgs.iter_mut(),
             MsgFlags::MSG_DONTWAIT,
             None,
         )
@@ -2329,12 +2329,17 @@ fn test_recvmmsg_timestampns() {
     // Receive the message
     let mut buffer = vec![0u8; message.len()];
     let cmsgspace = nix::cmsg_space!(TimeSpec);
-    let iov = vec![[IoSliceMut::new(&mut buffer)]];
+    let mut iov = vec![[IoSliceMut::new(&mut buffer)]];
     let mut data = MultiHeaders::preallocate(1, Some(cmsgspace));
-    let r: Vec<RecvMsg<()>> =
-        recvmmsg(in_socket.as_raw_fd(), &mut data, iov.iter(), flags, None)
-            .unwrap()
-            .collect();
+    let r: Vec<RecvMsg<()>> = recvmmsg(
+        in_socket.as_raw_fd(),
+        &mut data,
+        iov.iter_mut(),
+        flags,
+        None,
+    )
+    .unwrap()
+    .collect();
     let rtime = match r[0].cmsgs().next() {
         Some(ControlMessageOwned::ScmTimestampns(rtime)) => rtime,
         Some(_) => panic!("Unexpected control message"),