From 4e4918c7d6dea3f03f3ae49f2d5721beb03936f2 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Tue, 19 Jul 2016 16:54:43 -0400 Subject: [PATCH 1/5] xdmcp-display-factory: notify remote display when session ended gnome-shell and the session dbus daemon don't automatically exit when gnome-session does. They, instead, wait for the display to exit or regenerate. If the display is remote, that won't happen until the keep alive timeout. This commit changes GDM to explicitly notify the remote display when the session is over, so that it can regenerate immediately. --- daemon/gdm-xdmcp-display-factory.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/daemon/gdm-xdmcp-display-factory.c b/daemon/gdm-xdmcp-display-factory.c index d01f7ff..4132dd1 100644 --- a/daemon/gdm-xdmcp-display-factory.c +++ b/daemon/gdm-xdmcp-display-factory.c @@ -183,61 +183,64 @@ struct GdmXdmcpDisplayFactoryPrivate /* configuration */ guint port; gboolean use_multicast; char *multicast_address; gboolean honor_indirect; char *willing_script; guint max_displays_per_host; guint max_displays; guint max_pending_displays; guint max_wait; guint max_wait_indirect; }; enum { PROP_0, PROP_PORT, PROP_USE_MULTICAST, PROP_MULTICAST_ADDRESS, PROP_HONOR_INDIRECT, PROP_WILLING_SCRIPT, PROP_MAX_DISPLAYS_PER_HOST, PROP_MAX_DISPLAYS, PROP_MAX_PENDING_DISPLAYS, PROP_MAX_WAIT, PROP_MAX_WAIT_INDIRECT, }; static void gdm_xdmcp_display_factory_class_init (GdmXdmcpDisplayFactoryClass *klass); static void gdm_xdmcp_display_factory_init (GdmXdmcpDisplayFactory *manager); static void gdm_xdmcp_display_factory_finalize (GObject *object); - +static void gdm_xdmcp_send_alive (GdmXdmcpDisplayFactory *factory, + GdmAddress *address, + CARD16 dspnum, + CARD32 sessid); static gpointer xdmcp_display_factory_object = NULL; G_DEFINE_TYPE (GdmXdmcpDisplayFactory, gdm_xdmcp_display_factory, GDM_TYPE_DISPLAY_FACTORY) /* Theory of operation: * * Process idles waiting for UDP packets on port 177. * Incoming packets are decoded and checked against tcp_wrapper. * * A typical session looks like this: * * Display sends Query/BroadcastQuery to Manager. * * Manager selects an appropriate authentication scheme from the * display's list of supported ones and sends Willing/Unwilling. * * Assuming the display accepts the auth. scheme it sends back a * Request. * * If the manager accepts to service the display (i.e. loadavg is low) * it sends back an Accept containing a unique SessionID. The * SessionID is stored in an accept queue by the Manager. Should the * manager refuse to start a session a Decline is sent to the display. * * The display returns a Manage request containing the supplied * SessionID. The manager will then start a session on the display. In * case the SessionID is not on the accept queue the manager returns * Refuse. If the manager fails to open the display for connections * Failed is returned. * @@ -2026,68 +2029,78 @@ on_hostname_selected (GdmXdmcpChooserDisplay *display, g_warning ("Unable to get address: %s", gai_strerror (gaierr)); g_free (xdmcp_port); return; } g_free (xdmcp_port); /* just take the first one */ ai = ai_list; if (ai != NULL) { char *ip; ic->chosen_address = gdm_address_new_from_sockaddr (ai->ai_addr, ai->ai_addrlen); ip = NULL; gdm_address_get_numeric_info (ic->chosen_address, &ip, NULL); g_debug ("GdmXdmcpDisplayFactory: hostname resolves to %s", ip ? ip : "(null)"); g_free (ip); } freeaddrinfo (ai_list); } static void on_display_status_changed (GdmDisplay *display, GParamSpec *arg1, GdmXdmcpDisplayFactory *factory) { int status; GdmDisplayStore *store; + GdmAddress *address; + gint32 session_number; + int display_number; store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); status = gdm_display_get_status (display); g_debug ("GdmXdmcpDisplayFactory: xdmcp display status changed: %d", status); switch (status) { case GDM_DISPLAY_FINISHED: + g_object_get (display, + "remote-address", &address, + "x11-display-number", &display_number, + "session-number", &session_number, + NULL); + gdm_xdmcp_send_alive (factory, address, display_number, session_number); + gdm_display_store_remove (store, display); break; case GDM_DISPLAY_FAILED: gdm_display_store_remove (store, display); break; case GDM_DISPLAY_UNMANAGED: break; case GDM_DISPLAY_PREPARED: break; case GDM_DISPLAY_MANAGED: break; default: g_assert_not_reached (); break; } } static GdmDisplay * gdm_xdmcp_display_create (GdmXdmcpDisplayFactory *factory, const char *hostname, GdmAddress *address, int displaynum) { GdmDisplay *display; GdmDisplayStore *store; gboolean use_chooser; g_debug ("GdmXdmcpDisplayFactory: Creating xdmcp display for %s:%d", hostname ? hostname : "(null)", displaynum); -- 2.9.3 From 7b9f559b6b5c8217840e3441b4eb5a29080a19c4 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 20 Jul 2016 11:43:45 -0400 Subject: [PATCH 2/5] worker: kill process group when session exits Send a hangup signal to the session pg when it exits, so things have a chance to get cleaned up. --- daemon/gdm-session-worker.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/daemon/gdm-session-worker.c b/daemon/gdm-session-worker.c index f37b92a..ffb728f 100644 --- a/daemon/gdm-session-worker.c +++ b/daemon/gdm-session-worker.c @@ -1603,60 +1603,62 @@ run_script (GdmSessionWorker *worker, worker->priv->username, worker->priv->x11_display_name, worker->priv->display_is_local? NULL : worker->priv->hostname, worker->priv->x11_authority_file); } static void session_worker_child_watch (GPid pid, int status, GdmSessionWorker *worker) { g_debug ("GdmSessionWorker: child (pid:%d) done (%s:%d)", (int) pid, WIFEXITED (status) ? "status" : WIFSIGNALED (status) ? "signal" : "unknown", WIFEXITED (status) ? WEXITSTATUS (status) : WIFSIGNALED (status) ? WTERMSIG (status) : -1); #ifdef WITH_CONSOLE_KIT close_ck_session (worker); #endif gdm_session_worker_uninitialize_pam (worker, PAM_SUCCESS); gdm_dbus_worker_emit_session_exited (GDM_DBUS_WORKER (worker), worker->priv->service, status); + killpg (pid, SIGHUP); + worker->priv->child_pid = -1; run_script (worker, GDMCONFDIR "/PostSession"); } static void gdm_session_worker_watch_child (GdmSessionWorker *worker) { g_debug ("GdmSession worker: watching pid %d", worker->priv->child_pid); worker->priv->child_watch_id = g_child_watch_add (worker->priv->child_pid, (GChildWatchFunc)session_worker_child_watch, worker); } static gboolean _is_loggable_file (const char* filename) { struct stat file_info; if (g_lstat (filename, &file_info) < 0) { return FALSE; } return S_ISREG (file_info.st_mode) && g_access (filename, R_OK | W_OK) == 0; } static void rotate_logs (const char *path, guint n_copies) -- 2.9.3 From ede6833989a691b0ee4a27834c572814564fc541 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 12 Dec 2016 10:35:32 -0500 Subject: [PATCH 3/5] display: port GdmDisplay to xcb Xlib will kill the process if it notices the display connection has gone away. This is suboptimal for the main gdm process! This commit ports the Xlib code to xcb, so it won't have the above fragility. https://bugzilla.gnome.org/show_bug.cgi?id=776059 --- configure.ac | 1 + daemon/gdm-slave.c | 312 ++++++++++++++++++++++++----------------------------- 2 files changed, 142 insertions(+), 171 deletions(-) diff --git a/configure.ac b/configure.ac index a131535..f9ad18a 100644 --- a/configure.ac +++ b/configure.ac @@ -57,60 +57,61 @@ AC_SUBST(GETTEXT_PACKAGE) AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [gettext package]) dnl --------------------------------------------------------------------------- dnl - Dependencies dnl --------------------------------------------------------------------------- GLIB_REQUIRED_VERSION=2.36.0 GTK_REQUIRED_VERSION=2.91.1 LIBCANBERRA_GTK_REQUIRED_VERSION=0.4 ACCOUNTS_SERVICE_REQUIRED_VERSION=0.6.12 EXTRA_COMPILE_WARNINGS(yes) PKG_CHECK_MODULES(GTHREAD, gthread-2.0) AC_SUBST(GTHREAD_CFLAGS) AC_SUBST(GTHREAD_LIBS) PKG_CHECK_MODULES(COMMON, gobject-2.0 >= $GLIB_REQUIRED_VERSION gio-2.0 >= $GLIB_REQUIRED_VERSION gio-unix-2.0 >= $GLIB_REQUIRED_VERSION ) AC_SUBST(COMMON_CFLAGS) AC_SUBST(COMMON_LIBS) PKG_CHECK_MODULES(DAEMON, gobject-2.0 >= $GLIB_REQUIRED_VERSION gio-2.0 >= $GLIB_REQUIRED_VERSION gio-unix-2.0 >= $GLIB_REQUIRED_VERSION accountsservice >= $ACCOUNTS_SERVICE_REQUIRED_VERSION + xcb ) AC_SUBST(DAEMON_CFLAGS) AC_SUBST(DAEMON_LIBS) GLIB_GSETTINGS PKG_CHECK_MODULES(XLIB, x11 xau xrandr, , [AC_PATH_XTRA if test "x$no_x" = xyes; then AC_MSG_ERROR("no (requires X development libraries)") else XLIB_LIBS="$X_PRE_LIBS $X_LIBS -lXau -lX11 -lXext -lXrandr $X_EXTRA_LIBS" XLIB_CFLAGS=$X_CFLAGS fi]) AC_SUBST(XLIB_CFLAGS) AC_SUBST(XLIB_LIBS) PKG_CHECK_MODULES(GTK, gtk+-3.0 >= $GTK_REQUIRED_VERSION ) AC_SUBST(GTK_CFLAGS) AC_SUBST(GTK_LIBS) PKG_CHECK_MODULES(CANBERRA_GTK, libcanberra-gtk3 >= $LIBCANBERRA_GTK_REQUIRED_VERSION ) AC_SUBST(CANBERRA_GTK_CFLAGS) AC_SUBST(CANBERRA_GTK_LIBS) AC_ARG_WITH(selinux, diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index b4bbcfd..c0a53fe 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -10,398 +10,358 @@ * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * */ #include "config.h" #include #include #include #include #include #include #include #include #include #include #include #include #include #include #include -#include /* for Display */ -#include /* for XA_PIXMAP */ +#include + #include /* for watch cursor */ #include #include #ifdef WITH_SYSTEMD #include #endif #include "gdm-common.h" #include "gdm-xerrors.h" #include "gdm-slave.h" #include "gdm-display.h" #include "gdm-display-glue.h" #include "gdm-server.h" #define GDM_SLAVE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GDM_TYPE_SLAVE, GdmSlavePrivate)) struct GdmSlavePrivate { GPid pid; guint output_watch_id; guint error_watch_id; - Display *server_display; + xcb_connection_t *xcb_connection; + int xcb_screen_number; char *session_id; GdmDisplay *display; /* cached display values */ char *display_name; int display_number; char *display_hostname; gboolean display_is_local; char *display_seat_id; char *display_x11_authority_file; char *windowpath; GBytes *display_x11_cookie; gboolean display_is_initial; }; enum { PROP_0, PROP_SESSION_ID, PROP_DISPLAY, PROP_DISPLAY_NAME, PROP_DISPLAY_NUMBER, PROP_DISPLAY_HOSTNAME, PROP_DISPLAY_IS_LOCAL, PROP_DISPLAY_SEAT_ID, PROP_DISPLAY_X11_AUTHORITY_FILE, PROP_DISPLAY_IS_INITIAL, }; enum { STARTED, STOPPED, LAST_SIGNAL }; static guint signals [LAST_SIGNAL] = { 0, }; static void gdm_slave_class_init (GdmSlaveClass *klass); static void gdm_slave_init (GdmSlave *slave); static void gdm_slave_finalize (GObject *object); G_DEFINE_ABSTRACT_TYPE (GdmSlave, gdm_slave, G_TYPE_OBJECT) #define CURSOR_WATCH XC_watch GQuark gdm_slave_error_quark (void) { static GQuark ret = 0; if (ret == 0) { ret = g_quark_from_static_string ("gdm-slave-error-quark"); } return ret; } -static XRRScreenResources * -get_screen_resources (Display *dpy) +static xcb_screen_t * +get_screen (xcb_connection_t *connection, + int screen_number) { - int major = 0, minor = 0; + xcb_screen_t *screen = NULL; + xcb_screen_iterator_t iter; - if (!XRRQueryVersion(dpy, &major, &minor)) { - return NULL; + iter = xcb_setup_roots_iterator (xcb_get_setup (connection)); + while (iter.rem) { + if (screen_number == 0) + screen = iter.data; + screen_number--; + xcb_screen_next (&iter); } - if (major > 1) { - return NULL; + if (screen != NULL) { + return screen; } - if (minor >= 3) { - return XRRGetScreenResourcesCurrent (dpy, - DefaultRootWindow (dpy)); + return NULL; +} + +static xcb_window_t +get_root_window (xcb_connection_t *connection, + int screen_number) +{ + xcb_screen_t *screen = NULL; + + screen = get_screen (connection, screen_number); + + if (screen != NULL) { + return screen->root; } - return XRRGetScreenResources (dpy, DefaultRootWindow (dpy)); + return XCB_WINDOW_NONE; } static void determine_initial_cursor_position (GdmSlave *slave, int *x, int *y) { - XRRScreenResources *resources; - RROutput primary_output; - int i; - - /* If this function fails for whatever reason, - * put the pointer in the lower right corner of the screen. - */ - *x = .9 * DisplayWidth (slave->priv->server_display, - DefaultScreen (slave->priv->server_display)); - *y = .9 * DisplayHeight (slave->priv->server_display, - DefaultScreen (slave->priv->server_display)); - - gdm_error_trap_push (); - resources = get_screen_resources (slave->priv->server_display); - primary_output = XRRGetOutputPrimary (slave->priv->server_display, - DefaultRootWindow (slave->priv->server_display)); - gdm_error_trap_pop (); - - if (resources == NULL) { - return; - } - - for (i = 0; i < resources->noutput; i++) { - XRROutputInfo *output_info; - - if (primary_output == None) { - primary_output = resources->outputs[0]; - } - - if (resources->outputs[i] != primary_output) { - continue; - } - - output_info = XRRGetOutputInfo (slave->priv->server_display, - resources, - resources->outputs[i]); - - if (output_info->connection != RR_Disconnected && - output_info->crtc != 0) { - XRRCrtcInfo *crtc_info; - - crtc_info = XRRGetCrtcInfo (slave->priv->server_display, - resources, - output_info->crtc); - /* position it sort of in the lower right - */ - *x = crtc_info->x + .9 * crtc_info->width; - *y = crtc_info->y + .9 * crtc_info->height; - XRRFreeCrtcInfo (crtc_info); - } - - XRRFreeOutputInfo (output_info); - break; - } - - XRRFreeScreenResources (resources); + *x = 50; + *y = 50; } void gdm_slave_set_initial_cursor_position (GdmSlave *slave) { - if (slave->priv->server_display != NULL) { + if (slave->priv->xcb_connection != NULL) { int x, y; + xcb_window_t root_window = XCB_WINDOW_NONE; determine_initial_cursor_position (slave, &x, &y); - XWarpPointer(slave->priv->server_display, - None, - DefaultRootWindow (slave->priv->server_display), - 0, 0, - 0, 0, - x, y); + + root_window = get_root_window (slave->priv->xcb_connection, + slave->priv->xcb_screen_number); + + if (root_window != XCB_WINDOW_NONE) { + xcb_warp_pointer (slave->priv->xcb_connection, + XCB_WINDOW_NONE, + root_window, + 0, 0, + 0, 0, + x, y); + } } } static void -gdm_slave_setup_xhost_auth (XHostAddress *host_entries, XServerInterpretedAddress *si_entries) +gdm_slave_setup_xhost_auth (XHostAddress *host_entries) { - si_entries[0].type = "localuser"; - si_entries[0].typelength = strlen ("localuser"); - si_entries[1].type = "localuser"; - si_entries[1].typelength = strlen ("localuser"); - si_entries[2].type = "localuser"; - si_entries[2].typelength = strlen ("localuser"); - - si_entries[0].value = "root"; - si_entries[0].valuelength = strlen ("root"); - si_entries[1].value = GDM_USERNAME; - si_entries[1].valuelength = strlen (GDM_USERNAME); - si_entries[2].value = "gnome-initial-setup"; - si_entries[2].valuelength = strlen ("gnome-initial-setup"); - host_entries[0].family = FamilyServerInterpreted; - host_entries[0].address = (char *) &si_entries[0]; - host_entries[0].length = sizeof (XServerInterpretedAddress); + host_entries[0].address = "localuser\0root"; + host_entries[0].length = sizeof ("localuser\0root"); host_entries[1].family = FamilyServerInterpreted; - host_entries[1].address = (char *) &si_entries[1]; - host_entries[1].length = sizeof (XServerInterpretedAddress); + host_entries[1].address = "localuser\0" GDM_USERNAME; + host_entries[1].length = sizeof ("localuser\0" GDM_USERNAME); host_entries[2].family = FamilyServerInterpreted; - host_entries[2].address = (char *) &si_entries[2]; - host_entries[2].length = sizeof (XServerInterpretedAddress); + host_entries[2].address = "localuser\0gnome-initial-setup"; + host_entries[2].length = sizeof ("localuser\0gnome-initial-setup"); } static void gdm_slave_set_windowpath (GdmSlave *slave) { /* setting WINDOWPATH for clients */ - Atom prop; - Atom actualtype; - int actualformat; - unsigned long nitems; - unsigned long bytes_after; - unsigned char *buf; + xcb_intern_atom_cookie_t atom_cookie; + xcb_intern_atom_reply_t *atom_reply = NULL; + xcb_get_property_cookie_t get_property_cookie; + xcb_get_property_reply_t *get_property_reply = NULL; + xcb_window_t root_window = XCB_WINDOW_NONE; const char *windowpath; char *newwindowpath; - unsigned long num; + uint32_t num; char nums[10]; int numn; - prop = XInternAtom (slave->priv->server_display, "XFree86_VT", False); - if (prop == None) { + atom_cookie = xcb_intern_atom (slave->priv->xcb_connection, 0, strlen("XFree86_VT"), "XFree86_VT"); + atom_reply = xcb_intern_atom_reply (slave->priv->xcb_connection, atom_cookie, NULL); + + if (atom_reply == NULL) { g_debug ("no XFree86_VT atom\n"); - return; + + goto out; + } + + root_window = get_root_window (slave->priv->xcb_connection, + slave->priv->xcb_screen_number); + + if (root_window == XCB_WINDOW_NONE) { + g_debug ("couldn't find root window\n"); + goto out; } - if (XGetWindowProperty (slave->priv->server_display, - DefaultRootWindow (slave->priv->server_display), prop, 0, 1, - False, AnyPropertyType, &actualtype, &actualformat, - &nitems, &bytes_after, &buf)) { + + get_property_cookie = xcb_get_property (slave->priv->xcb_connection, + FALSE, + root_window, + atom_reply->atom, + XCB_ATOM_INTEGER, + 0, + 1); + + get_property_reply = xcb_get_property_reply (slave->priv->xcb_connection, get_property_cookie, NULL); + + if (get_property_reply == NULL) { g_debug ("no XFree86_VT property\n"); - return; - } - - if (nitems != 1) { - g_debug ("%lu items in XFree86_VT property!\n", nitems); - XFree (buf); - return; - } - - switch (actualtype) { - case XA_CARDINAL: - case XA_INTEGER: - case XA_WINDOW: - switch (actualformat) { - case 8: - num = (*(uint8_t *)(void *)buf); - break; - case 16: - num = (*(uint16_t *)(void *)buf); - break; - case 32: - num = (*(long *)(void *)buf); - break; - default: - g_debug ("format %d in XFree86_VT property!\n", actualformat); - XFree (buf); - return; - } - break; - default: - g_debug ("type %lx in XFree86_VT property!\n", actualtype); - XFree (buf); - return; + goto out; } - XFree (buf); + + num = ((uint32_t *) xcb_get_property_value (get_property_reply))[0]; windowpath = getenv ("WINDOWPATH"); - numn = snprintf (nums, sizeof (nums), "%lu", num); + + numn = snprintf (nums, sizeof (nums), "%u", num); if (!windowpath) { newwindowpath = malloc (numn + 1); sprintf (newwindowpath, "%s", nums); } else { newwindowpath = malloc (strlen (windowpath) + 1 + numn + 1); sprintf (newwindowpath, "%s:%s", windowpath, nums); } - slave->priv->windowpath = newwindowpath; - g_setenv ("WINDOWPATH", newwindowpath, TRUE); +out: + g_clear_pointer (&atom_reply, free); + g_clear_pointer (&get_property_reply, free); } gboolean gdm_slave_connect_to_x11_display (GdmSlave *slave) { + xcb_auth_info_t *auth_info = NULL; gboolean ret; ret = FALSE; /* We keep our own (windowless) connection (dsp) open to avoid the * X server resetting due to lack of active connections. */ g_debug ("GdmSlave: Server is ready - opening display %s", slave->priv->display_name); /* Give slave access to the display independent of current hostname */ if (slave->priv->display_x11_cookie != NULL) { - XSetAuthorization ("MIT-MAGIC-COOKIE-1", - strlen ("MIT-MAGIC-COOKIE-1"), - (gpointer) - g_bytes_get_data (slave->priv->display_x11_cookie, NULL), - g_bytes_get_size (slave->priv->display_x11_cookie)); + auth_info = g_alloca (sizeof (xcb_auth_info_t)); + + auth_info->namelen = strlen ("MIT-MAGIC-COOKIE-1"); + auth_info->name = "MIT-MAGIC-COOKIE-1"; + auth_info->datalen = g_bytes_get_size (slave->priv->display_x11_cookie); + auth_info->data = (gpointer) g_bytes_get_data (slave->priv->display_x11_cookie, NULL); } - slave->priv->server_display = XOpenDisplay (slave->priv->display_name); + slave->priv->xcb_connection = xcb_connect_to_display_with_auth_info (slave->priv->display_name, + auth_info, + &slave->priv->xcb_screen_number); - if (slave->priv->server_display == NULL) { + if (xcb_connection_has_error (slave->priv->xcb_connection)) { + g_clear_pointer (&slave->priv->xcb_connection, xcb_disconnect); g_warning ("Unable to connect to display %s", slave->priv->display_name); ret = FALSE; } else if (slave->priv->display_is_local) { - XServerInterpretedAddress si_entries[3]; XHostAddress host_entries[3]; + xcb_void_cookie_t cookies[3]; int i; g_debug ("GdmSlave: Connected to display %s", slave->priv->display_name); ret = TRUE; /* Give programs run by the slave and greeter access to the * display independent of current hostname */ - gdm_slave_setup_xhost_auth (host_entries, si_entries); - - gdm_error_trap_push (); + gdm_slave_setup_xhost_auth (host_entries); for (i = 0; i < G_N_ELEMENTS (host_entries); i++) { - XAddHost (slave->priv->server_display, &host_entries[i]); + cookies[i] = xcb_change_hosts_checked (slave->priv->xcb_connection, + XCB_HOST_MODE_INSERT, + host_entries[i].family, + host_entries[i].length, + (uint8_t *) host_entries[i].address); } - XSync (slave->priv->server_display, False); - if (gdm_error_trap_pop ()) { - g_warning ("Failed to give slave programs access to the display. Trying to proceed."); + for (i = 0; i < G_N_ELEMENTS (cookies); i++) { + xcb_generic_error_t *xcb_error; + + xcb_error = xcb_request_check (slave->priv->xcb_connection, cookies[i]); + + if (xcb_error != NULL) { + g_debug ("Failed to give system user '%s' access to the display. Trying to proceed.", host_entries[i].address + sizeof ("localuser")); + free (xcb_error); + } else { + g_debug ("Gave system user '%s' access to the display.", host_entries[i].address + sizeof ("localuser")); + } } gdm_slave_set_windowpath (slave); } else { g_debug ("GdmSlave: Connected to display %s", slave->priv->display_name); ret = TRUE; } if (ret) { g_signal_emit (slave, signals [STARTED], 0); } return ret; } static gboolean gdm_slave_real_start (GdmSlave *slave) { gboolean res; GError *error; const char *x11_cookie; gsize x11_cookie_size; g_debug ("GdmSlave: Starting slave"); /* cache some values up front */ error = NULL; res = gdm_display_is_local (slave->priv->display, &slave->priv->display_is_local, &error); if (! res) { g_warning ("Failed to get value: %s", error->message); @@ -495,104 +455,114 @@ gdm_slave_start (GdmSlave *slave) g_object_ref (slave); ret = GDM_SLAVE_GET_CLASS (slave)->start (slave); g_object_unref (slave); return ret; } gboolean gdm_slave_stop (GdmSlave *slave) { gboolean ret; g_return_val_if_fail (GDM_IS_SLAVE (slave), FALSE); g_debug ("GdmSlave: stopping slave"); g_object_ref (slave); ret = GDM_SLAVE_GET_CLASS (slave)->stop (slave); g_signal_emit (slave, signals [STOPPED], 0); g_object_unref (slave); return ret; } gboolean gdm_slave_add_user_authorization (GdmSlave *slave, const char *username, char **filenamep) { - XServerInterpretedAddress si_entries[3]; XHostAddress host_entries[3]; + xcb_void_cookie_t cookies[3]; int i; gboolean res; GError *error; char *filename; filename = NULL; if (filenamep != NULL) { *filenamep = NULL; } g_debug ("GdmSlave: Requesting user authorization"); error = NULL; res = gdm_display_add_user_authorization (slave->priv->display, username, &filename, &error); if (! res) { g_warning ("Failed to add user authorization: %s", error->message); g_error_free (error); } else { g_debug ("GdmSlave: Got user authorization: %s", filename); } if (filenamep != NULL) { *filenamep = g_strdup (filename); } g_free (filename); /* Remove access for the programs run by slave and greeter now that the * user session is starting. */ - gdm_slave_setup_xhost_auth (host_entries, si_entries); - gdm_error_trap_push (); + gdm_slave_setup_xhost_auth (host_entries); for (i = 0; i < G_N_ELEMENTS (host_entries); i++) { - XRemoveHost (slave->priv->server_display, &host_entries[i]); + cookies[i] = xcb_change_hosts_checked (slave->priv->xcb_connection, + XCB_HOST_MODE_DELETE, + host_entries[i].family, + host_entries[i].length, + (uint8_t *) host_entries[i].address); } - XSync (slave->priv->server_display, False); - if (gdm_error_trap_pop ()) { - g_warning ("Failed to remove slave program access to the display. Trying to proceed."); + + for (i = 0; i < G_N_ELEMENTS (cookies); i++) { + xcb_generic_error_t *xcb_error; + + xcb_error = xcb_request_check (slave->priv->xcb_connection, cookies[i]); + + if (xcb_error != NULL) { + g_warning ("Failed to remove greeter program access to the display. Trying to proceed."); + free (xcb_error); + } } return res; } static char * gdm_slave_parse_enriched_login (GdmSlave *slave, const char *username) { char **argv; int username_len; GPtrArray *env; GError *error; gboolean res; char *parsed_username; char *command; char *std_output; char *std_error; parsed_username = NULL; if (username == NULL || username[0] == '\0') { return NULL; } /* A script may be used to generate the automatic/timed login name based on the display/host by ending the name with the pipe symbol '|'. */ username_len = strlen (username); -- 2.9.3 From 89b3bc7ba5fbfa0948a02644cbe031961435e23d Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 19 Dec 2016 10:56:27 -0500 Subject: [PATCH 4/5] slave: close display connection when done with it --- daemon/gdm-slave.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index c0a53fe..cd26965 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -411,60 +411,61 @@ gdm_slave_real_start (GdmSlave *slave) if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } error = NULL; res = gdm_display_get_seat_id (slave->priv->display, &slave->priv->display_seat_id, &error); if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } error = NULL; res = gdm_display_is_initial (slave->priv->display, &slave->priv->display_is_initial, &error); if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } return TRUE; } static gboolean gdm_slave_real_stop (GdmSlave *slave) { g_debug ("GdmSlave: Stopping slave"); + g_clear_pointer (&slave->priv->xcb_connection, xcb_disconnect); g_clear_object (&slave->priv->display); return TRUE; } gboolean gdm_slave_start (GdmSlave *slave) { gboolean ret; g_return_val_if_fail (GDM_IS_SLAVE (slave), FALSE); g_debug ("GdmSlave: starting slave"); g_object_ref (slave); ret = GDM_SLAVE_GET_CLASS (slave)->start (slave); g_object_unref (slave); return ret; } gboolean gdm_slave_stop (GdmSlave *slave) { gboolean ret; g_return_val_if_fail (GDM_IS_SLAVE (slave), FALSE); g_debug ("GdmSlave: stopping slave"); -- 2.9.3 From 122433b95dd3e099b300bce97ce1a5c99d7e2ccd Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 20 Jul 2016 17:23:05 -0400 Subject: [PATCH 5/5] slave: kill off clients from display when finished When we're done with the display we need to kill off any clients that are lingering and close our own connection to the display. This is so, for instance, processes from the session don't stick around on a -noreset Xvnc server (or something) https://bugzilla.gnome.org/show_bug.cgi?id=776059 --- daemon/gdm-slave.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c index cd26965..d965d4c 100644 --- a/daemon/gdm-slave.c +++ b/daemon/gdm-slave.c @@ -411,64 +411,90 @@ gdm_slave_real_start (GdmSlave *slave) if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } error = NULL; res = gdm_display_get_seat_id (slave->priv->display, &slave->priv->display_seat_id, &error); if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } error = NULL; res = gdm_display_is_initial (slave->priv->display, &slave->priv->display_is_initial, &error); if (! res) { g_warning ("Failed to get value: %s", error->message); g_error_free (error); return FALSE; } return TRUE; } static gboolean gdm_slave_real_stop (GdmSlave *slave) { g_debug ("GdmSlave: Stopping slave"); - g_clear_pointer (&slave->priv->xcb_connection, xcb_disconnect); - g_clear_object (&slave->priv->display); + if (slave->priv->xcb_connection) { + const xcb_setup_t *setup; - return TRUE; + /* These 3 bits are reserved/unused by the X protocol */ + guint32 unused_bits = 0b11100000000000000000000000000000; + XID highest_client, client; + guint32 client_increment; + + setup = xcb_get_setup (slave->priv->xcb_connection); + + /* resource_id_mask is the bits given to each client for + * addressing resources */ + highest_client = (XID) ~unused_bits & ~setup->resource_id_mask; + client_increment = setup->resource_id_mask + 1; + + /* Kill every client but ourselves, then close our own connection + */ + for (client = 0; + client <= highest_client; + client += client_increment) { + + if (client != setup->resource_id_base) + xcb_kill_client (slave->priv->xcb_connection, client); + } + xcb_flush (slave->priv->xcb_connection); + g_clear_pointer (&slave->priv->xcb_connection, xcb_disconnect); + } + g_clear_object (&slave->priv->display); + + return TRUE; } gboolean gdm_slave_start (GdmSlave *slave) { gboolean ret; g_return_val_if_fail (GDM_IS_SLAVE (slave), FALSE); g_debug ("GdmSlave: starting slave"); g_object_ref (slave); ret = GDM_SLAVE_GET_CLASS (slave)->start (slave); g_object_unref (slave); return ret; } gboolean gdm_slave_stop (GdmSlave *slave) { gboolean ret; g_return_val_if_fail (GDM_IS_SLAVE (slave), FALSE); g_debug ("GdmSlave: stopping slave"); g_object_ref (slave); ret = GDM_SLAVE_GET_CLASS (slave)->stop (slave); -- 2.9.3