From 71161305f191d1fe1242ccca47657f9ab51caad4 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 17 Jun 2015 16:45:49 +0100 Subject: 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 --- src/login/logind-user.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 6720899def..bfbdc9b1b8 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -105,7 +105,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; @@ -113,9 +113,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; @@ -258,6 +255,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; @@ -453,6 +459,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) @@ -704,7 +716,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) { -- cgit v1.2.3-54-g00ecf