Fixes for empty and multiple-entry control message buffers. (#915)

* Fixes for empty and multiple-entry control message buffers. - Fix the handling of empty buffers passed to `SendAncillaryBuffer::new` and `RecvAncillaryBuffer::new`. - Fix the buffer size computation for multiple messages to include only one copy of the padding for alignment. * Update to qemu 8.1.2. * Update to linux-raw-sys 0.4.11 In particular, this brings in sunfishcode/linux-raw-sys#92, a fix for the `CMSG_NXTHDR` macro which fixes test failures in this PR.
bytecodealliance · Nov 8, 2023 · 31dfad1 · 31dfad1
1 parent 1edcdfe
commit 31dfad1
Show file tree

Hide file tree

Showing 7 changed files with 617 additions and 10 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -226,7 +226,7 @@ jobs:
     name: Test
     runs-on: ${{ matrix.os }}
     env:
-      QEMU_BUILD_VERSION: 8.1.0
+      QEMU_BUILD_VERSION: 8.1.2
       # Enabling testing of experimental features.
       RUSTFLAGS: --cfg rustix_use_experimental_features
     strategy:
@@ -547,7 +547,7 @@ jobs:
     env:
       # -D warnings is commented out in our install-rust action; re-add it here.
       RUSTFLAGS: -D warnings -D elided-lifetimes-in-paths
-      QEMU_BUILD_VERSION: 8.1.0
+      QEMU_BUILD_VERSION: 8.1.2
     steps:
     - uses: actions/checkout@v3
       with:
@@ -639,7 +639,7 @@ jobs:
       RUSTFLAGS: --cfg rustix_use_experimental_asm -D warnings -D elided-lifetimes-in-paths
       RUSTDOCFLAGS: --cfg rustix_use_experimental_asm
       CARGO_TARGET_POWERPC64LE_UNKNOWN_LINUX_GNU_RUSTFLAGS: --cfg rustix_use_experimental_asm
-      QEMU_BUILD_VERSION: 8.1.0
+      QEMU_BUILD_VERSION: 8.1.2
     steps:
     - uses: actions/checkout@v3
       with:

diff --git a/Cargo.toml b/Cargo.toml
@@ -36,7 +36,7 @@ once_cell = { version = "1.5.2", optional = true }
 # libc backend can be selected via adding `--cfg=rustix_use_libc` to
 # `RUSTFLAGS` or enabling the `use-libc` cargo feature.
 [target.'cfg(all(not(rustix_use_libc), not(miri), target_os = "linux", target_endian = "little", any(target_arch = "arm", all(target_arch = "aarch64", target_pointer_width = "64"), target_arch = "riscv64", all(rustix_use_experimental_asm, target_arch = "powerpc64"), all(rustix_use_experimental_asm, target_arch = "mips"), all(rustix_use_experimental_asm, target_arch = "mips32r6"), all(rustix_use_experimental_asm, target_arch = "mips64"), all(rustix_use_experimental_asm, target_arch = "mips64r6"), target_arch = "x86", all(target_arch = "x86_64", target_pointer_width = "64"))))'.dependencies]
-linux-raw-sys = { version = "0.4.8", default-features = false, features = ["general", "errno", "ioctl", "no_std", "elf"] }
+linux-raw-sys = { version = "0.4.11", default-features = false, features = ["general", "errno", "ioctl", "no_std", "elf"] }
 libc_errno = { package = "errno", version = "0.3.1", default-features = false, optional = true }
 libc = { version = "0.2.150", default-features = false, features = ["extra_traits"], optional = true }
 
@@ -53,7 +53,7 @@ libc = { version = "0.2.150", default-features = false, features = ["extra_trait
 # Some syscalls do not have libc wrappers, such as in `io_uring`. For these,
 # the libc backend uses the linux-raw-sys ABI and `libc::syscall`.
 [target.'cfg(all(any(target_os = "android", target_os = "linux"), any(rustix_use_libc, miri, not(all(target_os = "linux", target_endian = "little", any(target_arch = "arm", all(target_arch = "aarch64", target_pointer_width = "64"), target_arch = "riscv64", all(rustix_use_experimental_asm, target_arch = "powerpc64"), all(rustix_use_experimental_asm, target_arch = "mips"), all(rustix_use_experimental_asm, target_arch = "mips32r6"), all(rustix_use_experimental_asm, target_arch = "mips64"), all(rustix_use_experimental_asm, target_arch = "mips64r6"), target_arch = "x86", all(target_arch = "x86_64", target_pointer_width = "64")))))))'.dependencies]
-linux-raw-sys = { version = "0.4.8", default-features = false, features = ["general", "ioctl", "no_std"] }
+linux-raw-sys = { version = "0.4.11", default-features = false, features = ["general", "ioctl", "no_std"] }
 
 # For the libc backend on Windows, use the Winsock2 API in windows-sys.
 [target.'cfg(windows)'.dependencies.windows-sys]

diff --git a/src/net/send_recv/msg.rs b/src/net/send_recv/msg.rs
@@ -17,7 +17,34 @@ use core::{ptr, slice};
 
 use super::{RecvFlags, SendFlags, SocketAddrAny, SocketAddrV4, SocketAddrV6};
 
-/// Macro for defining the amount of space used by CMSGs.
+/// Macro for defining the amount of space to allocate in a buffer for use with
+/// [`RecvAncillaryBuffer::new`] and [`SendAncillaryBuffer::new`].
+///
+/// # Examples
+///
+/// Allocate a buffer for a single file descriptor:
+/// ```
+/// # use rustix::cmsg_space;
+/// let mut space = [0; rustix::cmsg_space!(ScmRights(1))];
+/// ```
+///
+/// Allocate a buffer for credentials:
+/// ```
+/// # #[cfg(linux_kernel)]
+/// # {
+/// # use rustix::cmsg_space;
+/// let mut space = [0; rustix::cmsg_space!(ScmCredentials(1))];
+/// # }
+/// ```
+///
+/// Allocate a buffer for two file descriptors and credentials:
+/// ```
+/// # #[cfg(linux_kernel)]
+/// # {
+/// # use rustix::cmsg_space;
+/// let mut space = [0; rustix::cmsg_space!(ScmRights(2), ScmCredentials(1))];
+/// # }
+/// ```
 #[macro_export]
 macro_rules! cmsg_space {
     // Base Rules
@@ -33,12 +60,41 @@ macro_rules! cmsg_space {
     };
 
     // Combo Rules
-    (($($($x:tt)*),+)) => {
+    ($firstid:ident($firstex:expr), $($restid:ident($restex:expr)),*) => {{
+        // We only have to add `cmsghdr` alignment once; all other times we can
+        // use `cmsg_aligned_space`.
+        let sum = $crate::cmsg_space!($firstid($firstex));
         $(
-            cmsg_space!($($x)*) +
-        )+
-        0
+            let sum = sum + $crate::cmsg_aligned_space!($restid($restex));
+        )*
+        sum
+    }};
+}
+
+/// Like `cmsg_space`, but doesn't add padding for `cmsghdr` alignment.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! cmsg_aligned_space {
+    // Base Rules
+    (ScmRights($len:expr)) => {
+        $crate::net::__cmsg_aligned_space(
+            $len * ::core::mem::size_of::<$crate::fd::BorrowedFd<'static>>(),
+        )
+    };
+    (ScmCredentials($len:expr)) => {
+        $crate::net::__cmsg_aligned_space(
+            $len * ::core::mem::size_of::<$crate::net::UCred>(),
+        )
     };
+
+    // Combo Rules
+    ($firstid:ident($firstex:expr), $($restid:ident($restex:expr)),*) => {{
+        let sum = cmsg_aligned_space!($firstid($firstex));
+        $(
+            let sum = sum + cmsg_aligned_space!($restid($restex));
+        )*
+        sum
+    }};
 }
 
 #[doc(hidden)]
@@ -47,6 +103,11 @@ pub const fn __cmsg_space(len: usize) -> usize {
     // `&[u8]` to the required alignment boundary.
     let len = len + align_of::<c::cmsghdr>();
 
+    __cmsg_aligned_space(len)
+}
+
+#[doc(hidden)]
+pub const fn __cmsg_aligned_space(len: usize) -> usize {
     // Convert `len` to `u32` for `CMSG_SPACE`. This would be `try_into()` if
     // we could call that in a `const fn`.
     let converted_len = len as u32;
@@ -97,6 +158,10 @@ pub enum RecvAncillaryMessage<'a> {
 
 /// Buffer for sending ancillary messages with [`sendmsg`], [`sendmsg_v4`],
 /// [`sendmsg_v6`], [`sendmsg_unix`], and [`sendmsg_any`].
+///
+/// Use the [`push`] function to add messages to send.
+///
+/// [`push`]: SendAncillaryBuffer::push
 pub struct SendAncillaryBuffer<'buf, 'slice, 'fd> {
     /// Raw byte buffer for messages.
     buffer: &'buf mut [u8],
@@ -126,6 +191,44 @@ impl Default for SendAncillaryBuffer<'_, '_, '_> {
 
 impl<'buf, 'slice, 'fd> SendAncillaryBuffer<'buf, 'slice, 'fd> {
     /// Create a new, empty `SendAncillaryBuffer` from a raw byte buffer.
+    ///
+    /// The buffer size may be computed with [`cmsg_space`], or it may be
+    /// zero for an empty buffer, however in that case, consider `default()`
+    /// instead, or even using [`send`] instead of `sendmsg`.
+    ///
+    /// # Examples
+    ///
+    /// Allocate a buffer for a single file descriptor:
+    /// ```
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::SendAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmRights(1))];
+    /// let mut cmsg_buffer = SendAncillaryBuffer::new(&mut space);
+    /// ```
+    ///
+    /// Allocate a buffer for credentials:
+    /// ```
+    /// # #[cfg(linux_kernel)]
+    /// # {
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::SendAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmCredentials(1))];
+    /// let mut cmsg_buffer = SendAncillaryBuffer::new(&mut space);
+    /// # }
+    /// ```
+    ///
+    /// Allocate a buffer for two file descriptors and credentials:
+    /// ```
+    /// # #[cfg(linux_kernel)]
+    /// # {
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::SendAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmRights(2), ScmCredentials(1))];
+    /// let mut cmsg_buffer = SendAncillaryBuffer::new(&mut space);
+    /// # }
+    /// ```
+    ///
+    /// [`send`]: crate::net::send
     #[inline]
     pub fn new(buffer: &'buf mut [u8]) -> Self {
         Self {
@@ -229,6 +332,10 @@ impl<'slice, 'fd> Extend<SendAncillaryMessage<'slice, 'fd>>
 }
 
 /// Buffer for receiving ancillary messages with [`recvmsg`].
+///
+/// Use the [`drain`] function to iterate over the received messages.
+///
+/// [`drain`]: RecvAncillaryBuffer::drain
 #[derive(Default)]
 pub struct RecvAncillaryBuffer<'buf> {
     /// Raw byte buffer for messages.
@@ -249,6 +356,44 @@ impl<'buf> From<&'buf mut [u8]> for RecvAncillaryBuffer<'buf> {
 
 impl<'buf> RecvAncillaryBuffer<'buf> {
     /// Create a new, empty `RecvAncillaryBuffer` from a raw byte buffer.
+    ///
+    /// The buffer size may be computed with [`cmsg_space`], or it may be
+    /// zero for an empty buffer, however in that case, consider `default()`
+    /// instead, or even using [`recv`] instead of `recvmsg`.
+    ///
+    /// # Examples
+    ///
+    /// Allocate a buffer for a single file descriptor:
+    /// ```
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::RecvAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmRights(1))];
+    /// let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut space);
+    /// ```
+    ///
+    /// Allocate a buffer for credentials:
+    /// ```
+    /// # #[cfg(linux_kernel)]
+    /// # {
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::RecvAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmCredentials(1))];
+    /// let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut space);
+    /// # }
+    /// ```
+    ///
+    /// Allocate a buffer for two file descriptors and credentials:
+    /// ```
+    /// # #[cfg(linux_kernel)]
+    /// # {
+    /// # use rustix::cmsg_space;
+    /// # use rustix::net::RecvAncillaryBuffer;
+    /// let mut space = [0; rustix::cmsg_space!(ScmRights(2), ScmCredentials(1))];
+    /// let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut space);
+    /// # }
+    /// ```
+    ///
+    /// [`recv`]: crate::net::recv
     #[inline]
     pub fn new(buffer: &'buf mut [u8]) -> Self {
         Self {
@@ -311,6 +456,12 @@ impl Drop for RecvAncillaryBuffer<'_> {
 /// boundary.
 #[inline]
 fn align_for_cmsghdr(buffer: &mut [u8]) -> &mut [u8] {
+    // If the buffer is empty, we won't be writing anything into it, so it
+    // doesn't need to be aligned.
+    if buffer.is_empty() {
+        return buffer;
+    }
+
     let align = align_of::<c::cmsghdr>();
     let addr = buffer.as_ptr() as usize;
     let adjusted = (addr + (align - 1)) & align.wrapping_neg();

diff --git a/tests/net/cmsg.rs b/tests/net/cmsg.rs
@@ -0,0 +1,34 @@
+#[test]
+fn test_empty_buffers() {
+    use rustix::fd::AsFd;
+    use rustix::net::{RecvAncillaryBuffer, SendAncillaryBuffer, SendAncillaryMessage};
+    use rustix::pipe::pipe;
+
+    let (_read_end, write_end) = pipe().unwrap();
+    let we = [write_end.as_fd()];
+
+    let mut cmsg_buffer = SendAncillaryBuffer::new(&mut []);
+    let msg = SendAncillaryMessage::ScmRights(&we);
+    assert!(!cmsg_buffer.push(msg));
+
+    let mut cmsg_buffer = SendAncillaryBuffer::default();
+    let msg = SendAncillaryMessage::ScmRights(&we);
+    assert!(!cmsg_buffer.push(msg));
+
+    let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut []);
+    assert!(cmsg_buffer.drain().next().is_none());
+
+    let mut cmsg_buffer = RecvAncillaryBuffer::default();
+    assert!(cmsg_buffer.drain().next().is_none());
+}
+
+#[test]
+fn test_buffer_sizes() {
+    use rustix::cmsg_space;
+
+    assert!(cmsg_space!(ScmRights(0)) > 0);
+    assert!(cmsg_space!(ScmRights(1)) >= cmsg_space!(ScmRights(0)));
+    assert!(cmsg_space!(ScmRights(2)) < cmsg_space!(ScmRights(1), ScmRights(1)));
+    assert!(cmsg_space!(ScmRights(1)) * 2 >= cmsg_space!(ScmRights(1), ScmRights(1)));
+    assert!(cmsg_space!(ScmRights(1), ScmRights(0)) >= cmsg_space!(ScmRights(1)));
+}
diff --git a/tests/net/main.rs b/tests/net/main.rs
@@ -6,6 +6,8 @@
 #![cfg_attr(core_c_str, feature(core_c_str))]
 
 mod addr;
+#[cfg(unix)]
+mod cmsg;
 mod connect_bind_send;
 #[cfg(feature = "event")]
 mod poll;