- djm@cvs.openbsd.org 2010/09/20 07:19:27

     [mux.c]
     "atomically" create the listening mux socket by binding it on a temorary
     name and then linking it into position after listen() has succeeded.
     this allows the mux clients to determine that the server socket is
     either ready or stale without races. stale server sockets are now
     automatically removed
     ok deraadt

1 files changed, 46 insertions, 4 deletions

diff --git a/mux.c b/mux.c
index 5c3857ee8..c010b614e 100644
--- a/mux.c
+++ b/mux.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: mux.c,v 1.21 2010/06/25 23:15:36 djm Exp $ */
+/* $OpenBSD: mux.c,v 1.22 2010/09/20 07:19:27 djm Exp $ */
 /*
  * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
  *
@@ -1026,6 +1026,9 @@ muxserver_listen(void)
         struct sockaddr_un addr;
         socklen_t sun_len;
         mode_t old_umask;
+        char *orig_control_path = options.control_path;
+        char rbuf[16+1];
+        u_int i, r;
 
         if (options.control_path == NULL ||
             options.control_master == SSHCTL_MASTER_NO)
@@ -1033,6 +1036,23 @@ muxserver_listen(void)
 
         debug("setting up multiplex master socket");
 
+        /*
+         * Use a temporary path before listen so we can pseudo-atomically
+         * establish the listening socket in its final location to avoid
+         * other processes racing in between bind() and listen() and hitting
+         * an unready socket.
+         */
+        for (i = 0; i < sizeof(rbuf) - 1; i++) {
+                r = arc4random_uniform(26+26+10);
+                rbuf[i] = (r < 26) ? 'a' + r :
+                    (r < 26*2) ? 'A' + r - 26 :
+                    '0' + r - 26 - 26;
+        }
+        rbuf[sizeof(rbuf) - 1] = '\0';
+        options.control_path = NULL;
+        xasprintf(&options.control_path, "%s.%s", orig_control_path, rbuf);
+        debug3("%s: temporary control path %s", __func__, options.control_path);
+
         memset(&addr, '\0', sizeof(addr));
         addr.sun_family = AF_UNIX;
         sun_len = offsetof(struct sockaddr_un, sun_path) +
@@ -1051,6 +1071,7 @@ muxserver_listen(void)
                 if (errno == EINVAL || errno == EADDRINUSE) {
                         error("ControlSocket %s already exists, "
                             "disabling multiplexing", options.control_path);
+ disable_mux_master:
                         close(muxserver_sock);
                         muxserver_sock = -1;
                         xfree(options.control_path);
@@ -1065,12 +1086,29 @@ muxserver_listen(void)
         if (listen(muxserver_sock, 64) == -1)
                 fatal("%s listen(): %s", __func__, strerror(errno));
 
+        /* Now atomically "move" the mux socket into position */
+        if (link(options.control_path, orig_control_path) != 0) {
+                if (errno != EEXIST) {
+                        fatal("%s: link mux listener %s => %s: %s", __func__, 
+                            options.control_path, orig_control_path,
+                            strerror(errno));
+                }
+                error("ControlSocket %s already exists, disabling multiplexing",
+                    orig_control_path);
+                xfree(orig_control_path);
+                unlink(options.control_path);
+                goto disable_mux_master;
+        }
+        unlink(options.control_path);
+        xfree(options.control_path);
+        options.control_path = orig_control_path;
+
         set_nonblock(muxserver_sock);
 
         mux_listener_channel = channel_new("mux listener",
             SSH_CHANNEL_MUX_LISTENER, muxserver_sock, muxserver_sock, -1,
             CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT,
-            0, addr.sun_path, 1);
+            0, options.control_path, 1);
         mux_listener_channel->mux_rcb = mux_master_read_cb;
         debug3("%s: mux listener channel %d fd %d", __func__,
             mux_listener_channel->self, mux_listener_channel->sock);
@@ -1823,9 +1861,13 @@ muxclient(const char *path)
                         fatal("Control socket connect(%.100s): %s", path,
                             strerror(errno));
                 }
-                if (errno == ENOENT)
+                if (errno == ECONNREFUSED &&
+                    options.control_master != SSHCTL_MASTER_NO) {
+                        debug("Stale control socket %.100s, unlinking", path);
+                        unlink(path);
+                } else if (errno == ENOENT) {
                         debug("Control socket \"%.100s\" does not exist", path);
-                else {
+                } else {
                         error("Control socket connect(%.100s): %s", path,
                             strerror(errno));
                 }