From 77b79723a6595728dfd48940754bef08150d48e0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Sep 2017 18:26:20 +0200 Subject: [PATCH 1/5] fs-util: propagate EEXIST error in symlink_idempotent() as EEXIST We really shouldn't silently translate the error code here for no reason. --- src/basic/fs-util.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index fdedeffb9a..af63171041 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -359,22 +359,25 @@ int touch(const char *path) { } int symlink_idempotent(const char *from, const char *to) { - _cleanup_free_ char *p = NULL; int r; assert(from); assert(to); if (symlink(from, to) < 0) { + _cleanup_free_ char *p = NULL; + if (errno != EEXIST) return -errno; r = readlink_malloc(to, &p); - if (r < 0) + if (r == -EINVAL) /* Not a symlink? In that case return the original error we encountered: -EEXIST */ + return -EEXIST; + if (r < 0) /* Any other error? In that case propagate it as is */ return r; - if (!streq(p, from)) - return -EINVAL; + if (!streq(p, from)) /* Not the symlink we want it to be? In that case, propagate the original -EEXIST */ + return -EEXIST; } return 0; From 95f7fbbf887955bdbb790d61091487b48d51de9d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Sep 2017 18:27:09 +0200 Subject: [PATCH 2/5] socket: make sure we warn loudly about symlinks we can't create Note that this change does not make symlink creation failing fatal. I am not entirely sure about whether it should be, but I am leaning towards not making it fatal for two reasons: symlinks like this tend to be a compatibility feature, and hence unlikely to be essential for operation, in a way this breaks compatibility, and while doing that is not off the table, we should probably avoid it if we are not entirely sure it's a good thing. Note that this also changes plain symlink() to symlink_idempotent() so that existing symlinks with the right destination are nothing we log about. Fixes: #6920 --- src/core/socket.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 3b84ffa2a7..d1d4007588 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -38,6 +38,7 @@ #include "exit-status.h" #include "fd-util.h" #include "format-util.h" +#include "fs-util.h" #include "in-addr-util.h" #include "io-util.h" #include "label.h" @@ -1324,6 +1325,7 @@ static int mq_address_create( static int socket_symlink(Socket *s) { const char *p; char **i; + int r; assert(s); @@ -1331,8 +1333,11 @@ static int socket_symlink(Socket *s) { if (!p) return 0; - STRV_FOREACH(i, s->symlinks) - symlink_label(p, *i); + STRV_FOREACH(i, s->symlinks) { + r = symlink_idempotent(p, *i); + if (r < 0) + log_unit_warning_errno(UNIT(s), r, "Failed to create symlink %s → %s, ignoring: %m", p, *i); + } return 0; } From 1af87ab7d65b102e3ce5a64e8b47e4d08fc407a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Sep 2017 18:27:56 +0200 Subject: [PATCH 3/5] socket: create leading directories for socket symlinks It really doesn't hurt creating prefix directories if necessary, as we tend to do that for other file nodes we create, too. Fixes: #6920 --- src/core/socket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/socket.c b/src/core/socket.c index d1d4007588..157a6dbae7 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1175,7 +1175,7 @@ static int fifo_address_create( assert(path); - mkdir_parents_label(path, directory_mode); + (void) mkdir_parents_label(path, directory_mode); r = mac_selinux_create_file_prepare(path, S_IFIFO); if (r < 0) @@ -1334,6 +1334,8 @@ static int socket_symlink(Socket *s) { return 0; STRV_FOREACH(i, s->symlinks) { + (void) mkdir_parents_label(*i, s->directory_mode); + r = symlink_idempotent(p, *i); if (r < 0) log_unit_warning_errno(UNIT(s), r, "Failed to create symlink %s → %s, ignoring: %m", p, *i); From 3ecc7fc3e631d1f5de7bd0913227ae2f2e8f8929 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Sep 2017 18:36:41 +0200 Subject: [PATCH 4/5] man: document that inability to execute Symlinks= is not fatal Fixes: #6920 --- man/systemd.socket.xml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml index 2dfca713e4..68d01cccc5 100644 --- a/man/systemd.socket.xml +++ b/man/systemd.socket.xml @@ -804,14 +804,13 @@ Symlinks= - Takes a list of file system paths. The - specified paths will be created as symlinks to the AF_UNIX - socket path or FIFO path of this socket unit. If this setting - is used, only one AF_UNIX socket in the file system or one - FIFO may be configured for the socket unit. Use this option to - manage one or more symlinked alias names for a socket, binding - their lifecycle together. If the empty string is assigned, the - list of paths is reset. Defaults to the empty list. + Takes a list of file system paths. The specified paths will be created as symlinks to the + AF_UNIX socket path or FIFO path of this socket unit. If this setting is used, only one + AF_UNIX socket in the file system or one FIFO may be configured for the socket unit. Use + this option to manage one or more symlinked alias names for a socket, binding their lifecycle together. Note + that if creation of a symlink fails this is not considered fatal for the socket unit, and the socket unit may + still start. If an empty string is assigned, the list of paths is reset. Defaults to an empty + list. From 22b20752e20ce0e637083067b450f61cf4acf691 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Sep 2017 17:48:28 +0200 Subject: [PATCH 5/5] socket: if RemoveOnStop= is turned on for a socket, try to unlink() pre-existing symlinks Normally, Symlinks= failing is not considered fatal nor destructive. Let's slightly alter behaviour here if RemoveOnStop= is turned on. In that case the use in a way opted for destructive behaviour and we do unlink all sockets and symlinks when the socket unit goes down. And that means we might as well unlink any pre-existing if this mode is selected. Yeah, it's a bit of a stretch to do this, but @OhNoMoreGit is right: if RemoveOnStop= is on we are destructive regarding any pre-existing symlinks on stop, and it would be quite weird if we wouldn't be on start. --- src/core/socket.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/core/socket.c b/src/core/socket.c index 157a6dbae7..ba70756d83 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1337,6 +1337,16 @@ static int socket_symlink(Socket *s) { (void) mkdir_parents_label(*i, s->directory_mode); r = symlink_idempotent(p, *i); + + if (r == -EEXIST && s->remove_on_stop) { + /* If there's already something where we want to create the symlink, and the destructive + * RemoveOnStop= mode is set, then we might as well try to remove what already exists and try + * again. */ + + if (unlink(*i) >= 0) + r = symlink_idempotent(p, *i); + } + if (r < 0) log_unit_warning_errno(UNIT(s), r, "Failed to create symlink %s → %s, ignoring: %m", p, *i); }