diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index e8a2ce394a1..1391e9642e7 100644 --- a/docs/ENVIRONMENT.md +++ b/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 diff --git a/src/core/execute.c b/src/core/execute.c index 72f3534908a..c731dec0bd6 100644 --- a/src/core/execute.c +++ b/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; diff --git a/src/core/execute.h b/src/core/execute.h index 4aff50b4424..43070ce1117 100644 --- a/src/core/execute.h +++ b/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 */ diff --git a/src/core/service.c b/src/core/service.c index cb5233a5cf1..785038e0085 100644 --- a/src/core/service.c +++ b/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"); } } diff --git a/src/core/service.h b/src/core/service.h index 778551d8441..4116e40d8f3 100644 --- a/src/core/service.h +++ b/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;