core: reliably check if varlink socket has been deserialized

Follow-up for 6906c028e83b77b35eaaf87b27d0fe5c6e1984b7

The mentioned commit uses access() to check if varlink socket
already exists in the filesystem, but that isn't sufficient.

> Varlink sockets are not serialized until v252, so upgrading from
> v251 or older means we will not listen anymore on the varlink sockets.
>
> See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1074789
> for more details as this was found when updating from Debian Bullseye to a new version.

After this commit, the set up of varlink_server is effectively
split into two steps. manager_varlink_init_system(), which is
called after deserialization, would no longer skip listening
even if Manager.varlink_server is in place, but actually
check if we're listening on desired sockets.
Then, manager_deserialize() can be switched back to using
manager_setup_varlink_server().

Alternative to #33817

Co-authored-by: Luca Boccassi <bluca@debian.org>
(cherry picked from commit d4e5c66ed469c822ca5346c7a445ec1446b1d17f)

diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c
index 3e6168d912a87d59a9a78496fdc37a8f5298cc91..8005f6d39a1b645c5a4d6e2427c22d651ce1a647 100644 (file)
--- a/src/core/core-varlink.c
+++ b/src/core/core-varlink.c
@@ -5,6 +5,7 @@
 #include "strv.h"
 #include "user-util.h"
 #include "varlink.h"
+#include "varlink-internal.h"
 #include "varlink-io.systemd.UserDatabase.h"
 #include "varlink-io.systemd.ManagedOOM.h"
 
@@ -500,12 +501,17 @@ static void vl_disconnect(VarlinkServer *s, Varlink *link, void *userdata) {
                 m->managed_oom_varlink = varlink_unref(link);
 }
 
-static int manager_setup_varlink_server(Manager *m, VarlinkServer **ret) {
+int manager_setup_varlink_server(Manager *m) {
         _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
         int r;
 
         assert(m);
-        assert(ret);
+
+        if (m->varlink_server)
+                return 0;
+
+        if (!MANAGER_IS_SYSTEM(m))
+                return -EINVAL;
 
         r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
         if (r < 0)
@@ -533,51 +539,51 @@ static int manager_setup_varlink_server(Manager *m, VarlinkServer **ret) {
         if (r < 0)
                 return log_debug_errno(r, "Failed to register varlink disconnect handler: %m");
 
-        *ret = TAKE_PTR(s);
-        return 0;
+        r = varlink_server_attach_event(s, m->event, EVENT_PRIORITY_IPC);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to attach varlink connection to event loop: %m");
+
+        m->varlink_server = TAKE_PTR(s);
+        return 1;
 }
 
 static int manager_varlink_init_system(Manager *m) {
-        _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
         int r;
 
         assert(m);
 
-        if (m->varlink_server)
-                return 1;
-
         if (!MANAGER_IS_SYSTEM(m))
                 return 0;
 
-        r = manager_setup_varlink_server(m, &s);
+        r = manager_setup_varlink_server(m);
         if (r < 0)
                 return log_error_errno(r, "Failed to set up varlink server: %m");
+        bool fresh = r > 0;
 
         if (!MANAGER_IS_TEST_RUN(m)) {
                 (void) mkdir_p_label("/run/systemd/userdb", 0755);
 
                 FOREACH_STRING(address, "/run/systemd/userdb/io.systemd.DynamicUser", VARLINK_ADDR_PATH_MANAGED_OOM_SYSTEM) {
-                        if (MANAGER_IS_RELOADING(m)) {
-                                /* If manager is reloading, we skip listening on existing addresses, since
-                                 * the fd should be acquired later through deserialization. */
-                                if (access(address, F_OK) >= 0)
+                        if (!fresh) {
+                                /* We might have got sockets through deserialization. Do not bind to them twice. */
+
+                                bool found = false;
+                                LIST_FOREACH(sockets, ss, m->varlink_server->sockets)
+                                        if (path_equal(ss->address, address)) {
+                                                found = true;
+                                                break;
+                                        }
+
+                                if (found)
                                         continue;
-                                if (errno != ENOENT)
-                                        return log_error_errno(errno,
-                                                               "Failed to check if varlink socket '%s' exists: %m", address);
                         }
 
-                        r = varlink_server_listen_address(s, address, 0666);
+                        r = varlink_server_listen_address(m->varlink_server, address, 0666);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to bind to varlink socket '%s': %m", address);
                 }
         }
 
-        r = varlink_server_attach_event(s, m->event, EVENT_PRIORITY_IPC);
-        if (r < 0)
-                return log_error_errno(r, "Failed to attach varlink connection to event loop: %m");
-
-        m->varlink_server = TAKE_PTR(s);
         return 1;
 }
 

diff --git a/src/core/core-varlink.h b/src/core/core-varlink.h
index 20507a418746d500b87bf3e5d27d0f343f2ff828..4b776200289be2bc63e168969a0c3f41a1926c6a 100644 (file)
--- a/src/core/core-varlink.h
+++ b/src/core/core-varlink.h
@@ -3,6 +3,8 @@
 
 #include "manager.h"
 
+int manager_setup_varlink_server(Manager *m);
+
 int manager_varlink_init(Manager *m);
 void manager_varlink_done(Manager *m);
 

diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c
index b4af82b4e17cbf7a93d162284fbc15208c607311..1d2959abf4395fe7e26a9c16c556c12aa7cb63b9 100644 (file)
--- a/src/core/manager-serialize.c
+++ b/src/core/manager-serialize.c
@@ -506,7 +506,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                                 return r;
                 } else if ((val = startswith(l, "varlink-server-socket-address="))) {
                         if (!m->varlink_server && MANAGER_IS_SYSTEM(m)) {
-                                r = manager_varlink_init(m);
+                                r = manager_setup_varlink_server(m);
                                 if (r < 0) {
                                         log_warning_errno(r, "Failed to setup varlink server, ignoring: %m");
                                         continue;

diff --git a/src/shared/varlink-internal.h b/src/shared/varlink-internal.h
index 715202a49e1ef2aec9412645d33045e087e746df..bc301084cdb7759bd2d6651f37a7f45c814a017c 100644 (file)
--- a/src/shared/varlink-internal.h
+++ b/src/shared/varlink-internal.h
@@ -6,5 +6,45 @@
 #include "fdset.h"
 #include "varlink.h"
 
+typedef struct VarlinkServerSocket VarlinkServerSocket;
+
+struct VarlinkServerSocket {
+        VarlinkServer *server;
+
+        int fd;
+        char *address;
+
+        sd_event_source *event_source;
+
+        LIST_FIELDS(VarlinkServerSocket, sockets);
+};
+
+struct VarlinkServer {
+        unsigned n_ref;
+        VarlinkServerFlags flags;
+
+        LIST_HEAD(VarlinkServerSocket, sockets);
+
+        Hashmap *methods;              /* Fully qualified symbol name of a method → VarlinkMethod */
+        Hashmap *interfaces;           /* Fully qualified interface name → VarlinkInterface* */
+        Hashmap *symbols;              /* Fully qualified symbol name of method/error → VarlinkSymbol* */
+        VarlinkConnect connect_callback;
+        VarlinkDisconnect disconnect_callback;
+
+        sd_event *event;
+        int64_t event_priority;
+
+        unsigned n_connections;
+        Hashmap *by_uid;               /* UID_TO_PTR(uid) → UINT_TO_PTR(n_connections) */
+
+        void *userdata;
+        char *description;
+
+        unsigned connections_max;
+        unsigned connections_per_uid_max;
+
+        bool exit_on_idle;
+};
+
 int varlink_server_serialize(VarlinkServer *s, FILE *f, FDSet *fds);
 int varlink_server_deserialize_one(VarlinkServer *s, const char *value, FDSet *fds);

diff --git a/src/shared/varlink.c b/src/shared/varlink.c
index 034e72b1db8af34ce7aa4be800778febd9b748d4..0a6d2c822f5f92eaa2c2d501b82443740fa35241 100644 (file)
--- a/src/shared/varlink.c
+++ b/src/shared/varlink.c
@@ -210,46 +210,6 @@ struct Varlink {
         pid_t exec_pid;
 };
 
-typedef struct VarlinkServerSocket VarlinkServerSocket;
-
-struct VarlinkServerSocket {
-        VarlinkServer *server;
-
-        int fd;
-        char *address;
-
-        sd_event_source *event_source;
-
-        LIST_FIELDS(VarlinkServerSocket, sockets);
-};
-
-struct VarlinkServer {
-        unsigned n_ref;
-        VarlinkServerFlags flags;
-
-        LIST_HEAD(VarlinkServerSocket, sockets);
-
-        Hashmap *methods;              /* Fully qualified symbol name of a method → VarlinkMethod */
-        Hashmap *interfaces;           /* Fully qualified interface name → VarlinkInterface* */
-        Hashmap *symbols;              /* Fully qualified symbol name of method/error → VarlinkSymbol* */
-        VarlinkConnect connect_callback;
-        VarlinkDisconnect disconnect_callback;
-
-        sd_event *event;
-        int64_t event_priority;
-
-        unsigned n_connections;
-        Hashmap *by_uid;               /* UID_TO_PTR(uid) → UINT_TO_PTR(n_connections) */
-
-        void *userdata;
-        char *description;
-
-        unsigned connections_max;
-        unsigned connections_per_uid_max;
-
-        bool exit_on_idle;
-};
-
 static const char* const varlink_state_table[_VARLINK_STATE_MAX] = {
         [VARLINK_IDLE_CLIENT]              = "idle-client",
         [VARLINK_AWAITING_REPLY]           = "awaiting-reply",