Hello,
I think I may have found some problems regarding abstract namespace sockets in
HAProxy (1.5.1).
Test config:
==============================
defaults
mode tcp
listen test1
bind unix@/tmp/test1.sock
server test1 127.0.0.1:22
listen test2
bind abns@test2
server test2 unix@/tmp/test1.sock
listen test3
bind abns@test3
server test3 abns@test2
==============================
And now the problems:
1)
When bind() fails (function uxst_bind_listener()), the fail path doesn't
consider the abstract namespace and tries to unlink paths held in
uninitiliazed memory (tempname and backname). See the strace excerpt;
the strings still hold the path from test1.
===============================================================================================
23722 bind(5, {sa_family=AF_FILE, path=@"test2"}, 110) = -1 EADDRINUSE (Address
already in use)
23722 unlink("/tmp/test1.sock.23722.tmp") = -1 ENOENT (No such file or
directory)
23722 close(5) = 0
23722 unlink("/tmp/test1.sock.23722.bak") = -1 ENOENT (No such file or
directory)
===============================================================================================
The first attached patch (haproxy_do_not_unlink_abns.diff) seems to fix it.
BTW, if abns@ bind() fails during a "-sf" reload, then /tmp/test1.sock of the
old HAProxy instance is unlinked/unreachable and thus cannot accept new
connections. The new unix@ bind() succeeded, the ".tmp" got renamed, the ".bak"
was unlinked. So far so good. But when the abns@ bind() fails, then the whole
new HAProxy instance fails and leaves the old instance "degraded".
I do not know how to simply fix this.
2)
I think the addrlen for bind() for abstract namespace sockets may be incorrect,
or perhaps unfriendly for other programs. At the very least socat (with
ABSTRACT-CONNECT on one end) cannot connect to the "test2" socket.
When haproxy is on both ends of such a socket, everything is fine, because it
uses the incorrect (?) addressing consistently :).
The second attached patch (haproxy_correct_abns_length.diff) seems to fix it.
No idea whether this is the right way to do it.
3)
HAProxy reload (the "-sf" stuff) fails when an abns@ socket is already bound.
The shutdown() trick doesn't seem to work, nor does SO_REUSEADDR. I don't know
whether it can be done without the old process closing the socket first. And
even then, the ERR_FATAL upon bind() failure in uxst_bind_listener() would
prevent the call of tell_old_pids().
I don't know how to fix this.
I can work around all of those problems by using plain old unix sockets
instead, but I like the idea of abstract namespace sockets :).
Regards,
--
hodor
commit dba4fc2068d6f16ff7ff2fa8d0b1c41d054b1f35
Author: Jan Seda <[email protected]>
Date: Thu Jun 26 20:44:05 2014 +0200
Do not unlink() abstract namespace sockets upon failure.
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index f83d34e..c9a52ff 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -309,11 +309,11 @@ static int uxst_bind_listener(struct listener *listener, char *errmsg, int errle
if (ret < 0 && errno == ENOENT)
unlink(path);
err_unlink_temp:
- if (!ext)
+ if (!ext && path[0])
unlink(tempname);
close(fd);
err_unlink_back:
- if (!ext)
+ if (!ext && path[0])
unlink(backname);
err_return:
if (msg && errlen) {
commit a2823cd72d59d29a9de7e7e9b29ab371067b1952
Author: Jan Seda <[email protected]>
Date: Thu Jun 26 23:36:07 2014 +0200
Abstract sockets: use correct length for interoperability
diff --git a/include/common/standard.h b/include/common/standard.h
index 8811c6f..887bd84 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -22,6 +22,7 @@
#ifndef _COMMON_STANDARD_H
#define _COMMON_STANDARD_H
+#include <stddef.h>
#include <limits.h>
#include <string.h>
#include <time.h>
@@ -721,7 +722,11 @@ static inline int get_addr_len(const struct sockaddr_storage *addr)
case AF_INET6:
return sizeof(struct sockaddr_in6);
case AF_UNIX:
- return sizeof(struct sockaddr_un);
+ if(((struct sockaddr_un *)addr)->sun_path[0])
+ return sizeof(struct sockaddr_un);
+ else
+ /* abstract namespace; the +1 is the leading zero */
+ return offsetof(struct sockaddr_un, sun_path) + 1 + strnlen(&((struct sockaddr_un *)addr)->sun_path[1], 108-1);
}
return 0;
}
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index c9a52ff..5ffd6ee 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -246,7 +246,7 @@ static int uxst_bind_listener(struct listener *listener, char *errmsg, int errle
goto err_unlink_temp;
}
- if (!ext && bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
+ if (!ext && bind(fd, (struct sockaddr *)&addr, get_addr_len((struct sockaddr_storage *)&addr)) < 0) {
/* note that bind() creates the socket <tempname> on the file system */
msg = "cannot bind UNIX socket";
goto err_unlink_temp;
