From dfa2b389a627652e0e190473817073c44cf19f2b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 May 2022 16:15:26 +0200 Subject: [PATCH 1/4] socket-util: change sockaddr_un_set_path() to return recognizable error on 108ch limit This way we can implement nice fallbacks later on. While we are at it, provide a test for this (one that is a bit over the top, but then again, we can never have enough tests). --- src/basic/socket-util.c | 5 +++- src/test/test-socket-util.c | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 8d0494ece5..0dfe2a7dbc 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -1236,7 +1236,10 @@ int sockaddr_un_set_path(struct sockaddr_un *ret, const char *path) { * addresses!), which the kernel doesn't. We do this to reduce chance of incompatibility with other apps that * do not expect non-NUL terminated file system path. */ if (l+1 > sizeof(ret->sun_path)) - return -EINVAL; + return path[0] == '@' ? -EINVAL : -ENAMETOOLONG; /* return a recognizable error if this is + * too long to fit into a sockaddr_un, but + * is a file system path, and thus might be + * connectible via O_PATH indirection. */ *ret = (struct sockaddr_un) { .sun_family = AF_UNIX, diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 3245516f9a..cd1e374ec8 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -11,11 +12,15 @@ #include "escape.h" #include "exit-status.h" #include "fd-util.h" +#include "fs-util.h" #include "in-addr-util.h" #include "io-util.h" #include "log.h" #include "macro.h" +#include "path-util.h" #include "process-util.h" +#include "random-util.h" +#include "rm-rf.h" #include "socket-util.h" #include "string-util.h" #include "tests.h" @@ -478,4 +483,46 @@ TEST(ipv6_enabled) { log_info("IPv6 enabled: %s", yes_no(socket_ipv6_is_enabled())); } +TEST(sockaddr_un_set_path) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_(unlink_and_freep) char *sh = NULL; + _cleanup_free_ char *j = NULL; + union sockaddr_union sa; + _cleanup_close_ int fd1 = -1, fd2 = -1, fd3 = -1; + + assert_se(mkdtemp_malloc("/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaXXXXXX", &t) >= 0); + assert_se(strlen(t) > SUN_PATH_LEN); + + assert_se(j = path_join(t, "sock")); + assert_se(sockaddr_un_set_path(&sa.un, j) == -ENAMETOOLONG); /* too long for AF_UNIX socket */ + + assert_se(asprintf(&sh, "/tmp/%" PRIx64, random_u64()) >= 0); + assert_se(symlink(t, sh) >= 0); /* create temporary symlink, to access it anyway */ + + free(j); + assert_se(j = path_join(sh, "sock")); + assert_se(sockaddr_un_set_path(&sa.un, j) >= 0); + + fd1 = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); + assert_se(fd1 >= 0); + assert_se(bind(fd1, &sa.sa, SOCKADDR_LEN(sa)) >= 0); + assert_se(listen(fd1, 1) >= 0); + + sh = unlink_and_free(sh); /* remove temporary symlink */ + + fd2 = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); + assert_se(fd2 >= 0); + assert_se(connect(fd2, &sa.sa, SOCKADDR_LEN(sa)) < 0); + assert_se(errno == ENOENT); /* we removed the symlink, must fail */ + + free(j); + assert_se(j = path_join(t, "sock")); + + fd3 = open(j, O_CLOEXEC|O_PATH|O_NOFOLLOW); + assert_se(fd3 > 0); + assert_se(sockaddr_un_set_path(&sa.un, FORMAT_PROC_FD_PATH(fd3)) >= 0); /* connect via O_PATH instead, circumventing 108ch limit */ + + assert_se(connect(fd2, &sa.sa, SOCKADDR_LEN(sa)) >= 0); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 28fe6a8072350134f7ff62b6ab7f01081fa11c9a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 May 2022 16:22:16 +0200 Subject: [PATCH 2/4] fileio: propagate original error if we notice AF_UNIX connect() is not going to work let's not make up new errors in these checks that validate if connect() work at all. After all, we don't really know if the ENXIO we saw earlier actually is really caused by the inode being an AF_UNIX socket, we just have the suspicion... --- src/basic/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index e7b670ab2e..1483c76228 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -776,7 +776,7 @@ int read_full_file_full( /* Seeking is not supported on AF_UNIX sockets */ if (offset != UINT64_MAX) - return -ESPIPE; + return -ENXIO; if (dir_fd == AT_FDCWD) r = sockaddr_un_set_path(&sa.un, filename); From 2c032478fca9c57526faec422ffbb87ecafc1251 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 May 2022 16:23:05 +0200 Subject: [PATCH 3/4] fileio: fix error propagation --- src/basic/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 1483c76228..c2497ff856 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -809,7 +809,7 @@ int read_full_file_full( return r; if (bind(sk, &bsa.sa, r) < 0) - return r; + return -errno; } if (connect(sk, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) From a98042e7a3a6d81e0cdb256458695a82f6dcafd3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 13 May 2022 21:49:53 +0900 Subject: [PATCH 4/4] userdb: fix error handling --- src/userdb/userdbctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index 9effb3c61e..eddbe765b0 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -957,7 +957,7 @@ static int display_services(int argc, char *argv[], void *userdata) { fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); if (fd < 0) - return log_error_errno(r, "Failed to allocate AF_UNIX/SOCK_STREAM socket: %m"); + return log_error_errno(errno, "Failed to allocate AF_UNIX/SOCK_STREAM socket: %m"); if (connect(fd, &sockaddr.sa, sockaddr_len) < 0) { no = strjoin("No (", errno_to_name(errno), ")");