xwayland: Do not keep the cursor's pixmap aroundHEADmaster

Currently, Xwayland creates a pixmap backed by shared memory buffer as
soon as an X11 cursor is realized, which is destroyed when the cursor is
eventually unrealized.

If an X11 client is leaking cursors, Xwayland will be creating new
pixmaps continuously, which will eventually cause an error once the
limit is reached, and get Xwayland killed.

However, we do not need the shared memory buffer to stay around, we
already have the buffer retention mechanism which will take care of
keeping the buffer around until the Wayland compositor is done with it,
so we could just create and destroy the pixmap as needed when setting
the cursor.

That would not fix the leak in the X11 application, yet that would
mitigate the risk of Xwayland being killed by reaching the shared memory
limits, until the client itself reaches the limit of X11 resources.

v2: Don't increase the pixmap refcnt to destroy it just after (Michel)

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Suggested-by: Michel Dänzer <michel@daenzer.net>
See-also: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1773
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1754>

1 files changed, 10 insertions, 24 deletions

diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
index 9ee427063..fa77f6daa 100644
--- a/hw/xwayland/xwayland-cursor.c
+++ b/hw/xwayland/xwayland-cursor.c
@@ -43,8 +43,6 @@
 
 #define DELAYED_X_CURSOR_TIMEOUT 5 /* ms */
 
-static DevPrivateKeyRec xwl_cursor_private_key;
-
 static void
 expand_source_and_mask(CursorPtr cursor, CARD32 *data)
 {
@@ -80,29 +78,15 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
 static Bool
 xwl_realize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
 {
-    PixmapPtr pixmap;
-
-    pixmap = xwl_shm_create_pixmap(screen, cursor->bits->width,
-                                   cursor->bits->height, 32,
-                                   CREATE_PIXMAP_USAGE_BACKING_PIXMAP);
-    dixSetPrivate(&cursor->devPrivates, &xwl_cursor_private_key, pixmap);
-
     return TRUE;
 }
 
 static Bool
 xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
 {
-    PixmapPtr pixmap;
     struct xwl_screen *xwl_screen;
     struct xwl_seat *xwl_seat;
 
-    pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
-    if (!pixmap)
-        return TRUE;
-
-    dixSetPrivate(&cursor->devPrivates, &xwl_cursor_private_key, NULL);
-
     /* When called from FreeCursor(), device is always NULL */
     xwl_screen = xwl_screen_get(screen);
     xorg_list_for_each_entry(xwl_seat, &xwl_screen->seat_list, link) {
@@ -110,7 +94,7 @@ xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
             xwl_seat->x_cursor = NULL;
     }
 
-    return xwl_shm_destroy_pixmap(pixmap);
+    return TRUE;
 }
 
 static void
@@ -173,8 +157,9 @@ xwl_cursor_attach_pixmap(struct xwl_seat *xwl_seat,
     xwl_cursor->frame_cb = wl_surface_frame(xwl_cursor->surface);
     wl_callback_add_listener(xwl_cursor->frame_cb, &frame_listener, xwl_cursor);
 
-    /* Hold a reference on the pixmap until it's released by the compositor */
-    pixmap->refcnt++;
+    /* The pixmap will be destroyed in xwl_cursor_buffer_release_callback()
+     * once the compositor is done with it.
+     */
     xwl_pixmap_set_buffer_release_cb(pixmap,
                                      xwl_cursor_buffer_release_callback,
                                      pixmap);
@@ -220,7 +205,9 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
     }
 
     cursor = xwl_seat->x_cursor;
-    pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
+    pixmap = xwl_shm_create_pixmap(xwl_screen->screen, cursor->bits->width,
+                                   cursor->bits->height, 32,
+                                   CREATE_PIXMAP_USAGE_BACKING_PIXMAP);
     if (!pixmap)
         return;
 
@@ -263,7 +250,9 @@ xwl_tablet_tool_set_cursor(struct xwl_tablet_tool *xwl_tablet_tool)
     }
 
     cursor = xwl_seat->x_cursor;
-    pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
+    pixmap = xwl_shm_create_pixmap(xwl_screen->screen, cursor->bits->width,
+                                   cursor->bits->height, 32,
+                                   CREATE_PIXMAP_USAGE_BACKING_PIXMAP);
     if (!pixmap)
         return;
 
@@ -443,9 +432,6 @@ static miPointerScreenFuncRec xwl_pointer_screen_funcs = {
 Bool
 xwl_screen_init_cursor(struct xwl_screen *xwl_screen)
 {
-    if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR, 0))
-        return FALSE;
-
     return miPointerInitialize(xwl_screen->screen,
                                &xwl_pointer_sprite_funcs,
                                &xwl_pointer_screen_funcs, TRUE);