Merge pull request #22552 from poettering/nss-by-pass-bus-rework

fix nss deadlock between dbus-daemon and PID 1

docs/ENVIRONMENT.md

@@ -219,10 +219,6 @@ All tools:
   user/group records for dynamically registered service users (i.e. users
   registered through `DynamicUser=1`).
 
-* `$SYSTEMD_NSS_BYPASS_BUS=1` — if set, `nss-systemd` won't use D-Bus to do
-  dynamic user lookups. This is primarily useful to make `nss-systemd` work
-  safely from within `dbus-daemon`.
-
 `systemd-timedated`:
 
 * `$SYSTEMD_TIMEDATED_NTP_SERVICES=…` — colon-separated list of unit names of

src/core/execute.c

@@ -1865,11 +1865,11 @@ static int build_environment(
                 our_env[n_env++] = x;
         }
 
-        /* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use D-Bus look up dynamic
-         * users via PID 1, possibly dead-locking the dbus daemon. This way it will not use D-Bus to resolve names, but
-         * check the database directly. */
-        if (p->flags & EXEC_NSS_BYPASS_BUS) {
-                x = strdup("SYSTEMD_NSS_BYPASS_BUS=1");
+        /* If this is D-Bus, tell the nss-systemd module, since it relies on being able to use blocking
+         * Varlink calls back to us for look up dynamic users in PID 1. Break the deadlock between D-Bus and
+         * PID 1 by disabling use of PID1' NSS interface for looking up dynamic users. */
+        if (p->flags & EXEC_NSS_DYNAMIC_BYPASS) {
+                x = strdup("SYSTEMD_NSS_DYNAMIC_BYPASS=1");
                 if (!x)
                         return -ENOMEM;
                 our_env[n_env++] = x;

src/core/execute.h

@@ -376,7 +376,7 @@ typedef enum ExecFlags {
         EXEC_APPLY_TTY_STDIN       = 1 << 2,
         EXEC_PASS_LOG_UNIT         = 1 << 3, /* Whether to pass the unit name to the service's journal stream connection */
         EXEC_CHOWN_DIRECTORIES     = 1 << 4, /* chown() the runtime/state/cache/log directories to the user we run as, under all conditions */
-        EXEC_NSS_BYPASS_BUS        = 1 << 5, /* Set the SYSTEMD_NSS_BYPASS_BUS environment variable, to disable nss-systemd for dbus */
+        EXEC_NSS_DYNAMIC_BYPASS    = 1 << 5, /* Set the SYSTEMD_NSS_DYNAMIC_BYPASS environment variable, to disable nss-systemd blocking on PID 1, for use by dbus-daemon */
         EXEC_CGROUP_DELEGATE       = 1 << 6,
         EXEC_IS_CONTROL            = 1 << 7,
         EXEC_CONTROL_CGROUP        = 1 << 8, /* Place the process not in the indicated cgroup but in a subcgroup '/.control', but only EXEC_CGROUP_DELEGATE and EXEC_IS_CONTROL is set, too */

src/core/service.c

@@ -398,6 +398,8 @@ static void service_done(Unit *u) {
         s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source);
         s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source);
 
+        s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
+
         service_release_resources(u);
 }
 
@@ -709,17 +711,19 @@ static int service_setup_bus_name(Service *s) {
         assert(s);
 
         /* If s->bus_name is not set, then the unit will be refused by service_verify() later. */
-        if (s->type != SERVICE_DBUS || !s->bus_name)
+        if (!s->bus_name)
                 return 0;
 
-        r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
-        if (r < 0)
-                return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+        if (s->type == SERVICE_DBUS) {
+                r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
+                if (r < 0)
+                        return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
 
-        /* We always want to be ordered against dbus.socket if both are in the transaction. */
-        r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
-        if (r < 0)
-                return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+                /* We always want to be ordered against dbus.socket if both are in the transaction. */
+                r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, true, UNIT_DEPENDENCY_FILE);
+                if (r < 0)
+                        return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m");
+        }
 
         r = unit_watch_bus_name(UNIT(s), s->bus_name);
         if (r == -EEXIST)
@@ -1677,7 +1681,7 @@ static int service_spawn(
                 return -ENOMEM;
 
         /* System D-Bus needs nss-systemd disabled, so that we don't deadlock */
-        SET_FLAG(exec_params.flags, EXEC_NSS_BYPASS_BUS,
+        SET_FLAG(exec_params.flags, EXEC_NSS_DYNAMIC_BYPASS,
                  MANAGER_IS_SYSTEM(UNIT(s)->manager) && unit_has_name(UNIT(s), SPECIAL_DBUS_SERVICE));
 
         strv_free_and_replace(exec_params.environment, final_env);
@@ -4320,6 +4324,60 @@ static int service_get_timeout(Unit *u, usec_t *timeout) {
         return 1;
 }
 
+static bool pick_up_pid_from_bus_name(Service *s) {
+        assert(s);
+
+        /* If the service is running but we have no main PID yet, get it from the owner of the D-Bus name */
+
+        return !pid_is_valid(s->main_pid) &&
+                IN_SET(s->state,
+                       SERVICE_START,
+                       SERVICE_START_POST,
+                       SERVICE_RUNNING,
+                       SERVICE_RELOAD);
+}
+
+static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
+        const sd_bus_error *e;
+        Unit *u = userdata;
+        uint32_t pid;
+        Service *s;
+        int r;
+
+        assert(reply);
+        assert(u);
+
+        s = SERVICE(u);
+        s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
+
+        if (!s->bus_name || !pick_up_pid_from_bus_name(s))
+                return 1;
+
+        e = sd_bus_message_get_error(reply);
+        if (e) {
+                r = sd_bus_error_get_errno(e);
+                log_warning_errno(r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r));
+                return 1;
+        }
+
+        r = sd_bus_message_read(reply, "u", &pid);
+        if (r < 0) {
+                bus_log_parse_error(r);
+                return 1;
+        }
+
+        if (!pid_is_valid(pid)) {
+                log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "GetConnectionUnixProcessID() returned invalid PID");
+                return 1;
+        }
+
+        log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, (pid_t) pid);
+
+        service_set_main_pid(s, pid);
+        unit_watch_pid(UNIT(s), pid, false);
+        return 1;
+}
+
 static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
 
         Service *s = SERVICE(u);
@@ -4350,28 +4408,25 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
                 else if (s->state == SERVICE_START && new_owner)
                         service_enter_start_post(s);
 
-        } else if (new_owner &&
-                   s->main_pid <= 0 &&
-                   IN_SET(s->state,
-                          SERVICE_START,
-                          SERVICE_START_POST,
-                          SERVICE_RUNNING,
-                          SERVICE_RELOAD)) {
-
-                _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
-                pid_t pid;
+        } else if (new_owner && pick_up_pid_from_bus_name(s)) {
 
                 /* Try to acquire PID from bus service */
 
-                r = sd_bus_get_name_creds(u->manager->api_bus, s->bus_name, SD_BUS_CREDS_PID, &creds);
-                if (r >= 0)
-                        r = sd_bus_creds_get_pid(creds, &pid);
-                if (r >= 0) {
-                        log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, pid);
+                s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);
 
-                        service_set_main_pid(s, pid);
-                        unit_watch_pid(UNIT(s), pid, false);
-                }
+                r = sd_bus_call_method_async(
+                                u->manager->api_bus,
+                                &s->bus_name_pid_lookup_slot,
+                                "org.freedesktop.DBus",
+                                "/org/freedesktop/DBus",
+                                "org.freedesktop.DBus",
+                                "GetConnectionUnixProcessID",
+                                bus_name_pid_lookup_callback,
+                                s,
+                                "s",
+                                s->bus_name);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to request owner PID of service name, ignoring: %m");
         }
 }
 

src/core/service.h

@@ -195,6 +195,8 @@ struct Service {
         NotifyAccess notify_access;
         NotifyState notify_state;
 
+        sd_bus_slot *bus_name_pid_lookup_slot;
+
         sd_event_source *exec_fd_event_source;
 
         ServiceFDStore *fd_store;