Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sending multiple ControlMessage in a single Unix Socket packet fails #756

Closed
roblabla opened this issue Sep 1, 2017 · 1 comment · Fixed by #918
Closed

Sending multiple ControlMessage in a single Unix Socket packet fails #756

roblabla opened this issue Sep 1, 2017 · 1 comment · Fixed by #918
Labels

Comments

@roblabla
Copy link
Contributor

roblabla commented Sep 1, 2017

This is a continuation of #464 and #473. For some reason, the attached test still fails.

IIRC, there's a problem in the way we encode multiple Control Messages which causes the kernel to only consider the first Control Message sent. The specifics of where the bug is, I have no idea.

The failing test
#[test]
fn test_scm_rights_multiple_cmsg() {
    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::uio::IoVec;

    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; 1], CmsgSpace<[RawFd; 1]>)>::new();
        let (data, fds) = match recvmsg(receive.as_raw_fd(), &iovec, Some(&mut space), MsgFlags::empty()) {
            Ok(msg) => {
                let mut iter = msg.cmsgs();
                let fdone;
                let fdtwo;
                if let Some(ControlMessage::ScmRights(fds)) = iter.next() {
                    fdone = fds[0];
                } else {
                    panic!();
                }
                if let Some(ControlMessage::ScmRights(fds)) = iter.next() {
                    fdtwo = fds[0];
                } else {
                    panic!();
                }
                (iovec[0].as_slice(), [fdone, fdtwo])
            },
            Err(_) => {
                panic!();
            }
        };
        assert_eq!(data, [1u8, 2u8, 3u8, 4u8, 5u8, 6u8, 7u8, 8u8]);
        assert_eq!(fds.len(), 2);
    });

    let slice = [1u8, 2u8, 3u8, 4u8, 5u8, 6u8, 7u8, 8u8];
    let iov = [IoVec::from_slice(&slice)];
    let arrone = [0];
    let arrtwo = [1]; // pass stdin and stdout
    let cmsg = [ControlMessage::ScmRights(&arrone), ControlMessage::ScmRights(&arrtwo)];
    sendmsg(send.as_raw_fd(), &iov, &cmsg, MsgFlags::empty(), None).unwrap();
    thread.join().unwrap();
}
@jonas-schievink
Copy link
Contributor

jonas-schievink commented Jul 1, 2018

I have found out (via experiment) that Linux will merge two control messages of type SCM_RIGHTS that contain one file descriptor each into a single one containing both file descriptors, so this test failing doesn't mean that it's not working, it's just not testing what it should be testing. You can see this in action by running the test above with strace (but I've also verified this with a C++ program):

Check out what the test actually sends over the socket:

strace -f -e 'sendmsg' target/debug/deps/test-e0330ca09d8faf9f --nocapture test_scm_rights_multiple_cmsg

The result should contain exactly one sendmsg occurrence:

[pid 29717] sendmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\1\2\3\4\5\6\7\10", iov_len=8}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[0]}, {cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[1]}], msg_controllen=48, msg_flags=0}, 0) = 8

As you can see, it contains two separate messages, each with only one fd in them (cmsg_data=[0] and cmsg_data=[1]). Now let's check out what's actually received:

strace -f -e 'recvmsg' target/debug/deps/test-e0330ca09d8faf9f --nocapture test_scm_rights_multiple_cmsg

Again, the output should contain exactly one recvmsg:

[pid 29650] recvmsg(4, {msg_name=0x7fa0651fe340, msg_namelen=128->0, msg_iov=[{iov_base="\1\2\3\4\5\6\7\10", iov_len=8}], msg_iovlen=1, msg_control=[{cmsg_len=24, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, cmsg_data=[5, 6]}], msg_controllen=24, msg_flags=0}, 0) = 8

This one contains just a single message, but with two fds: cmsg_data=[5, 6]

(this is on current master, but reproduces with #874 applied)
EDIT: My mistake, this only happens after #874 is applied - current master gives EINVAL

It's definitely a good idea to check that stuffing N file descriptors into the ancillary data results in N fds coming out on the other end, the test just need to be more permissive and perhaps sum the fd counts in all control messages to be safe.

Additionally, a test should be written that sends two different control messages over the socket so they can't possibly be merged by the kernel. This would properly test decoding of multiple messages.

bors bot added a commit that referenced this issue Jul 5, 2018
918: Fix passing multiple file descriptors / control messages via sendmsg r=asomers a=jonas-schievink

Fixes #464
Closes #874 because it's incorporated here
Closes #756 because it adds the test from that issue (with fixes)

Co-authored-by: alecmocatta <alec@mocatta.net>
bors bot added a commit that referenced this issue Jul 5, 2018
918: Fix passing multiple file descriptors / control messages via sendmsg r=asomers a=jonas-schievink

Fixes #464
Closes #874 because it's incorporated here
Closes #756 because it adds the test from that issue (with fixes)

Co-authored-by: alecmocatta <alec@mocatta.net>
@bors bors bot closed this as completed in #918 Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants