Commit 9b6d9b24 authored by Ray Strode's avatar Ray Strode
Browse files

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.

#660
parent b41c993f
......@@ -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);
......
# 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,
......
<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node>
<interface name="org.freedesktop.DBus.Peer">
<method name="GetMachineId">
<arg direction="out" type="s"/>
</method>
<method name="Ping">
</method>
</interface>
</node>
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment