BUG/MEDIUM: unix: never unlink a unix socket from the file system
James Brown reported some cases where a race condition happens between
the old and the new processes resulting in the leaving process removing
a newly bound unix socket. Jeff gave all the details he observed here :
https://www.mail-archive.com/haproxy@formilux.org/msg25001.html
The unix socket removal was an attempt at an optimal cleanup, which
almost never works anyway since the process is supposed to be chrooted.
And in the rare cases where it works it occasionally creates trouble.
There was already a workaround in place to avoid removing this socket
when it's been inherited from a parent's file descriptor.
So let's finally kill this useless stuff now to definitely get rid of
this persistent problem.
This fix should be backported to all stable releases.
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index d68267e..5aa45a7 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -107,44 +107,6 @@
}
-/* Tries to destroy the UNIX stream socket <path>. The socket must not be used
- * anymore. It practises best effort, and no error is returned.
- */
-static void destroy_uxst_socket(const char *path)
-{
- struct sockaddr_un addr;
- int sock, ret;
-
- /* if the path was cleared, we do nothing */
- if (!*path)
- return;
-
- /* We might have been chrooted, so we may not be able to access the
- * socket. In order to avoid bothering the other end, we connect with a
- * wrong protocol, namely SOCK_DGRAM. The return code from connect()
- * is enough to know if the socket is still live or not. If it's live
- * in mode SOCK_STREAM, we get EPROTOTYPE or anything else but not
- * ECONNREFUSED. In this case, we do not touch it because it's used
- * by some other process.
- */
- sock = socket(PF_UNIX, SOCK_DGRAM, 0);
- if (sock < 0)
- return;
-
- addr.sun_family = AF_UNIX;
- strncpy(addr.sun_path, path, sizeof(addr.sun_path));
- addr.sun_path[sizeof(addr.sun_path) - 1] = 0;
- ret = connect(sock, (struct sockaddr *)&addr, sizeof(addr));
- if (ret < 0 && errno == ECONNREFUSED) {
- /* Connect failed: the socket still exists but is not used
- * anymore. Let's remove this socket now.
- */
- unlink(path);
- }
- close(sock);
-}
-
-
/********************************
* 2) listener-oriented functions
********************************/
@@ -357,15 +319,9 @@
goto err_rename;
}
- /* Cleanup: If we're bound to an fd inherited from the parent, we
- * want to ensure that destroy_uxst_socket() will never remove the
- * path, and for this we simply clear the path to the socket, which
- * under Linux corresponds to an abstract socket.
- */
+ /* Cleanup: only unlink if we didn't inherit the fd from the parent */
if (!ext && path[0])
unlink(backname);
- else
- ((struct sockaddr_un *)&listener->addr)->sun_path[0] = 0;
/* the socket is now listening */
listener->fd = fd;
@@ -405,7 +361,6 @@
{
if (listener->state > LI_ASSIGNED) {
unbind_listener(listener);
- destroy_uxst_socket(((struct sockaddr_un *)&listener->addr)->sun_path);
}
return ERR_NONE;
}