epoll: Fix leaked listen bug (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 28 Sep 2017 01:40:03 +0000 (21:40 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 29 Sep 2017 14:00:03 +0000 (10:00 -0400)
During the close callbacks for a data FD, the rock CB was called first.
That would close the listen FD (if it was open).  Then we'd call the
epoll CB, which would do an __epoll_ctl_del().  That would open the
listen FD.  However, at that point, the callback that would make sure
the listen FD was closed wouldn't run (it already ran).

The end result for dropbear was that listen FDs were getting leaked.
The DB parent would have the child's listen line open (e.g.
/net/tcp/1/listen, while the child had 1/data).  The child also had the
parent's 0/listen.  The latter had the effect of us never being able to
restart sshd while a client was connected, since someone would always be
listening on port 22.

Rebuild glibc and dropbear.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/plan9_sockets.c
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/sys/plan9_helpers.h
user/iplib/epoll.c

index c6c8d6a..f50fd32 100644 (file)
@@ -318,15 +318,13 @@ int _sock_get_opts(int type)
 /* Opens the FD for "listen", and attaches it to the Rock.  When the dfd (and
  * thus the Rock) closes, we'll close the listen file too.  Returns the FD on
  * success, -1 on error.  This is racy, like a lot of other Rock stuff. */
-static int _rock_get_listen_fd(Rock *r)
+static int _rock_open_listen_fd(Rock *r)
 {
        char listen_file[Ctlsize + 3];
        char *p;
        int ret;
        int open_flags;
 
-       if (r->has_listen_fd)
-               return r->listen_fd;
        if (r->ctl == NULL || r->ctl[0] != '/') {
                fprintf(stderr, "r->ctl corrupt: '%s' (domain = %d)\n",
                        (r->ctl == NULL ? "NULL" : r->ctl), r->domain);
@@ -358,13 +356,17 @@ static int _rock_get_listen_fd(Rock *r)
 
 /* Used by user/iplib (e.g. epoll).  Returns an FD for the listen file, opened
  * O_PATH, for this conversation.  Returns -1 on error. */
-int _sock_lookup_listen_fd(int sock_fd)
+int _sock_lookup_listen_fd(int sock_fd, bool can_open)
 {
        Rock *r = _sock_findrock(sock_fd, 0);
 
        if (!r || r->domain == PF_UNIX)
                return -1;
-       return _rock_get_listen_fd(r);
+       if (r->has_listen_fd)
+               return r->listen_fd;
+       if (!can_open)
+               return -1;
+       return _rock_open_listen_fd(r);
 }
 
 /* Given an FD, opens the FD with the name 'sibling' in the same directory.
index 2747bd9..daeb1e5 100644 (file)
@@ -74,7 +74,7 @@ extern void _sock_ingetaddr(Rock *, struct sockaddr_in *, socklen_t *,
                                                        const char *);
 extern int _sock_strip_opts(int type);
 extern int _sock_get_opts(int type);
-extern int _sock_lookup_listen_fd(int sock_fd);
+extern int _sock_lookup_listen_fd(int sock_fd, bool can_open);
 extern int _sock_open_ctlfd(Rock *r);
 
 int get_sibling_fd(int fd, const char *sibling);
index 9339a58..a25fc79 100644 (file)
@@ -386,7 +386,7 @@ static int __epoll_ctl_add(struct epoll_ctlr *ep, int fd,
         * As far as tracking the FD goes for epoll_wait() reporting, if the app
         * wants to track the FD they think we are using, then they already passed
         * that in event->data. */
-       sock_listen_fd = _sock_lookup_listen_fd(fd);
+       sock_listen_fd = _sock_lookup_listen_fd(fd, TRUE);
        if (sock_listen_fd >= 0) {
                listen_event.events = EPOLLET | EPOLLIN | EPOLLHUP;
                listen_event.data = event->data;
@@ -434,8 +434,13 @@ static int __epoll_ctl_del(struct epoll_ctlr *ep, int fd,
        int ret, sock_listen_fd;
 
        /* If we were dealing with a socket shim FD, we tapped both the listen and
-        * the data file and need to untap both of them. */
-       sock_listen_fd = _sock_lookup_listen_fd(fd);
+        * the data file and need to untap both of them.
+        *
+        * We could be called from a close_cb, and we already closed the listen FD.
+        * In that case, we don't want to try and open it.  If the listen FD isn't
+        * open, then we know it isn't in an epoll set.  We also know the data FD
+        * isn't epolled either, since we always epoll both FDs for rocks. */
+       sock_listen_fd = _sock_lookup_listen_fd(fd, FALSE);
        if (sock_listen_fd >= 0) {
                /* It's possible to fail here.  Even though we tapped it already, if the
                 * deletion was triggered from close callbacks, it's possible for the