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

[Djalal Harouni]

To get the state of the session, the session_get_state() is used.
This function will check if the session->scope_job is set then it will
automatically return SESSION_OPENING. This is buggy in the context of
session closing:

At logout or D-Bus TerminateSession() fifo_fd is removed:

   =>  session_get_state() == SESSION_CLOSING   (correct)

Then we have session_stop() which calls session_stop_scope(), this
will queue the scope job to be removed and set the session->scope_job

   =>  session_get_state() == SESSION_OPENING   (incorrect)

After the scope job is removed the state will be again correct

   =>  session_get_state() == SESSION_CLOSING   (correct)

To fix this and make sure that the state will always be SESSION_CLOSING
we add a flag that is used to differentiate between SESSION_OPENING and
SESSION_CLOSING.

The 'scope_opening' flag will be set to true only during real session
opening in session_start_scope(), and it will be set to false just after
the session fifo fd was successfully created, which means that during
session closing it will be false.

And update session_get_state() to check if the 'scope_opening' is true
before returning the SESSION_OPENING, if it is not then SESSION_CLOSING
will be returned which is the correct behaviour.
---
 src/login/logind-session-dbus.c | 3 +++
 src/login/logind-session.c      | 4 +++-
 src/login/logind-session.h      | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 54db864..099ade6 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -670,6 +670,9 @@ int session_send_create_reply(Session *s, sd_bus_error *error, bool opening) {
         if (fifo_fd < 0)
                 return fifo_fd;
 
+        /* Clean this up as soon as we have the fifo_fd */
+        s->scope_opening = false;
+
         /* Update the session state file before we notify the client
          * about the result. */
         session_save(s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index bec59c0..848e8a1 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -496,6 +496,8 @@ static int session_start_scope(Session *s) {
 
                         free(s->scope_job);
                         s->scope_job = job;
+                        /* session scope is being created */
+                        s->scope_opening = true;
                 }
         }
 
@@ -880,7 +882,7 @@ void session_add_to_gc_queue(Session *s) {
 SessionState session_get_state(Session *s) {
         assert(s);
 
-        if (s->scope_job)
+        if (s->scope_opening)
                 return SESSION_OPENING;
 
         if (s->fifo_fd < 0)
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index ebe3902..205491a 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -110,6 +110,7 @@ struct Session {
 
         bool in_gc_queue:1;
         bool started:1;
+        bool scope_opening:1; /* session scope is being created */
 
         sd_bus_message *create_message;
 
-- 
1.8.3.1