From dcdbaaa04012541ad2813cf83559d91d52f208b9 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Thu, 10 Dec 2020 15:14:20 -0500 Subject: session-worker: Don't switch back VTs until session is fully exited There's a race condition on shutdown where the session worker is switching VTs back to the initial VT at the same time as the session exit is being processed. This means that manager may try to start a login screen (because of the VT switch) when autologin is enabled when there shouldn't be a login screen. This commit makes sure both the PostSession script, and session-exited signal emission are complete before initiating the VT switch back to the initial VT. https://gitlab.gnome.org/GNOME/gdm/-/issues/660 --- daemon/gdm-session-worker.c | 93 ++++++++++++++++++++++++++++++++++------- daemon/meson.build | 7 ++++ daemon/org.freedesktop.DBus.xml | 12 ++++++ 3 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 daemon/org.freedesktop.DBus.xml diff --git a/daemon/gdm-session-worker.c b/daemon/gdm-session-worker.c index d2cfde51..36c3e5df 100644 --- a/daemon/gdm-session-worker.c +++ b/daemon/gdm-session-worker.c @@ -66,6 +66,7 @@ #include "gdm-pam-extensions.h" #endif +#include "gdm-dbus-glue.h" #include "gdm-session-worker.h" #include "gdm-session-glue.h" #include "gdm-session.h" @@ -1051,18 +1052,6 @@ gdm_session_worker_uninitialize_pam (GdmSessionWorker *worker, gdm_session_worker_stop_auditor (worker); - /* If user-display-server is not enabled the login_vt is always - * identical to the session_vt. So in that case we never need to - * do a VT switch. */ -#ifdef ENABLE_USER_DISPLAY_SERVER - if (g_strcmp0 (worker->priv->display_seat_id, "seat0") == 0) { - /* Switch to the login VT if we are not the login screen. */ - if (worker->priv->session_vt != GDM_INITIAL_VT) { - jump_to_vt (worker, GDM_INITIAL_VT); - } - } -#endif - worker->priv->session_vt = 0; g_debug ("GdmSessionWorker: state NONE"); @@ -1775,6 +1764,53 @@ run_script (GdmSessionWorker *worker, worker->priv->x11_authority_file); } +static void +wait_until_dbus_signal_emission_to_manager_finishes (GdmSessionWorker *worker) +{ + g_autoptr (GdmDBusPeer) peer_proxy = NULL; + g_autoptr (GError) error = NULL; + gboolean pinged; + + peer_proxy = gdm_dbus_peer_proxy_new_sync (worker->priv->connection, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, + NULL, + "/org/freedesktop/DBus", + NULL, + &error); + + if (peer_proxy == NULL) { + g_debug ("GdmSessionWorker: could not create peer proxy to daemon: %s", + error->message); + return; + } + + pinged = gdm_dbus_peer_call_ping_sync (peer_proxy, NULL, &error); + + if (!pinged) { + g_debug ("GdmSessionWorker: could not ping daemon: %s", + error->message); + return; + } +} + +static void +jump_back_to_initial_vt (GdmSessionWorker *worker) +{ + if (worker->priv->session_vt == 0) + return; + + if (worker->priv->session_vt == GDM_INITIAL_VT) + return; + + if (g_strcmp0 (worker->priv->display_seat_id, "seat0") != 0) + return; + +#ifdef ENABLE_USER_DISPLAY_SERVER + jump_to_vt (worker, GDM_INITIAL_VT); + worker->priv->session_vt = 0; +#endif +} + static void session_worker_child_watch (GPid pid, int status, @@ -1789,18 +1825,40 @@ session_worker_child_watch (GPid pid, : WIFSIGNALED (status) ? WTERMSIG (status) : -1); - gdm_session_worker_uninitialize_pam (worker, PAM_SUCCESS); + worker->priv->child_pid = -1; + worker->priv->child_watch_id = 0; + run_script (worker, GDMCONFDIR "/PostSession"); + gdm_dbus_worker_emit_session_exited (GDM_DBUS_WORKER (worker), worker->priv->service, status); killpg (pid, SIGHUP); - worker->priv->child_pid = -1; - worker->priv->child_watch_id = 0; - run_script (worker, GDMCONFDIR "/PostSession"); + /* FIXME: It's important to give the manager an opportunity to process the + * session-exited emission above before switching VTs. + * + * This is because switching VTs makes the manager try to put a login screen + * up on VT 1, but it may actually want to try to auto login again in response + * to session-exited. + * + * This function just does a manager roundtrip over the bus to make sure the + * signal has been dispatched before jumping. + * + * Ultimately, we may want to improve the manager<->worker interface. + * + * See: + * + * https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/123 + * + * for some ideas and more discussion. + * + */ + wait_until_dbus_signal_emission_to_manager_finishes (worker); + + jump_back_to_initial_vt (worker); } static void @@ -2424,6 +2482,7 @@ gdm_session_worker_open_session (GdmSessionWorker *worker, out: if (error_code != PAM_SUCCESS) { gdm_session_worker_uninitialize_pam (worker, error_code); + worker->priv->session_vt = 0; return FALSE; } @@ -3549,6 +3608,8 @@ gdm_session_worker_finalize (GObject *object) gdm_session_worker_uninitialize_pam (worker, PAM_SUCCESS); } + jump_back_to_initial_vt (worker); + g_object_unref (worker->priv->user_settings); g_free (worker->priv->service); g_free (worker->priv->x11_display_name); diff --git a/daemon/meson.build b/daemon/meson.build index 72c7c3c1..2e61b644 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -1,4 +1,10 @@ # D-Bus interfaces +dbus_gen = gnome.gdbus_codegen('gdm-dbus-glue', + 'org.freedesktop.DBus.xml', + namespace: 'GdmDBus', + interface_prefix: 'org.freedesktop.DBus', + autocleanup: 'all', +) display_dbus_gen = gnome.gdbus_codegen('gdm-display-glue', 'gdm-display.xml', namespace: 'GdmDBus', @@ -87,6 +93,7 @@ gdm_session_worker_src = [ 'gdm-session-worker-job.c', 'gdm-session-worker-common.c', 'gdm-dbus-util.c', + dbus_gen, session_dbus_gen, session_worker_dbus_gen, gdm_session_enums, diff --git a/daemon/org.freedesktop.DBus.xml b/daemon/org.freedesktop.DBus.xml new file mode 100644 index 00000000..5e0814bd --- /dev/null +++ b/daemon/org.freedesktop.DBus.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + -- cgit v1.2.1