diff options
author | Simon McVittie <simon.mcvittie@collabora.co.uk> | 2015-06-17 16:45:49 +0100 |
---|---|---|
committer | Sven Eden <yamakuzure@gmx.net> | 2017-03-14 10:03:48 +0100 |
commit | 31e57d94c86e0a4f332f30033b10e402fec4aac7 (patch) | |
tree | 8b896253f123793c08b45f77b11c912ba9245876 /src/login | |
parent | 7b3cb911e7ddb15cc8f7b1bf0a19c5619b58e23c (diff) |
logind: save /run/systemd/users/UID before starting user@.service
Previously, this had a race condition during a user's first login.
Some component calls CreateSession (most likely by a PAM service
other than 'systemd-user' running pam_systemd), with the following
results:
- logind:
* create the user's XDG_RUNTIME_DIR
* tell pid 1 to create user-UID.slice
* tell pid 1 to start user@UID.service
Then these two processes race:
- logind:
* save information including XDG_RUNTIME_DIR to /run/systemd/users/UID
- the subprocess of pid 1 responsible for user@service:
* start a 'systemd-user' PAM session, which reads XDG_RUNTIME_DIR
and puts it in the environment
* run systemd --user, which requires XDG_RUNTIME_DIR in the
environment
If logind wins the race, which usually happens, everything is fine;
but if the subprocesses of pid 1 win the race, which can happen
under load, then systemd --user exits unsuccessfully.
To avoid this race, we have to write out /run/systemd/users/UID
even though the service has not "officially" started yet;
previously this did an early-return without saving anything.
Record its state as OPENING in this case.
Bug: https://github.com/systemd/systemd/issues/232
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Diffstat (limited to 'src/login')
-rw-r--r-- | src/login/logind-user.c | 22 |
1 files changed, 17 insertions, 5 deletions
diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 738e69cce..21d726812 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -106,7 +106,7 @@ void user_free(User *u) { free(u); } -int user_save(User *u) { +static int user_save_internal(User *u) { _cleanup_free_ char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; int r; @@ -114,9 +114,6 @@ int user_save(User *u) { assert(u); assert(u->state_file); - if (!u->started) - return 0; - r = mkdir_safe_label("/run/systemd/users", 0755, 0, 0); if (r < 0) goto finish; @@ -259,6 +256,15 @@ finish: return r; } +int user_save(User *u) { + assert(u); + + if (!u->started) + return 0; + + return user_save_internal (u); +} + int user_load(User *u) { _cleanup_free_ char *display = NULL, *realtime = NULL, *monotonic = NULL; Session *s = NULL; @@ -458,6 +464,12 @@ int user_start(User *u) { if (r < 0) return r; + /* Save the user data so far, because pam_systemd will read the + * XDG_RUNTIME_DIR out of it while starting up systemd --user. + * We need to do user_save_internal() because we have not + * "officially" started yet. */ + user_save_internal(u); + /* Spawn user systemd */ r = user_start_service(u); if (r < 0) @@ -709,7 +721,7 @@ UserState user_get_state(User *u) { if (u->stopping) return USER_CLOSING; - if (u->slice_job || u->service_job) + if (!u->started || u->slice_job || u->service_job) return USER_OPENING; if (u->sessions) { |