Re: kern/46463: netbsd-6: panic and filesystem corruption running tmux

[Martin Husemann <martin%duskware.de@localhost>]

The following reply was made to PR kern/46463; it has been noted by GNATS.

From: Martin Husemann <martin%duskware.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/46463: netbsd-6:  panic and filesystem corruption running tmux
Date: Thu, 31 May 2012 12:08:22 +0200

 So it turns out (subject to interpretation in detail and to be discussed
 on tech-kern) that we see a side effect of two bugs here.
 
 First, at the application level, in tmux, there is something wrong.
 The client attaching to a already established session does:
  (1) call kqueue() to be able to receive kevents
  (2) connect via a local socket to the already running tmux instance
  (3) pass stdin/stdout/stderr vis sendmsg with SCM_RIGHTS to the session mux
 
 Now in the case that triggered this PR, tmux was invoked with stdin closed.
 Not suprising, kqueue() in (1) returns 0. Later in (3), this file descriptor
 is passed to the mux (which expects to receive stdin).
 
 So, adding something like this:
 
   while ((fd = open("/dev/null", O_RDWR)) < 3)
      ;
   close(fd);
 
 early in tmux would make sure this does not happen, replacing the missing
 descriptors with /dev/null.
 
 Now for the kernel part. A kqueue was intended to be a local-proc-only thing.
 There is no sense passing it from one process to another. That is why they
 are not inherited during fork() for example. However, and I think this is a
 bug, they can be passed via SCM_RIGHTS messages.
 
 I can reproduce the crash with this test program:
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <err.h>
 #include <sys/event.h>
 #include <sys/time.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
 
 static int
 child_proc(int fd)
 {
        int kq, storage = -1;
        struct cmsghdr *msg;
        struct iovec iov;
        struct msghdr m;
 
        msg = malloc(CMSG_SPACE(sizeof(int)));
        iov.iov_base = &storage;
        iov.iov_len = sizeof(int);
        m.msg_iov = &iov;
        m.msg_iovlen = 1;
 
        if (recvmsg(fd, &m, 0) == -1)
                err(1, "child: could not recvmsg");
 
        kq = *(int *)CMSG_DATA(msg);
        printf("child (pid %d): received kq fd %d\n", getpid(), kq);
 
        return 0;
 }
 
 int main(void)
 {
        pid_t child;
        int s[2], storage, status, kq;
        struct cmsghdr *msg;
        struct iovec iov;
        struct msghdr m;
        struct kevent ev;
 
        kq = kqueue();
        if (kq < 0)
                err(1, "could not create kqueue");
 
        if (socketpair(AF_LOCAL, SOCK_STREAM, 0, s) == -1)
                err(1, "could not create a socket pair");
 
 
        child = fork();
        if (child == 0) {
                close(s[0]);
                return child_proc(s[1]);
        }
        close(s[1]);
 
        msg = malloc(CMSG_SPACE(sizeof(int)));
        iov.iov_base = &storage;
        iov.iov_len = sizeof(int);
 
        msg->cmsg_level = SOL_SOCKET;
        msg->cmsg_type = SCM_RIGHTS;
        msg->cmsg_len = CMSG_LEN(sizeof(int));
 
        m.msg_iov = &iov;
        m.msg_iovlen = 1;
        m.msg_name = NULL;
        m.msg_namelen = 0;
        m.msg_control = msg;
        m.msg_controllen = CMSG_SPACE(sizeof(int));
        *(int *)CMSG_DATA(msg) = kq;
 
        EV_SET(&ev, 1, EVFILT_TIMER, EV_ADD|EV_ENABLE, 0, 1, 0);
        if (kevent(kq, &ev, 1, NULL, 0, NULL) == -1)
                err(1, "could not add timer event");
 
        printf("parent (pid %d): sending kq fd %d\n", getpid(), kq);
        if (sendmsg(s[0], &m, 0) == -1)
                err(1, "could not sendmsg");
 
        close(kq);
 
        waitpid(child, &status, 0);
 
        return 0;
 }
 
 
 IMHO the correct fix would be to filter kqueue descriptors out of SCM_RIGHTS
 at the sending side and document this (in unp_internalize break out and
 return EPERM if there is a file_t with f_type == DTYPE_KQUEUE).
 
 Martin