From 33f835ec7c74629d4a6982c3b75630af2cd5d65b Mon Sep 17 00:00:00 2001

From: Simon McVittie <simon.mcvittie@collabora.co.uk>

Date: Wed, 17 Jun 2015 16:45:49 +0100

Subject: [PATCH] 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>

(cherry picked from commit 71161305f191d1fe1242ccca47657f9ab51caad4)

---

 src/login/logind-user.c | 22 +++++++++++++++++-----

 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/login/logind-user.c b/src/login/logind-user.c

index f4c4490e8f..21acf2afe3 100644

--- a/src/login/logind-user.c

+++ b/src/login/logind-user.c

@@ -104,7 +104,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;

@@ -112,9 +112,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;

@@ -257,6 +254,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;

@@ -452,6 +458,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)

@@ -703,7 +715,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) {