From e5347af0005a68ea9804690658abd876b2186ab6 Mon Sep 17 00:00:00 2001 From: Olivier Fourdan Date: Mon, 18 Jan 2021 10:22:01 +0100 Subject: [PATCH] xwayland: Check X11 clients prior to terminate Xwayland Currently, mutter checks for the presence of X11 windows to decide whether or not Xwayland can be terminated, when Xwayland is started on demand. Unfortunately, not all X11 clients will map a window all the time, an X11 client may keep the X11 connection opened after closing all its windows. In that case, we may terminate Xwayland while there are some X11 client connected still, and terminating Xwayland will also kill those X11 clients. To avoid that issue, check the X11 clients actually connected using the XRes extension. The XRes extension provides the PID of the (local) X11 clients connected to the Xserver, so we need to match that against the actual executable names, and compare with a list of known executables that we can safely ignore, such as ibus-x11 or gsd-xsettings. We also check against our own executable name, considering that the X11 window manager is also an X11 client connected to the Xserver. Also, XRes returning the PID of local clients only is not a problem considering that Xwayland does not listen to remote connections. However, if the user spawns a client remotely on another system using ssh tunneling (ssh -X), only clients which actually map a window will be accounted for. Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/1537 Part-of: --- config.h.meson | 3 + meson.build | 4 + meson_options.txt | 6 ++ src/wayland/meta-xwayland.c | 149 +++++++++++++++++++++++++++++++++++- 4 files changed, 161 insertions(+), 1 deletion(-) diff --git a/config.h.meson b/config.h.meson index f4d6000e1..4b7a4a107 100644 --- a/config.h.meson +++ b/config.h.meson @@ -82,3 +82,6 @@ /* Whether the memfd_create function exists */ #mesondefine HAVE_MEMFD_CREATE + +/* List of executable names to ignore when terminating Xwayland */ +#mesondefine XWAYLAND_IGNORE_EXECUTABLES diff --git a/meson.build b/meson.build index 5b4be8081..2cf119796 100644 --- a/meson.build +++ b/meson.build @@ -461,6 +461,10 @@ xwayland_grab_default_access_rules = get_option('xwayland_grab_default_access_ru cdata.set_quoted('XWAYLAND_GRAB_DEFAULT_ACCESS_RULES', xwayland_grab_default_access_rules) +xwayland_ignore_executables = get_option('xwayland_ignore_executables') +cdata.set_quoted('XWAYLAND_IGNORE_EXECUTABLES', + xwayland_ignore_executables) + cdata.set_quoted('MUTTER_PLUGIN_DIR', join_paths(pkglibdir, 'plugins')) cdata.set_quoted('MUTTER_LOCALEDIR', localedir) cdata.set_quoted('MUTTER_LIBEXECDIR', libexecdir) diff --git a/meson_options.txt b/meson_options.txt index 601f0a773..a0fb61636 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -158,3 +158,9 @@ option('xwayland_initfd', value: 'auto', description: 'Whether -initfd argument is passed to Xwayland to guarantee services (e.g. gsd-xsettings) startup before applications' ) + +option('xwayland_ignore_executables', + type: 'string', + value: 'gsd-xsettings,ibus-x11,pulseaudio,Xwayland', + description: 'Comma delimited list of executable names to ignore when terminating Xwayland' +) diff --git a/src/wayland/meta-xwayland.c b/src/wayland/meta-xwayland.c index b9832a587..c26a5f539 100644 --- a/src/wayland/meta-xwayland.c +++ b/src/wayland/meta-xwayland.c @@ -41,6 +41,9 @@ #include #include #include +#include + +#include #include "backends/meta-monitor-manager-private.h" #include "backends/meta-settings-private.h" @@ -126,6 +129,146 @@ meta_xwayland_is_xwayland_surface (MetaWaylandSurface *surface) return wl_resource_get_client (surface->resource) == manager->client; } +static char * +meta_xwayland_get_exe_from_proc_entry (const char *proc_entry) +{ + g_autofree char *exepath; + char *executable; + char *p; + + exepath = g_file_read_link (proc_entry, NULL); + if (!exepath) + return NULL; + + p = strrchr (exepath, G_DIR_SEPARATOR); + if (p) + executable = g_strdup (++p); + else + executable = g_strdup (exepath); + + return executable; +} + +static char * +meta_xwayland_get_exe_from_pid (uint32_t pid) +{ + g_autofree char *proc_entry; + char *executable; + + proc_entry = g_strdup_printf ("/proc/%i/exe", pid); + executable = meta_xwayland_get_exe_from_proc_entry (proc_entry); + + return executable; +} + +static char * +meta_xwayland_get_self_exe (void) +{ + g_autofree char *proc_entry; + char *executable; + + proc_entry = g_strdup_printf ("/proc/self/exe"); + executable = meta_xwayland_get_exe_from_proc_entry (proc_entry); + + return executable; +} + +static gboolean +can_xwayland_ignore_exe (const char *executable, + const char *self) +{ + char ** ignore_executables; + gboolean ret; + + if (!g_strcmp0 (executable, self)) + return TRUE; + + ignore_executables = g_strsplit_set (XWAYLAND_IGNORE_EXECUTABLES, ",", -1); + ret = g_strv_contains ((const char * const *) ignore_executables, executable); + g_strfreev (ignore_executables); + + return ret; +} + +static uint32_t +meta_xwayland_get_client_pid (xcb_connection_t *xcb, + uint32_t client) +{ + xcb_res_client_id_spec_t spec = { 0 }; + xcb_res_query_client_ids_cookie_t cookie; + xcb_res_query_client_ids_reply_t *reply = NULL; + uint32_t pid = 0, *value; + + spec.client = client; + spec.mask = XCB_RES_CLIENT_ID_MASK_LOCAL_CLIENT_PID; + + cookie = xcb_res_query_client_ids (xcb, 1, &spec); + reply = xcb_res_query_client_ids_reply (xcb, cookie, NULL); + + if (reply == NULL) + return 0; + + xcb_res_client_id_value_iterator_t it; + for (it = xcb_res_query_client_ids_ids_iterator (reply); + it.rem; + xcb_res_client_id_value_next (&it)) + { + spec = it.data->spec; + if (spec.mask & XCB_RES_CLIENT_ID_MASK_LOCAL_CLIENT_PID) + { + value = xcb_res_client_id_value_value (it.data); + pid = *value; + break; + } + } + free (reply); + + return pid; +} + +static gboolean +can_terminate_xwayland (Display *xdisplay) +{ + xcb_connection_t *xcb = XGetXCBConnection (xdisplay); + xcb_res_query_clients_cookie_t cookie; + xcb_res_query_clients_reply_t *reply = NULL; + xcb_res_client_iterator_t it; + gboolean can_terminate; + char *self; + + cookie = xcb_res_query_clients (xcb); + reply = xcb_res_query_clients_reply (xcb, cookie, NULL); + + /* Could not get the list of X11 clients, better not terminate Xwayland */ + if (reply == NULL) + return FALSE; + + can_terminate = TRUE; + self = meta_xwayland_get_self_exe (); + for (it = xcb_res_query_clients_clients_iterator (reply); + it.rem && can_terminate; + xcb_res_client_next (&it)) + { + uint32_t pid; + char *executable; + + pid = meta_xwayland_get_client_pid (xcb, it.data->resource_base); + if (pid == 0) + { + /* Unknown PID, don't risk terminating it */ + can_terminate = FALSE; + break; + } + + executable = meta_xwayland_get_exe_from_pid (pid); + can_terminate = can_xwayland_ignore_exe (executable, self); + g_free (executable); + } + free (reply); + + return can_terminate; +} + static gboolean try_display (int display, char **filename_out, @@ -385,10 +528,14 @@ static gboolean shutdown_xwayland_cb (gpointer data) { MetaXWaylandManager *manager = data; + MetaDisplay *display = meta_get_display (); + + if (!can_terminate_xwayland (display->x11_display->xdisplay)) + return G_SOURCE_CONTINUE; meta_verbose ("Shutting down Xwayland"); manager->xserver_grace_period_id = 0; - meta_display_shutdown_x11 (meta_get_display ()); + meta_display_shutdown_x11 (display); meta_xwayland_stop_xserver (manager); return G_SOURCE_REMOVE; }