"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/usr.bin/ssh/mux.c b/usr.bin/ssh/mux.c
index 92de5636dab..6418383de29 100644
--- a/usr.bin/ssh/mux.c
+++ b/usr.bin/ssh/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>
  *
@@ -1008,6 +1008,9 @@ muxserver_listen(void)
 {
 	struct sockaddr_un addr;
 	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)
@@ -1015,6 +1018,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;
 	addr.sun_len = offsetof(struct sockaddr_un, sun_path) +
@@ -1033,6 +1053,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);
@@ -1047,12 +1068,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);
@@ -1798,9 +1836,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));
 		}