From 1c785d74c1b70a495a406b05c347060269c327b7 Mon Sep 17 00:00:00 2001 From: John Lindgren Date: Sat, 30 Sep 2023 01:31:37 -0400 Subject: [PATCH] xwayland: do not set xsurface->data for unmanaged surface xsurface->data is presumed to be a (struct view *) if set, so it must be left NULL for an unmanaged surface. Otherwise view_from_wlr_surface() may return a (struct xwayland_unmanaged *) where a (struct view *) was expected, leading to a crash. valgrind backtrace: Invalid read of size 1 at 0x48F8FFC: wlr_scene_node_set_enabled (wlr_scene.c:903) by 0x124C9F: ssd_set_active (ssd.c:323) by 0x124C9F: ssd_set_active (ssd.c:318) by 0x124C9F: view_set_activated (view.c:215) by 0x118851: focus_change_notify (seat.c:353) by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241) by 0x48FC8F2: wlr_seat_keyboard_enter (wlr_seat_keyboard.c:298) by 0x119E60: seat_focus.lto_priv.0 (seat.c:473) by 0x1248FD: seat_focus_surface (seat.c:491) by 0x1248FD: unmanaged_handle_map (xwayland-unmanaged.c:51) by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241) by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241) by 0x490FC91: surface_commit_state (wlr_compositor.c:499) by 0x56A24F5: ffi_call_unix64 (unix64.S:104) by 0x569EF5D: ffi_call_int.lto_priv.0 (ffi64.c:673) Address 0xa0e15ff30788b68 is not stack'd, malloc'd or (recently) free'd Fixes: 4028a9482f9399e3c3587f5c6eca6c0b128c9afc ("seat: use focus_change event to update focused/active view") --- src/xwayland-unmanaged.c | 8 ++++++-- src/xwayland.c | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/xwayland-unmanaged.c b/src/xwayland-unmanaged.c index 8f8faf79..8c0067bd 100644 --- a/src/xwayland-unmanaged.c +++ b/src/xwayland-unmanaged.c @@ -144,7 +144,6 @@ unmanaged_handle_override_redirect(struct wl_listener *listener, void *data) unmanaged_handle_unmap(&unmanaged->unmap, NULL); } unmanaged_handle_destroy(&unmanaged->destroy, NULL); - xsurface->data = NULL; xwayland_view_create(server, xsurface, mapped); } @@ -185,7 +184,12 @@ xwayland_unmanaged_create(struct server *server, struct xwayland_unmanaged *unmanaged = znew(*unmanaged); unmanaged->server = server; unmanaged->xwayland_surface = xsurface; - xsurface->data = unmanaged; + /* + * xsurface->data is presumed to be a (struct view *) if set, + * so it must be left NULL for an unmanaged surface (it should + * be NULL already at this point). + */ + assert(!xsurface->data); wl_signal_add(&xsurface->events.request_configure, &unmanaged->request_configure); diff --git a/src/xwayland.c b/src/xwayland.c index c07a18b7..ec9fe4a2 100644 --- a/src/xwayland.c +++ b/src/xwayland.c @@ -221,7 +221,8 @@ handle_destroy(struct wl_listener *listener, void *data) /* * Break view <-> xsurface association. Note that the xsurface * may not actually be destroyed at this point; it may become an - * "unmanaged" surface instead. + * "unmanaged" surface instead (in that case it is important + * that xsurface->data not point to the destroyed view). */ xwayland_view->xwayland_surface->data = NULL; xwayland_view->xwayland_surface = NULL; -- 2.52.0