[systemd-devel] [PATCH 4/7] logind: close races on user state at logout

[Djalal Harouni]

To get the state of the user, the user_get_state() is used.
This function will check if the user->slice_job or the user->service_job
are set then it will automatically return USER_OPENING. This is buggy
in the context of user closing:

At logout or D-Bus TerminateUser() calls user_stop()
user_stop() => session_stop() on sessions => session_stop_scope()

  =>  user_get_state() == USER_CLOSING or USER_ACTIVE or USER_ONLINE
                                                        (incorrect)

  (depends on session closing execution and if we have finished the
  session scope jobs or not, if all the scopes are being queued then
  their session->scope_job will be set which means all sessions are in
  the OPENING_STATE, so user state will be USER_ONLINE).

  The previous patch improves session_get_state() logic, and if scopes
  are being queued during logout then sessions are in SESSION_CLOSING
  state which makes user_get_state() return USER_CLOSING.

So at user_stop()
user_stop_slice() queues the slice and sets user->slice_job
user_stop_service() queues the service and sets user->service_job

  =>  user_get_state() == USER_OPENING  (incorrect)

After the slice and service jobs are removed the state will be again
correct.

  =>  user_get_state() == USER_CLOSING  (correct)

To fix this and make sure that the state will always be USER_CLOSING
we add a flag that is used to differentiate between USER_OPENING and
USER_CLOSING when 'slice_job' and 'service_job' are set.

The 'slice_opening' flag for 'user->slice_job' and 'service_opening'
for 'user->service_job' are set to true only during real user opening,
later if the service and slice were successfully created their
corresponding 'opening' flag will be set to false, which means that
during user closing they are already false.

Update user_get_state() to check if 'slice_opening' or
'service_opening' are set, if so return USER_OPENING.
---
 src/login/logind-dbus.c | 2 ++
 src/login/logind-user.c | 6 +++++-
 src/login/logind-user.h | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 0560707..24482fd 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2017,11 +2017,13 @@ int match_job_removed(sd_bus *bus, sd_bus_message *message, void *userdata, sd_b
                         /* Clean this up after session_jobs_reply() */
                         free(user->service_job);
                         user->service_job = NULL;
+                        user->service_opening = false;
                 }
 
                 if (streq_ptr(path, user->slice_job)) {
                         free(user->slice_job);
                         user->slice_job = NULL;
+                        user->slice_opening = false;
                 }
 
                 user_save(user);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index bdb6915..8183721 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -352,6 +352,8 @@ static int user_start_slice(User *u) {
 
                         free(u->slice_job);
                         u->slice_job = job;
+                        /* User slice is being created */
+                        u->slice_opening = true;
                 }
         }
 
@@ -385,6 +387,8 @@ static int user_start_service(User *u) {
 
                         free(u->service_job);
                         u->service_job = job;
+                        /* User service is being created */
+                        u->service_opening = true;
                 }
         }
 
@@ -637,7 +641,7 @@ UserState user_get_state(User *u) {
 
         assert(u);
 
-        if (u->slice_job || u->service_job)
+        if (u->slice_opening || u->service_opening)
                 return USER_OPENING;
 
         LIST_FOREACH(sessions_by_user, i, u->sessions) {
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 0062880..ac43361 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -61,6 +61,8 @@ struct User {
 
         bool in_gc_queue:1;
         bool started:1;
+        bool service_opening:1; /* User service is being created */
+        bool slice_opening:1; /* User slice is being created */
 
         LIST_HEAD(Session, sessions);
         LIST_FIELDS(User, gc_queue);
-- 
1.8.3.1