Merge pull request #24919 from anitazha/varlinkserialize

varlink/pid1 fixes for systemd-oomd (addresses #20330)

7 files changed, 239 insertions, 29 deletions

diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c
index 1fdcf885b8..843271593d 100644
--- a/src/core/core-varlink.c
+++ b/src/core/core-varlink.c
@@ -203,10 +203,25 @@ static int vl_method_subscribe_managed_oom_cgroups(
 
         _cleanup_(json_variant_unrefp) JsonVariant *v = NULL;
         Manager *m = ASSERT_PTR(userdata);
+        pid_t pid;
+        Unit *u;
         int r;
 
         assert(link);
 
+        r = varlink_get_peer_pid(link, &pid);
+        if (r < 0)
+                return r;
+
+        u = manager_get_unit_by_pid(m, pid);
+        if (!u)
+                return varlink_error(link, VARLINK_ERROR_PERMISSION_DENIED, NULL);
+
+        /* This is meant to be a deterrent and not actual security. The alternative is to check for the systemd-oom
+         * user that this unit runs as, but NSS lookups are blocking and not allowed from PID 1. */
+        if (!streq(u->id, "systemd-oomd.service"))
+                return varlink_error(link, VARLINK_ERROR_PERMISSION_DENIED, NULL);
+
         if (json_variant_elements(parameters) > 0)
                 return varlink_error_invalid_parameter(link, parameters);
 
@@ -482,24 +497,9 @@ static int manager_varlink_init_system(Manager *m) {
         if (!MANAGER_IS_SYSTEM(m))
                 return 0;
 
-        r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
-        if (r < 0)
-                return log_error_errno(r, "Failed to allocate varlink server object: %m");
-
-        varlink_server_set_userdata(s, m);
-
-        r = varlink_server_bind_method_many(
-                        s,
-                        "io.systemd.UserDatabase.GetUserRecord",  vl_method_get_user_record,
-                        "io.systemd.UserDatabase.GetGroupRecord", vl_method_get_group_record,
-                        "io.systemd.UserDatabase.GetMemberships", vl_method_get_memberships,
-                        "io.systemd.ManagedOOM.SubscribeManagedOOMCGroups",  vl_method_subscribe_managed_oom_cgroups);
-        if (r < 0)
-                return log_error_errno(r, "Failed to register varlink methods: %m");
-
-        r = varlink_server_bind_disconnect(s, vl_disconnect);
+        r = manager_setup_varlink_server(m, &s);
         if (r < 0)
-                return log_error_errno(r, "Failed to register varlink disconnect handler: %m");
+                return log_error_errno(r, "Failed to set up varlink server: %m");
 
         if (!MANAGER_IS_TEST_RUN(m)) {
                 (void) mkdir_p_label("/run/systemd/userdb", 0755);
@@ -582,6 +582,36 @@ static int manager_varlink_init_user(Manager *m) {
         return 1;
 }
 
+int manager_setup_varlink_server(Manager *m, VarlinkServer **ret) {
+        _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
+        int r;
+
+        assert(m);
+        assert(ret);
+
+        r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to allocate varlink server object: %m");
+
+        varlink_server_set_userdata(s, m);
+
+        r = varlink_server_bind_method_many(
+                        s,
+                        "io.systemd.UserDatabase.GetUserRecord",  vl_method_get_user_record,
+                        "io.systemd.UserDatabase.GetGroupRecord", vl_method_get_group_record,
+                        "io.systemd.UserDatabase.GetMemberships", vl_method_get_memberships,
+                        "io.systemd.ManagedOOM.SubscribeManagedOOMCGroups",  vl_method_subscribe_managed_oom_cgroups);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to register varlink methods: %m");
+
+        r = varlink_server_bind_disconnect(s, vl_disconnect);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to register varlink disconnect handler: %m");
+
+        *ret = TAKE_PTR(s);
+        return 0;
+}
+
 int manager_varlink_init(Manager *m) {
         return MANAGER_IS_SYSTEM(m) ? manager_varlink_init_system(m) : manager_varlink_init_user(m);
 }
diff --git a/src/core/core-varlink.h b/src/core/core-varlink.h
index 20507a4187..7f810d1f25 100644
--- a/src/core/core-varlink.h
+++ b/src/core/core-varlink.h
@@ -6,6 +6,10 @@
 int manager_varlink_init(Manager *m);
 void manager_varlink_done(Manager *m);
 
+/* Creates a new VarlinkServer and binds methods. Does not set up sockets or attach events.
+ * Used for manager serialize/deserialize. */
+int manager_setup_varlink_server(Manager *m, VarlinkServer **ret_s);
+
 /* The manager is expected to send an update to systemd-oomd if one of the following occurs:
  * - The value of ManagedOOM*= properties change
  * - A unit with ManagedOOM*= properties changes unit active state */
diff --git a/src/core/manager-serialize.c b/src/core/manager-serialize.c
index 914bd92e36..27cb0925ae 100644
--- a/src/core/manager-serialize.c
+++ b/src/core/manager-serialize.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "clean-ipc.h"
+#include "core-varlink.h"
 #include "dbus.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -13,6 +14,7 @@
 #include "syslog-util.h"
 #include "unit-serialize.h"
 #include "user-util.h"
+#include "varlink-internal.h"
 
 int manager_open_serialization(Manager *m, FILE **ret_f) {
         _cleanup_close_ int fd = -1;
@@ -175,6 +177,10 @@ int manager_serialize(
         if (r < 0)
                 return r;
 
+        r = varlink_server_serialize(m->varlink_server, f, fds);
+        if (r < 0)
+                return r;
+
         (void) fputc('\n', f);
 
         HASHMAP_FOREACH_KEY(u, t, m->units) {
@@ -290,6 +296,7 @@ static void manager_deserialize_gid_refs_one(Manager *m, const char *value) {
 }
 
 int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
+        bool deserialize_varlink_sockets = false;
         int r = 0;
 
         assert(m);
@@ -516,7 +523,32 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
 
                         if (strv_extend(&m->deserialized_subscribed, val) < 0)
                                 return -ENOMEM;
+                } else if ((val = startswith(l, "varlink-server-socket-address="))) {
+                        if (!m->varlink_server && MANAGER_IS_SYSTEM(m)) {
+                                _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
+
+                                r = manager_setup_varlink_server(m, &s);
+                                if (r < 0) {
+                                        log_warning_errno(r, "Failed to setup varlink server, ignoring: %m");
+                                        continue;
+                                }
+
+                                r = varlink_server_attach_event(s, m->event, SD_EVENT_PRIORITY_NORMAL);
+                                if (r < 0) {
+                                        log_warning_errno(r, "Failed to attach varlink connection to event loop, ignoring: %m");
+                                        continue;
+                                }
+
+                                m->varlink_server = TAKE_PTR(s);
+                                deserialize_varlink_sockets = true;
+                        }
 
+                        /* To void unnecessary deserialization (i.e. during reload vs. reexec) we only deserialize
+                         * the FDs if we had to create a new m->varlink_server. The deserialize_varlink_sockets flag
+                         * is initialized outside of the loop, is flipped after the VarlinkServer is setup, and
+                         * remains set until all serialized contents are handled. */
+                        if (deserialize_varlink_sockets)
+                                (void) varlink_server_deserialize_one(m->varlink_server, val, fds);
                 } else {
                         ManagerTimestamp q;
 
diff --git a/src/shared/meson.build b/src/shared/meson.build
index e805e9b898..9e11e13934 100644
--- a/src/shared/meson.build
+++ b/src/shared/meson.build
@@ -326,6 +326,7 @@ shared_sources = files(
         'utmp-wtmp.h',
         'varlink.c',
         'varlink.h',
+        'varlink-internal.h',
         'verb-log-control.c',
         'verb-log-control.h',
         'verbs.c',
diff --git a/src/shared/varlink-internal.h b/src/shared/varlink-internal.h
new file mode 100644
index 0000000000..715202a49e
--- /dev/null
+++ b/src/shared/varlink-internal.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include <stdio.h>
+
+#include "fdset.h"
+#include "varlink.h"
+
+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 580ebac4db..4f7ac97689 100644
--- a/src/shared/varlink.c
+++ b/src/shared/varlink.c
@@ -12,6 +12,7 @@
 #include "list.h"
 #include "process-util.h"
 #include "selinux-util.h"
+#include "serialize.h"
 #include "set.h"
 #include "socket-util.h"
 #include "string-table.h"
@@ -21,6 +22,7 @@
 #include "umask-util.h"
 #include "user-util.h"
 #include "varlink.h"
+#include "varlink-internal.h"
 
 #define VARLINK_DEFAULT_CONNECTIONS_MAX 4096U
 #define VARLINK_DEFAULT_CONNECTIONS_PER_UID_MAX 1024U
@@ -2197,6 +2199,16 @@ int varlink_server_add_connection(VarlinkServer *server, int fd, Varlink **ret)
         return 0;
 }
 
+static VarlinkServerSocket *varlink_server_socket_free(VarlinkServerSocket *ss) {
+        if (!ss)
+                return NULL;
+
+        free(ss->address);
+        return mfree(ss);
+}
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(VarlinkServerSocket *, varlink_server_socket_free);
+
 static int connect_callback(sd_event_source *source, int fd, uint32_t revents, void *userdata) {
         VarlinkServerSocket *ss = ASSERT_PTR(userdata);
         _cleanup_close_ int cfd = -1;
@@ -2233,12 +2245,13 @@ static int connect_callback(sd_event_source *source, int fd, uint32_t revents, v
         return 0;
 }
 
-int varlink_server_listen_fd(VarlinkServer *s, int fd) {
-        _cleanup_free_ VarlinkServerSocket *ss = NULL;
+static int varlink_server_create_listen_fd_socket(VarlinkServer *s, int fd, VarlinkServerSocket **ret_ss) {
+        _cleanup_(varlink_server_socket_freep) VarlinkServerSocket *ss = NULL;
         int r;
 
-        assert_return(s, -EINVAL);
-        assert_return(fd >= 0, -EBADF);
+        assert(s);
+        assert(fd >= 0);
+        assert(ret_ss);
 
         r = fd_nonblock(fd, true);
         if (r < 0)
@@ -2263,11 +2276,27 @@ int varlink_server_listen_fd(VarlinkServer *s, int fd) {
                         return r;
         }
 
+        *ret_ss = TAKE_PTR(ss);
+        return 0;
+}
+
+int varlink_server_listen_fd(VarlinkServer *s, int fd) {
+        _cleanup_(varlink_server_socket_freep) VarlinkServerSocket *ss = NULL;
+        int r;
+
+        assert_return(s, -EINVAL);
+        assert_return(fd >= 0, -EBADF);
+
+        r = varlink_server_create_listen_fd_socket(s, fd, &ss);
+        if (r < 0)
+                return r;
+
         LIST_PREPEND(sockets, s->sockets, TAKE_PTR(ss));
         return 0;
 }
 
 int varlink_server_listen_address(VarlinkServer *s, const char *address, mode_t m) {
+        _cleanup_(varlink_server_socket_freep) VarlinkServerSocket *ss = NULL;
         union sockaddr_union sockaddr;
         socklen_t sockaddr_len;
         _cleanup_close_ int fd = -1;
@@ -2299,10 +2328,15 @@ int varlink_server_listen_address(VarlinkServer *s, const char *address, mode_t
         if (listen(fd, SOMAXCONN) < 0)
                 return -errno;
 
-        r = varlink_server_listen_fd(s, fd);
+        r = varlink_server_create_listen_fd_socket(s, fd, &ss);
+        if (r < 0)
+                return r;
+
+        r = free_and_strdup(&ss->address, address);
         if (r < 0)
                 return r;
 
+        LIST_PREPEND(sockets, s->sockets, TAKE_PTR(ss));
         TAKE_FD(fd);
         return 0;
 }
@@ -2348,6 +2382,29 @@ int varlink_server_shutdown(VarlinkServer *s) {
         return 0;
 }
 
+static int varlink_server_add_socket_event_source(VarlinkServer *s, VarlinkServerSocket *ss, int64_t priority) {
+        _cleanup_(sd_event_source_unrefp) sd_event_source *es = NULL;
+
+        int r;
+
+        assert(s);
+        assert(s->event);
+        assert(ss);
+        assert(ss->fd >= 0);
+        assert(!ss->event_source);
+
+        r = sd_event_add_io(s->event, &es, ss->fd, EPOLLIN, connect_callback, ss);
+        if (r < 0)
+                return r;
+
+        r = sd_event_source_set_priority(es, priority);
+        if (r < 0)
+                return r;
+
+        ss->event_source = TAKE_PTR(es);
+        return 0;
+}
+
 int varlink_server_attach_event(VarlinkServer *s, sd_event *e, int64_t priority) {
         int r;
 
@@ -2363,13 +2420,7 @@ int varlink_server_attach_event(VarlinkServer *s, sd_event *e, int64_t priority)
         }
 
         LIST_FOREACH(sockets, ss, s->sockets) {
-                assert(!ss->event_source);
-
-                r = sd_event_add_io(s->event, &ss->event_source, ss->fd, EPOLLIN, connect_callback, ss);
-                if (r < 0)
-                        goto fail;
-
-                r = sd_event_source_set_priority(ss->event_source, priority);
+                r = varlink_server_add_socket_event_source(s, ss, priority);
                 if (r < 0)
                         goto fail;
         }
@@ -2528,3 +2579,84 @@ int varlink_server_set_description(VarlinkServer *s, const char *description) {
 
         return free_and_strdup(&s->description, description);
 }
+
+int varlink_server_serialize(VarlinkServer *s, FILE *f, FDSet *fds) {
+        assert(f);
+        assert(fds);
+
+        if (!s)
+                return 0;
+
+        LIST_FOREACH(sockets, ss, s->sockets) {
+                int copy;
+
+                assert(ss->address);
+                assert(ss->fd >= 0);
+
+                fprintf(f, "varlink-server-socket-address=%s", ss->address);
+
+                /* If we fail to serialize the fd, it will be considered an error during deserialization */
+                copy = fdset_put_dup(fds, ss->fd);
+                if (copy < 0)
+                        return copy;
+
+                fprintf(f, " varlink-server-socket-fd=%i", copy);
+
+                fputc('\n', f);
+        }
+
+        return 0;
+}
+
+int varlink_server_deserialize_one(VarlinkServer *s, const char *value, FDSet *fds) {
+        _cleanup_(varlink_server_socket_freep) VarlinkServerSocket *ss = NULL;
+        _cleanup_free_ char *address = NULL;
+        const char *v = ASSERT_PTR(value);
+        int r, fd = -1;
+        char *buf;
+        size_t n;
+
+        assert(s);
+        assert(fds);
+
+        n = strcspn(v, " ");
+        address = strndup(v, n);
+        if (!address)
+                return log_oom_debug();
+
+        if (v[n] != ' ')
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Failed to deserialize VarlinkServerSocket: %s: %m", value);
+        v = startswith(v + n + 1, "varlink-server-socket-fd=");
+        if (!v)
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Failed to deserialize VarlinkServerSocket fd %s: %m", value);
+
+        n = strcspn(v, " ");
+        buf = strndupa_safe(v, n);
+
+        r = safe_atoi(buf, &fd);
+        if (r < 0)
+                return log_debug_errno(r, "Unable to parse VarlinkServerSocket varlink-server-socket-fd=%s: %m", buf);
+
+        if (!fdset_contains(fds, fd))
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADF),
+                                       "VarlinkServerSocket varlink-server-socket-fd= has unknown fd %d: %m", fd);
+
+        ss = new(VarlinkServerSocket, 1);
+        if (!ss)
+                return log_oom_debug();
+
+        *ss = (VarlinkServerSocket) {
+                .server = s,
+                .address = TAKE_PTR(address),
+                .fd = fdset_remove(fds, fd),
+        };
+
+        r = varlink_server_add_socket_event_source(s, ss, SD_EVENT_PRIORITY_NORMAL);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to add VarlinkServerSocket event source to the event loop: %m");
+
+        LIST_PREPEND(sockets, s->sockets, TAKE_PTR(ss));
+        return 0;
+}
diff --git a/src/shared/varlink.h b/src/shared/varlink.h
index 66a1ff630e..9518cd9098 100644
--- a/src/shared/varlink.h
+++ b/src/shared/varlink.h
@@ -173,3 +173,4 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(VarlinkServer *, varlink_server_unref);
 #define VARLINK_ERROR_METHOD_NOT_IMPLEMENTED "org.varlink.service.MethodNotImplemented"
 #define VARLINK_ERROR_INVALID_PARAMETER "org.varlink.service.InvalidParameter"
 #define VARLINK_ERROR_SUBSCRIPTION_TAKEN "org.varlink.service.SubscriptionTaken"
+#define VARLINK_ERROR_PERMISSION_DENIED "org.varlink.service.PermissionDenied"