Panic in recvmsg msghdr decoding / read_sockaddr_os() with abstract unix socket address

[psychon]

Hi,

@notgull added support for recvmsg in #505 and then wanted to use that code in x11rb: psychon/x11rb#815 However, there rustix panics in one of the existing examples.

I tried to port this into a self-contained example. This requires something (e.g. an X11 server) to listen on the abstract unix socket at /tmp/.X11-unix/X0.

Cargo.toml and main.rs

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
rustix = { version = "0.37.19", default-features = false, features = ["std", "fs", "net"] }

use rustix::fd::OwnedFd;
use rustix::net::{
    connect_unix, recvmsg, sendmsg_noaddr, socket_with, AddressFamily, Protocol,
    RecvAncillaryBuffer, RecvFlags, SendFlags, SocketAddrUnix, SocketFlags, SocketType,
};

fn send_x11_setup_request(socket: &OwnedFd) -> Result<(), rustix::io::Errno> {
    let data = [b'l', 0, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0];
    let result = sendmsg_noaddr(
        socket,
        &[std::io::IoSlice::new(&data)],
        &mut Default::default(),
        SendFlags::empty(),
    )?;
    assert_eq!(data.len(), result);
    Ok(())
}

fn do_receive(socket: &OwnedFd) -> Result<(), rustix::io::Errno> {
    let mut buffer = [0; 100];
    let mut cmsg = vec![0u8; rustix::cmsg_space!(ScmRights(16))];
    let mut iov = [std::io::IoSliceMut::new(&mut buffer)];
    let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut cmsg);
    recvmsg(socket, &mut iov, &mut cmsg_buffer, RecvFlags::CMSG_CLOEXEC)?;
    Ok(())
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let socket = socket_with(
        AddressFamily::UNIX,
        SocketType::STREAM,
        SocketFlags::CLOEXEC,
        Protocol::default(),
    )?;
    connect_unix(
        &socket,
        &SocketAddrUnix::new_abstract_name(b"/tmp/.X11-unix/X0")?,
    )?;
    send_x11_setup_request(&socket)?;
    do_receive(&socket)?;
    Ok(())
}

Running the above program locally produces for me:

$ RUST_BACKTRACE=1 cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/foo`
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `48`,
 right: `0`', /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/read_sockaddr.rs:158:17
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:142:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:181:5
   4: rustix::backend::net::read_sockaddr::read_sockaddr_os
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/read_sockaddr.rs:158:17
   5: rustix::backend::net::read_sockaddr::maybe_read_sockaddr_os
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/read_sockaddr.rs:120:14
   6: rustix::backend::net::syscalls::recvmsg::{{closure}}::{{closure}}
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/syscalls.rs:284:26
   7: core::result::Result<T,E>::map
             at /usr/src/rustc-1.63.0/library/core/src/result.rs:769:25
   8: rustix::backend::net::syscalls::recvmsg::{{closure}}
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/syscalls.rs:281:9
   9: rustix::backend::net::msghdr::with_recv_msghdr
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/msghdr.rs:49:15
  10: rustix::backend::net::syscalls::recvmsg
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/syscalls.rs:257:5
  11: rustix::net::send_recv::msg::recvmsg
             at /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/net/send_recv/msg.rs:568:5
  12: foo::do_receive
             at ./src/main.rs:24:5
  13: foo::main
             at ./src/main.rs:40:5
  14: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Running this under strace says that the socket name is correct:

$ strace -e recvmsg ./target/debug/foo
recvmsg(3, {msg_name={sa_family=AF_UNIX, sun_path=@"/tmp/.X11-unix/X0"}, msg_namelen=128 => 20, msg_iov=[{iov_base="\1\0\v\0\0\0C\16", iov_len=100}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 8
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `48`,
 right: `0`', /home/psychon/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.37.19/src/backend/linux_raw/net/read_sockaddr.rs:158:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+++ exited with 101 +++

The number 48 is ASCII for 0, so this seems to be some kind of off-by-one?

I am not sure whether the kernel even zero-terminates abstract sockets. man 7 unix says:

an abstract socket address is distinguished (from a pathname socket) by the fact that sun_path[0] is a null byte ('\0'). The socket's address in this namespace is given by the additional bytes in sun_path that are covered by the specified length of the address structure. (Null bytes in the name have no special significance.) The name has no connection with filesystem pathnames. When the address of an abstract socket is returned, the returned addrlen is greater than sizeof(sa_family_t) (i.e., greater than 2), and the name of the socket is contained in the first (addrlen - sizeof(sa_family_t)) bytes of sun_path.

[psychon]

I had another idea: gdb with a breakpoint on rust_panic:

(gdb) frame 8
#8  0x00005555555692ad in rustix::backend::net::read_sockaddr::read_sockaddr_os (storage=0x7fffffffdfe0, len=20) at src/backend/linux_raw/net/read_sockaddr.rs:158
158	                assert_eq!(
(gdb) list
153	            assert!(len >= offsetof_sun_path);
154	            if len == offsetof_sun_path {
155	                SocketAddrAny::Unix(SocketAddrUnix::new(&[][..]).unwrap())
156	            } else {
157	                let decode = *storage.cast::<c::sockaddr_un>();
158	                assert_eq!(
159	                    decode.sun_path[len - 1 - offsetof_sun_path],
160	                    b'\0' as c::c_char
161	                );
162	                SocketAddrAny::Unix(
(gdb) print decode
$3 = linux_raw_sys::general::sockaddr_un {sun_family: 1, sun_path: [0, 47, 116, 109, 112, 47, 46, 88, 49, 49, 45, 117, 110, 105, 120, 47, 88, 48, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 
    0, 0, 0, 0, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 
    160, 235, 91, 85, 3, 0, 0, 0, 3, 0, 0, 0, 3, 0, 0, 0, 219, 238, 85, 85, 85, 85]}
(gdb) print *storage
$2 = linux_raw_sys::general::sockaddr {__storage: linux_raw_sys::general::__kernel_sockaddr_storage {__bindgen_anon_1: linux_raw_sys::general::__kernel_sockaddr_storage__bindgen_ty_1 {__bindgen_anon_1: linux_raw_sys::general::__kernel_sockaddr_storage__bindgen_ty_1__bindgen_ty_1 {ss_family: 1, __data: [0, 47, 116, 109, 112, 47, 46, 88, 49, 49, 45, 117, 110, 105, 120, 47, 88, 
          48, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 160, 
          235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 3, 0, 0, 0, 3, 0, 0, 0, 3, 0, 0, 0, 219, 238, 85, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 3, 
          0, 0, 0]}, __align: 0x2f706d742f000001}}}
(gdb) print len - 1 - offsetof_sun_path
$7 = 17
(gdb) print decode.sun_path[17]
$8 = 48
(gdb) print decode.sun_path[18]
$9 = 0

Decoding this via the Rust playground confirms that 1..18 is the correct slice to get the abstract socket path out:

fn main() {
    let sun_path = [0, 47, 116, 109, 112, 47, 46, 88, 49, 49, 45, 117, 110, 105, 120, 47, 88, 48, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1, 0, 
    0, 0, 0, 0, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 160, 235, 91, 85, 85, 85, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 
    160, 235, 91, 85, 3, 0, 0, 0, 3, 0, 0, 0, 3, 0, 0, 0, 219, 238, 85, 85, 85, 85];
    let relevant_part = &sun_path[1..18];
    println!("{:?}", std::str::from_utf8(relevant_part));
}

Ok("/tmp/.X11-unix/X0")

[psychon]

Ah, I re-read the man page.

Unix sockets bound to a path contain a null terminated string and the length of the sockaddr includes the zero byte:

its length is

      offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1

and sun_path contains the null-terminated pathname.

Abstract sockets instead do not include the zero byte:

an abstract socket address is distinguished (from a pathname socket) by the fact that sun_path[0] is a null byte ('\0').
[...]
and the name of the socket is contained in the first (addrlen - sizeof(sa_family_t)) bytes of sun_path.