UnixAddr: replace path_len with sun_len

Within UnixAddr, replace the path_len variable (length of the sun_path
field) with sun_len (length of the whole structure).  This is more
similar to how other sockaddr types work, and it's the same way that the
BSDs use the sun_len field.

Also, don't require that sun_path be nul-terminated. The OS doesn't
require it.

Author: asomers

src/sys/socket/addr.rs

@@ -3,6 +3,7 @@ use crate::{Result, NixPath};
 use crate::errno::Errno;
 use memoffset::offset_of;
 use std::{fmt, mem, net, ptr, slice};
+use std::convert::TryInto;
 use std::ffi::OsStr;
 use std::hash::{Hash, Hasher};
 use std::path::Path;
@@ -575,9 +576,11 @@ impl fmt::Display for Ipv6Addr {
 /// A wrapper around `sockaddr_un`.
 #[derive(Clone, Copy, Debug)]
 pub struct UnixAddr {
-    // INVARIANT: sun & path_len are valid as defined by docs for from_raw_parts
+    // INVARIANT: sun & sun_len are valid as defined by docs for from_raw_parts
     sun: libc::sockaddr_un,
-    path_len: usize,
+    /// The length of the valid part of `sun`, including the sun_family field
+    /// but excluding any trailing nul.
+    sun_len: u8,
 }
 
 // linux man page unix(7) says there are 3 kinds of unix socket:
@@ -594,8 +597,10 @@ enum UnixAddrKind<'a> {
     Abstract(&'a [u8]),
 }
 impl<'a> UnixAddrKind<'a> {
-    /// Safety: sun & path_len must be valid
-    unsafe fn get(sun: &'a libc::sockaddr_un, path_len: usize) -> Self {
+    /// Safety: sun & sun_len must be valid
+    unsafe fn get(sun: &'a libc::sockaddr_un, sun_len: u8) -> Self {
+        assert!(sun_len as usize >= offset_of!(libc::sockaddr_un, sun_path));
+        let path_len = sun_len as usize - offset_of!(libc::sockaddr_un, sun_path);
         if path_len == 0 {
             return Self::Unnamed;
         }
@@ -605,7 +610,7 @@ impl<'a> UnixAddrKind<'a> {
                 slice::from_raw_parts(sun.sun_path.as_ptr().add(1) as *const u8, path_len - 1);
             return Self::Abstract(name);
         }
-        let pathname = slice::from_raw_parts(sun.sun_path.as_ptr() as *const u8, path_len - 1);
+        let pathname = slice::from_raw_parts(sun.sun_path.as_ptr() as *const u8, path_len);
         Self::Pathname(Path::new(OsStr::from_bytes(pathname)))
     }
 }
@@ -626,18 +631,32 @@ impl UnixAddr {
                     return Err(Errno::ENAMETOOLONG);
                 }
 
+                let sun_len = (bytes.len() +
+                        offset_of!(libc::sockaddr_un, sun_path)).try_into()
+                        .unwrap();
+
+                #[cfg(any(target_os = "dragonfly",
+                          target_os = "freebsd",
+                          target_os = "ios",
+                          target_os = "macos",
+                          target_os = "netbsd",
+                          target_os = "illumos",
+                          target_os = "openbsd"))]
+                {
+                    ret.sun_len = sun_len;
+                }
                 ptr::copy_nonoverlapping(bytes.as_ptr(),
                                          ret.sun_path.as_mut_ptr() as *mut u8,
                                          bytes.len());
 
-                Ok(UnixAddr::from_raw_parts(ret, bytes.len() + 1))
+                Ok(UnixAddr::from_raw_parts(ret, sun_len))
             }
         })?
     }
 
     /// Create a new `sockaddr_un` representing an address in the "abstract namespace".
     ///
-    /// The leading null byte for the abstract namespace is automatically added;
+    /// The leading nul byte for the abstract namespace is automatically added;
     /// thus the input `path` is expected to be the bare name, not null-prefixed.
     /// This is a Linux-specific extension, primarily used to allow chrooted
     /// processes to communicate with processes having a different filesystem view.
@@ -653,39 +672,50 @@ impl UnixAddr {
             if path.len() >= ret.sun_path.len() {
                 return Err(Errno::ENAMETOOLONG);
             }
+            let sun_len = (path.len() +
+                1 +
+                offset_of!(libc::sockaddr_un, sun_path)).try_into()
+                .unwrap();
 
             // Abstract addresses are represented by sun_path[0] ==
             // b'\0', so copy starting one byte in.
             ptr::copy_nonoverlapping(path.as_ptr(),
                                      ret.sun_path.as_mut_ptr().offset(1) as *mut u8,
                                      path.len());
 
-            Ok(UnixAddr::from_raw_parts(ret, path.len() + 1))
+            Ok(UnixAddr::from_raw_parts(ret, sun_len))
         }
     }
 
-    /// Create a UnixAddr from a raw `sockaddr_un` struct and a size. `path_len` is the "addrlen"
-    /// of this address, but minus `offsetof(struct sockaddr_un, sun_path)`. Basically the length
-    /// of the data in `sun_path`.
+    /// Create a UnixAddr from a raw `sockaddr_un` struct and a size. `sun_len`
+    /// is the size of the valid portion of the struct, excluding any trailing
+    /// NUL.
     ///
     /// # Safety
-    /// This pair of sockaddr_un & path_len must be a valid unix addr, which means:
-    /// - path_len <= sockaddr_un.sun_path.len()
-    /// - if this is a unix addr with a pathname, sun.sun_path is a nul-terminated fs path and
-    ///   sun.sun_path[path_len - 1] == 0 || sun.sun_path[path_len] == 0
-    pub(crate) unsafe fn from_raw_parts(sun: libc::sockaddr_un, mut path_len: usize) -> UnixAddr {
-        if let UnixAddrKind::Pathname(_) = UnixAddrKind::get(&sun, path_len) {
-            if sun.sun_path[path_len - 1] != 0 {
-                assert_eq!(sun.sun_path[path_len], 0);
-                path_len += 1
-            }
+    /// This pair of sockaddr_un & sun_len must be a valid unix addr, which
+    /// means:
+    /// - sun_len >= offset_of(sockaddr_un, sun_path)
+    /// - sun_len <= sockaddr_un.sun_path.len() - offset_of(sockaddr_un, sun_path)
+    /// - if this is a unix addr with a pathname, sun.sun_path is a
+    ///   fs path, not necessarily nul-terminated.
+    pub(crate) unsafe fn from_raw_parts(sun: libc::sockaddr_un, sun_len: u8) -> UnixAddr {
+        #[cfg(any(target_os = "dragonfly",
+                  target_os = "freebsd",
+                  target_os = "ios",
+                  target_os = "macos",
+                  target_os = "netbsd",
+                  target_os = "illumos",
+                  target_os = "openbsd"))]
+        {
+            assert_eq!(sun_len, sun.sun_len);
         }
-        UnixAddr { sun, path_len }
+
+        UnixAddr { sun, sun_len }
     }
 
     fn kind(&self) -> UnixAddrKind<'_> {
         // SAFETY: our sockaddr is always valid because of the invariant on the struct
-        unsafe { UnixAddrKind::get(&self.sun, self.path_len) }
+        unsafe { UnixAddrKind::get(&self.sun, self.sun_len) }
     }
 
     /// If this address represents a filesystem path, return that path.
@@ -712,7 +742,7 @@ impl UnixAddr {
     /// Returns the addrlen of this socket - `offsetof(struct sockaddr_un, sun_path)`
     #[inline]
     pub fn path_len(&self) -> usize {
-        self.path_len
+        self.sun_len as usize - offset_of!(libc::sockaddr_un, sun_path)
     }
     /// Returns a pointer to the raw `sockaddr_un` struct
     #[inline]
@@ -957,12 +987,12 @@ impl SockAddr {
                 },
                 mem::size_of_val(addr) as libc::socklen_t
             ),
-            SockAddr::Unix(UnixAddr { ref sun, path_len }) => (
+            SockAddr::Unix(UnixAddr { ref sun, sun_len }) => (
                 // This cast is always allowed in C
                 unsafe {
                     &*(sun as *const libc::sockaddr_un as *const libc::sockaddr)
                 },
-                (path_len + offset_of!(libc::sockaddr_un, sun_path)) as libc::socklen_t
+                sun_len as libc::socklen_t
             ),
             #[cfg(any(target_os = "android", target_os = "linux"))]
             SockAddr::Netlink(NetlinkAddr(ref sa)) => (

src/sys/socket/mod.rs

@@ -5,7 +5,7 @@ use cfg_if::cfg_if;
 use crate::{Result, errno::Errno};
 use libc::{self, c_void, c_int, iovec, socklen_t, size_t,
         CMSG_FIRSTHDR, CMSG_NXTHDR, CMSG_DATA, CMSG_LEN};
-use memoffset::offset_of;
+use std::convert::TryInto;
 use std::{mem, ptr, slice};
 use std::os::unix::io::RawFd;
 #[cfg(target_os = "linux")]
@@ -1934,10 +1934,10 @@ pub fn sockaddr_storage_to_addr(
             Ok(SockAddr::Inet(InetAddr::V6(sin6)))
         }
         libc::AF_UNIX => {
-            let pathlen = len - offset_of!(sockaddr_un, sun_path);
             unsafe {
                 let sun = *(addr as *const _ as *const sockaddr_un);
-                Ok(SockAddr::Unix(UnixAddr::from_raw_parts(sun, pathlen)))
+                let sun_len = len.try_into().unwrap();
+                Ok(SockAddr::Unix(UnixAddr::from_raw_parts(sun, sun_len)))
             }
         }
         #[cfg(any(target_os = "android", target_os = "linux"))]