[gdm/wip/iainl/user-switching-logind-tty] gdm-session-worker: Ask logind for the login screen



commit 8221bd47dd5a108b82438c9ba36d652aee362bc9
Author: Iain Lane <iainl gnome org>
Date:   Tue Sep 24 17:11:47 2019 +0100

    gdm-session-worker: Ask logind for the login screen
    
    When a session ends, its "session worker" is closed. Since
    3e8220921bb608afd06ed677104fd2244b901a28 (3.33.4), we uninitialise PAM
    when this happens. As part of this procedure, we jump back to the login
    screen, if the screen being killed is not itself the login screen.
    
    This has broken fast user switching. It goes like this - this
    explanation is a bit complicated, bear with us:
    
    We want to jump back to the login screen when a normal user session
    ends, so that people can log in again. We do not want to do this when a
    login screen itself ends. When session workers start up, they query for
    the *currently active VT* and save this in `login_vt`. Then later on, we
    check if our session ID is the same as `login_vt`, and jump to
    `login_vt` if they are different - this means that it was a user session
    not a login session. Querying the currently active VT is fine for the
    first greeter, but when initiating a user switch it's wrong as this
    gives the user VT.
    
    GDM greeters are killed once they have spawned a session. They are
    associated with a logind session, and therefore a PAM session. There are
    some actions performed when unregistering PAM sessions, including the
    previously mentioned VT jump. Before
    3e8220921bb608afd06ed677104fd2244b901a28 we only uninitialised PAM when
    the session itself exited so the bug was masked, but now (since this
    commit), if the login screen's *worker* exits first - as happens in the
    normal case when GDM kills it - we also do this uninitialisation. Since
    we falsely recorded the login screen as the first user's VT, this means
    that checking `login_vt != session_vt` returns `TRUE` and we jump back
    to the previous user's session immediately after logging into the new
    session: fast user switching is broken.
    
    What we need to do instead is query logind to determine the actual VT in
    use for the login screen, rather than assuming it is the one which
    started the session. We only know the VT *after* we have started the PAM
    session, though, so we need to move the location where we retrieve the
    VT to just after PAM initialisation. There's a place where we
    temporarily set the `PAM_TTY` PAM variable which happens before we open
    PAM too - keep this as the current VT. This is reset after we've opened
    the PAM session, so it is OK for it to be temporarily wrong.
    
    Closes #515

 daemon/gdm-session-worker.c | 69 +++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 31 deletions(-)
---
diff --git a/daemon/gdm-session-worker.c b/daemon/gdm-session-worker.c
index 0e854158..6ca2e826 100644
--- a/daemon/gdm-session-worker.c
+++ b/daemon/gdm-session-worker.c
@@ -55,6 +55,8 @@
 #include <systemd/sd-journal.h>
 #endif
 
+#include <systemd/sd-login.h>
+
 #ifdef HAVE_SELINUX
 #include <selinux/selinux.h>
 #endif /* HAVE_SELINUX */
@@ -1107,30 +1109,46 @@ _get_xauth_for_pam (const char *x11_authority_file)
 }
 #endif
 
-static gboolean
-ensure_login_vt (GdmSessionWorker *worker)
+static unsigned short
+get_current_vt ()
 {
         int fd;
         struct vt_stat vt_state = { 0 };
-        gboolean got_login_vt = FALSE;
 
         fd = open ("/dev/tty0", O_RDWR | O_NOCTTY);
 
         if (fd < 0) {
                 g_debug ("GdmSessionWorker: couldn't open VT master: %m");
-                return FALSE;
+                return 0;
         }
 
         if (ioctl (fd, VT_GETSTATE, &vt_state) < 0) {
                 g_debug ("GdmSessionWorker: couldn't get current VT: %m");
-                goto out;
+                return 0;
         }
 
-        worker->priv->login_vt = vt_state.v_active;
-        got_login_vt = TRUE;
-out:
-        close (fd);
-        return got_login_vt;
+        g_debug ("GdmSessionWorker: current VT is %hu", vt_state.v_active);
+        return vt_state.v_active;
+}
+
+static unsigned int
+get_login_vt (GdmSessionWorker *worker)
+{
+        g_autofree char *login_session_id = NULL;
+        unsigned int login_window_vt = 0;
+        int sd_ret;
+
+        gdm_get_login_window_session_id (worker->priv->display_seat_id, &login_session_id);
+
+        sd_ret = sd_session_get_vt (login_session_id, &login_window_vt);
+
+        if (sd_ret == 0 && login_window_vt != 0) {
+                g_debug ("GdmSessionWorker: Login window is on %u", login_window_vt);
+                return login_window_vt;
+        } else {
+                g_critical ("GdmSessionWorker: Couldn't get login window VT: %m");
+                return 0;
+        }
 }
 
 static gboolean
@@ -1146,6 +1164,7 @@ gdm_session_worker_initialize_pam (GdmSessionWorker   *worker,
                                    const char         *seat_id,
                                    GError            **error)
 {
+        unsigned short         current_vt;
         struct pam_conv        pam_conversation;
         int                    error_code;
         char tty_string[256];
@@ -1229,8 +1248,9 @@ gdm_session_worker_initialize_pam (GdmSessionWorker   *worker,
 
         /* Temporarily set PAM_TTY with the currently active VT (login screen) 
            PAM_TTY will be reset with the users VT right before the user session is opened */
-        if (ensure_login_vt (worker)) {
-                g_snprintf (tty_string, 256, "/dev/tty%d", worker->priv->login_vt);
+        current_vt = get_current_vt ();
+        if (current_vt != 0) {
+                g_snprintf (tty_string, 256, "/dev/tty%hu", current_vt);
                 pam_set_item (worker->priv->pam_handle, PAM_TTY, tty_string);
         }
         if (!display_is_local)
@@ -2265,36 +2285,21 @@ fail:
 static gboolean
 set_xdg_vtnr_to_current_vt (GdmSessionWorker *worker)
 {
-        int fd;
         char vt_string[256];
-        struct vt_stat vt_state = { 0 };
+        unsigned short current_vt;
 
-        fd = open ("/dev/tty0", O_RDWR | O_NOCTTY);
+        current_vt = get_current_vt ();
 
-        if (fd < 0) {
-                g_debug ("GdmSessionWorker: couldn't open VT master: %m");
+        if (current_vt == 0)
                 return FALSE;
-        }
-
-        if (ioctl (fd, VT_GETSTATE, &vt_state) < 0) {
-                g_debug ("GdmSessionWorker: couldn't get current VT: %m");
-                goto fail;
-        }
 
-        close (fd);
-        fd = -1;
-
-        g_snprintf (vt_string, sizeof (vt_string), "%d", vt_state.v_active);
+        g_snprintf (vt_string, sizeof (vt_string), "%hu", current_vt);
 
         gdm_session_worker_set_environment_variable (worker,
                                                      "XDG_VTNR",
                                                      vt_string);
 
         return TRUE;
-
-fail:
-        close (fd);
-        return FALSE;
 }
 
 static gboolean
@@ -2420,6 +2425,8 @@ gdm_session_worker_open_session (GdmSessionWorker  *worker,
                 goto out;
         }
 
+        worker->priv->login_vt = get_login_vt (worker);
+
         g_debug ("GdmSessionWorker: state SESSION_OPENED");
         gdm_session_worker_set_state (worker, GDM_SESSION_WORKER_STATE_SESSION_OPENED);
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]