From 32646105196ed7a687f3684b69ea544dd01d2ade Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Thu, 23 Jan 2020 14:32:38 +0100 Subject: manager: Try looking up session based on PID first Unfortunately, GDM may be running multiple greeters, and each greeter is currently using the same user. So while in a lot of setups each user should only have one graphical session and also only one DBus session bus, this is not true for the gdm greeter. Lacking another solution (e.g. separate users), we need to be able to correctly lookup the session information for all greeter instances. We can do so by using sd_pid_get_session and using this information is safe if it does return something. See: #526 --- common/gdm-common.c | 30 ++++++++++++++++++++++++++---- common/gdm-common.h | 7 ++++--- daemon/gdm-manager.c | 2 +- libgdm/gdm-user-switching.c | 2 +- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/common/gdm-common.c b/common/gdm-common.c index 41bdb389..b8de7555 100644 --- a/common/gdm-common.c +++ b/common/gdm-common.c @@ -497,7 +497,7 @@ goto_login_session (GDBusConnection *connection, * since the data allocated is from libsystemd-logind, which * does not use GLib's g_malloc (). */ - if (!gdm_find_display_session_for_uid (getuid (), &our_session, &local_error)) { + if (!gdm_find_display_session (0, getuid (), &our_session, &local_error)) { g_propagate_prefixed_error (error, local_error, _("Could not identify the current session: ")); return FALSE; @@ -898,16 +898,38 @@ _systemd_session_is_active (const char *session_id) } gboolean -gdm_find_display_session_for_uid (const uid_t uid, - char **out_session_id, - GError **error) +gdm_find_display_session (GPid pid, + const uid_t uid, + char **out_session_id, + GError **error) { char *local_session_id = NULL; g_auto(GStrv) sessions = NULL; int n_sessions; + int res; g_return_val_if_fail (out_session_id != NULL, FALSE); + /* First try to look up the session using the pid. We need this + * at least for the greeter, because it currently runs multiple + * sessions under the same user. + * See also commit 2b52d8933c8ab38e7ee83318da2363d00d8c5581 + * which added an explicit dbus-run-session for all but seat0. + */ + res = sd_pid_get_session (pid, &local_session_id); + if (res >= 0) { + g_debug ("GdmCommon: Found session %s for PID %d, using", local_session_id, pid); + + *out_session_id = g_strdup (local_session_id); + free (local_session_id); + + return TRUE; + } else { + if (res != -ENODATA) + g_warning ("GdmCommon: Failed to retrieve session information for pid %d: %s", + pid, strerror (-res)); + } + g_debug ("Finding a graphical session for user %d", uid); n_sessions = sd_uid_get_sessions (uid, diff --git a/common/gdm-common.h b/common/gdm-common.h index 5c3fe137..001b70c1 100644 --- a/common/gdm-common.h +++ b/common/gdm-common.h @@ -51,9 +51,10 @@ int gdm_wait_on_and_disown_pid (int pid, int gdm_signal_pid (int pid, int signal); -gboolean gdm_find_display_session_for_uid (const uid_t uid, - char **out_session_id, - GError **error); +gboolean gdm_find_display_session (GPid pid, + const uid_t uid, + char **out_session_id, + GError **error); gboolean gdm_get_pwent_for_name (const char *name, struct passwd **pwentp); diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c index 907eca37..d8dfa843 100644 --- a/daemon/gdm-manager.c +++ b/daemon/gdm-manager.c @@ -501,7 +501,7 @@ get_display_and_details_for_bus_sender (GdmManager *self, goto out; } - ret = gdm_find_display_session_for_uid (caller_uid, &session_id, &error); + ret = gdm_find_display_session (pid, caller_uid, &session_id, &error); if (!ret) { g_debug ("GdmManager: Unable to find display session for uid %d: %s", diff --git a/libgdm/gdm-user-switching.c b/libgdm/gdm-user-switching.c index 3a33fcbb..20235fd8 100644 --- a/libgdm/gdm-user-switching.c +++ b/libgdm/gdm-user-switching.c @@ -203,7 +203,7 @@ goto_login_session (GDBusConnection *connection, /* Note that we mostly use free () here, instead of g_free () * since the data allocated is from libsystemd-logind, which * does not use GLib's g_malloc (). */ - if (!gdm_find_display_session_for_uid (getuid (), &our_session, &local_error)) { + if (!gdm_find_display_session (0, getuid (), &our_session, &local_error)) { g_propagate_prefixed_error (error, local_error, _("Could not identify the current session: ")); return FALSE; -- cgit v1.2.1 From febeb9a9295fcd47a63c8ccdb41b88387acf8aac Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Thu, 16 Apr 2020 15:17:56 +0200 Subject: session: Always use separate session bus for greeter sessions This is a workaround for the fact that we currently need to run multiple greeter sessions in multi-seat environments that use the same user. We should not be doing this in the first place. Doing this effectively prevents GNOME from using a systemd startup, which would cause relevant processes to be outside of the session scope preventing lookups of the logind session from the PID. Instead, we really should be running each of the greeter session as a separate (dynamic) user. But lacking that, this workaround should get multi-seat support up and running again for the time being. See: #526 --- NEWS | 3 +++ daemon/gdm-session.c | 26 ++++++++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index eba83b06..0ec0c08f 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +- Always use separate session bus for greeter sessions + This runs dbus-run-session, so the binary needs to be available + ============== Version 3.34.1 ============== diff --git a/daemon/gdm-session.c b/daemon/gdm-session.c index 4e303e70..ca7d98f1 100644 --- a/daemon/gdm-session.c +++ b/daemon/gdm-session.c @@ -2900,23 +2900,33 @@ gdm_session_start_session (GdmSession *self, g_free (command); } else { + /* FIXME: + * Always use a separate DBus bus for each greeter session. + * Firstly, this means that if we run multiple greeter session + * (which we really should not do, but have to currently), then + * each one will get its own DBus session bus. + * But, we also explicitly do this for seat0, because that way + * it cannot make use of systemd to run the GNOME session. This + * prevents the session lookup logic from getting confused. + * This has a similar effect as passing --builtin to gnome-session. + * + * We really should not be doing this. But the fix is to use + * separate dynamically created users and that requires some + * major refactorings. + */ if (run_launcher) { if (is_x11) { - program = g_strdup_printf (LIBEXECDIR "/gdm-x-session %s\"%s\"", + program = g_strdup_printf (LIBEXECDIR "/gdm-x-session %s\"dbus-run-session -- %s\"", register_session ? "--register-session " : "", self->selected_program); } else { - program = g_strdup_printf (LIBEXECDIR "/gdm-wayland-session %s\"%s\"", + program = g_strdup_printf (LIBEXECDIR "/gdm-wayland-session %s\"dbus-run-session -- %s\"", register_session ? "--register-session " : "", self->selected_program); } } else { - if (g_strcmp0 (self->display_seat_id, "seat0") != 0) { - program = g_strdup_printf ("dbus-run-session -- %s", - self->selected_program); - } else { - program = g_strdup (self->selected_program); - } + program = g_strdup_printf ("dbus-run-session -- %s", + self->selected_program); } } -- cgit v1.2.1