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")

diff --git a/src/xwayland-unmanaged.c b/src/xwayland-unmanaged.c
index 8f8faf7939bd2e882cc234a761b233811443d131..8c0067bdf46bc9fb21e1c32cd797d221969f0894 100644 (file)
--- 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 c07a18b76a98b99e1020d42e99c11f0e2595b896..ec9fe4a2ad22d02d32421a9cd8c2e80c882e474f 100644 (file)
--- 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;