Blob Blame History Raw
From de229615d80fd7c8a38ab5d5d7b30aa98f43721d Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 14 Sep 2020 16:20:09 -0400
Subject: [PATCH 1/3] manager: Don't leak session objects

The first is from create_user_session_for display.  Most callers don't
check the return value, so it should just be void.

The user data associated with the session also isn't unlinked from the
display when the display is finishing up, preventing the display and
session object from getting freed.

This commit makes both changes.
---
 daemon/gdm-manager.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c
index bff602a07..738671679 100644
--- a/daemon/gdm-manager.c
+++ b/daemon/gdm-manager.c
@@ -94,63 +94,63 @@ struct GdmManagerPrivate
 #ifdef  WITH_PLYMOUTH
         guint                     plymouth_is_running : 1;
 #endif
         guint                     did_automatic_login : 1;
 };
 
 enum {
         PROP_0,
         PROP_XDMCP_ENABLED,
         PROP_SHOW_LOCAL_GREETER
 };
 
 enum {
         DISPLAY_ADDED,
         DISPLAY_REMOVED,
         LAST_SIGNAL
 };
 
 typedef enum {
         SESSION_RECORD_LOGIN,
         SESSION_RECORD_LOGOUT,
         SESSION_RECORD_FAILED,
 } SessionRecord;
 
 static guint signals [LAST_SIGNAL] = { 0, };
 
 static void     gdm_manager_class_init  (GdmManagerClass *klass);
 static void     gdm_manager_init        (GdmManager      *manager);
 static void     gdm_manager_dispose     (GObject         *object);
 
-static GdmSession *create_user_session_for_display (GdmManager *manager,
-                                                    GdmDisplay *display,
-                                                    uid_t       allowed_user);
+static void     create_user_session_for_display (GdmManager *manager,
+                                                 GdmDisplay *display,
+                                                 uid_t       allowed_user);
 static void     start_user_session (GdmManager                *manager,
                                     StartUserSessionOperation *operation);
 static void     clean_user_session (GdmSession *session);
 
 static gpointer manager_object = NULL;
 
 static void manager_interface_init (GdmDBusManagerIface *interface);
 
 G_DEFINE_TYPE_WITH_CODE (GdmManager,
                          gdm_manager,
                          GDM_DBUS_TYPE_MANAGER_SKELETON,
                          G_IMPLEMENT_INTERFACE (GDM_DBUS_TYPE_MANAGER,
                                                 manager_interface_init));
 
 #ifdef WITH_PLYMOUTH
 static gboolean
 plymouth_is_running (void)
 {
         int      status;
         gboolean res;
         GError  *error;
 
         error = NULL;
         res = g_spawn_command_line_sync ("/bin/plymouth --ping",
                                          NULL, NULL, &status, &error);
         if (! res) {
                 g_debug ("Could not ping plymouth: %s", error->message);
                 g_error_free (error);
                 return FALSE;
         }
@@ -1343,61 +1343,62 @@ get_automatic_login_details (GdmManager *manager,
         return enabled;
 }
 
 static const char *
 get_username_for_greeter_display (GdmManager *manager,
                                   GdmDisplay *display)
 {
         gboolean doing_initial_setup = FALSE;
 
         g_object_get (G_OBJECT (display),
                       "doing-initial-setup", &doing_initial_setup,
                       NULL);
 
         if (doing_initial_setup) {
                 return INITIAL_SETUP_USERNAME;
         } else {
                 return GDM_USERNAME;
         }
 }
 
 static void
 set_up_automatic_login_session (GdmManager *manager,
                                 GdmDisplay *display)
 {
         GdmSession *session;
         char       *display_session_type = NULL;
 
         /* 0 is root user; since the daemon talks to the session object
          * directly, itself, for automatic login
          */
-        session = create_user_session_for_display (manager, display, 0);
+        create_user_session_for_display (manager, display, 0);
+        session = get_user_session_for_display (display);
 
         g_object_get (G_OBJECT (display),
                       "session-type", &display_session_type,
                       NULL);
 
         g_object_set (G_OBJECT (session),
                       "display-is-initial", FALSE,
                       NULL);
 
         g_debug ("GdmManager: Starting automatic login conversation");
         gdm_session_start_conversation (session, "gdm-autologin");
 }
 
 static void
 set_up_chooser_session (GdmManager *manager,
                         GdmDisplay *display)
 {
         const char *allowed_user;
         struct passwd *passwd_entry;
 
         allowed_user = get_username_for_greeter_display (manager, display);
 
         if (!gdm_get_pwent_for_name (allowed_user, &passwd_entry)) {
                 g_warning ("GdmManager: couldn't look up username %s",
                            allowed_user);
                 gdm_display_unmanage (display);
                 gdm_display_finish (display);
                 return;
         }
 
@@ -1549,60 +1550,62 @@ on_display_status_changed (GdmDisplay *display,
                       "session-type", &session_type,
                       NULL);
 
         status = gdm_display_get_status (display);
 
         switch (status) {
                 case GDM_DISPLAY_PREPARED:
                 case GDM_DISPLAY_MANAGED:
                         if ((display_number == -1 && status == GDM_DISPLAY_PREPARED) ||
                             (display_number != -1 && status == GDM_DISPLAY_MANAGED)) {
                                 char *session_class;
 
                                 g_object_get (display,
                                               "session-class", &session_class,
                                               NULL);
                                 if (g_strcmp0 (session_class, "greeter") == 0)
                                         set_up_session (manager, display);
                                 g_free (session_class);
                         }
                         break;
                 case GDM_DISPLAY_FAILED:
                 case GDM_DISPLAY_UNMANAGED:
                 case GDM_DISPLAY_FINISHED:
 #ifdef WITH_PLYMOUTH
                         if (quit_plymouth) {
                                 plymouth_quit_without_transition ();
                                 manager->priv->plymouth_is_running = FALSE;
                         }
 #endif
 
+                        g_object_set_data (G_OBJECT (display), "gdm-user-session", NULL);
+
                         if (display == manager->priv->automatic_login_display) {
                                 g_clear_weak_pointer (&manager->priv->automatic_login_display);
 
                                 manager->priv->did_automatic_login = TRUE;
 
 #ifdef ENABLE_WAYLAND_SUPPORT
                                 if (g_strcmp0 (session_type, "wayland") != 0 && status == GDM_DISPLAY_FAILED) {
                                         /* we're going to fall back to X11, so try to autologin again
                                          */
                                         manager->priv->did_automatic_login = FALSE;
                                 }
 #endif
                         }
                         break;
                 default:
                         break;
         }
 
 }
 
 static void
 on_display_removed (GdmDisplayStore *display_store,
                     GdmDisplay      *display,
                     GdmManager      *manager)
 {
         char    *id;
 
         gdm_display_get_id (display, &id, NULL);
         g_dbus_object_manager_server_unexport (manager->priv->object_manager, id);
         g_free (id);
@@ -2292,61 +2295,61 @@ on_session_reauthentication_started (GdmSession *session,
                                      int         pid_of_caller,
                                      const char *address,
                                      GdmManager *manager)
 {
         GDBusMethodInvocation *invocation;
         gpointer               source_tag;
 
         g_debug ("GdmManager: reauthentication started");
 
         source_tag = GINT_TO_POINTER (pid_of_caller);
 
         invocation = g_hash_table_lookup (manager->priv->open_reauthentication_requests,
                                           source_tag);
 
         if (invocation != NULL) {
                 g_hash_table_steal (manager->priv->open_reauthentication_requests,
                                     source_tag);
                 gdm_dbus_manager_complete_open_reauthentication_channel (GDM_DBUS_MANAGER (manager),
                                                                          invocation,
                                                                          address);
         }
 }
 
 static void
 clean_user_session (GdmSession *session)
 {
         g_object_set_data (G_OBJECT (session), "gdm-display", NULL);
         g_object_unref (session);
 }
 
-static GdmSession *
+static void
 create_user_session_for_display (GdmManager *manager,
                                  GdmDisplay *display,
                                  uid_t       allowed_user)
 {
         GdmSession *session;
         gboolean    display_is_local = FALSE;
         char       *display_name = NULL;
         char       *display_device = NULL;
         char       *remote_hostname = NULL;
         char       *display_auth_file = NULL;
         char       *display_seat_id = NULL;
         char       *display_id = NULL;
 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)
         char       *display_session_type = NULL;
         gboolean    greeter_is_wayland;
 #endif
 
         g_object_get (G_OBJECT (display),
                       "id", &display_id,
                       "x11-display-name", &display_name,
                       "is-local", &display_is_local,
                       "remote-hostname", &remote_hostname,
                       "x11-authority-file", &display_auth_file,
                       "seat-id", &display_seat_id,
 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)
                       "session-type", &display_session_type,
 #endif
                       NULL);
         display_device = get_display_device (manager, display);
 
@@ -2402,70 +2405,68 @@ create_user_session_for_display (GdmManager *manager,
                           "conversation-stopped",
                           G_CALLBACK (on_session_conversation_stopped),
                           manager);
         g_signal_connect (session,
                           "authentication-failed",
                           G_CALLBACK (on_session_authentication_failed),
                           manager);
         g_signal_connect (session,
                           "session-opened",
                           G_CALLBACK (on_user_session_opened),
                           manager);
         g_signal_connect (session,
                           "session-started",
                           G_CALLBACK (on_user_session_started),
                           manager);
         g_signal_connect (session,
                           "session-start-failed",
                           G_CALLBACK (on_session_start_failed),
                           manager);
         g_signal_connect (session,
                           "session-exited",
                           G_CALLBACK (on_user_session_exited),
                           manager);
         g_signal_connect (session,
                           "session-died",
                           G_CALLBACK (on_user_session_died),
                           manager);
         g_object_set_data (G_OBJECT (session), "gdm-display", display);
         g_object_set_data_full (G_OBJECT (display),
                                 "gdm-user-session",
-                                g_object_ref (session),
+                                session,
                                 (GDestroyNotify)
                                 clean_user_session);
 
 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)
         greeter_is_wayland = g_strcmp0 (display_session_type, "wayland") == 0;
         g_object_set (G_OBJECT (session), "ignore-wayland", !greeter_is_wayland, NULL);
 #endif
-
-        return session;
 }
 
 static void
 on_display_added (GdmDisplayStore *display_store,
                   const char      *id,
                   GdmManager      *manager)
 {
         GdmDisplay *display;
 
         display = gdm_display_store_lookup (display_store, id);
 
         if (display != NULL) {
                 g_dbus_object_manager_server_export (manager->priv->object_manager,
                                                      gdm_display_get_object_skeleton (display));
 
                 g_signal_connect (display, "notify::status",
                                   G_CALLBACK (on_display_status_changed),
                                   manager);
                 g_signal_emit (manager, signals[DISPLAY_ADDED], 0, id);
         }
 }
 
 GQuark
 gdm_manager_error_quark (void)
 {
         static GQuark ret = 0;
         if (ret == 0) {
                 ret = g_quark_from_static_string ("gdm_manager_error");
         }
 
-- 
2.26.2