winewayland.drv: Ensure Wayland surface handlers don't access invalid data.

In our setup with a dedicated event dispatch thread, libwayland ensures that object proxies associated with an event handler remain valid (or NULL) while the handler is executing. However, no such guarantees are given for the proxy user data. It is thus possible for the user data to become invalid (e.g., its memory freed from a different thread) right after the event handler is entered. This is an issue for wayland_surface associated proxies since they may receive unsolicited events from the compositor at any time (e.g., xdg_surface.configure), even while we are destroying the wayland_surface. To avoid the problem, we introduce a lock that protects access to xdg_surface user data and ensures that the associated wayland_surface remains valid for the duration of the handler. Co-authored-by: Rémi Bernon <rbernon@codeweavers.com>

winewayland.drv: Ensure Wayland surface handlers don't access invalid data.
a7ec328f · Alexandros Frantzis · Alexandre Julliard · 285c47d1 · a7ec328f
Commit a7ec328f authored May 29, 2023 by Alexandros Frantzis Committed by Alexandre Julliard Jun 29, 2023
Hide whitespace changes
Inline Side-by-side

Showing with 22 additions and 2 deletions

wayland_surface.c dlls/winewayland.drv/wayland_surface.c +22 -2

No files found.
--- a/dlls/winewayland.drv/wayland_surface.c
+++ b/dlls/winewayland.drv/wayland_surface.c
@@ -31,18 +31,35 @@

 WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv);

+/* Protects access to the user data of xdg_surface */
+static pthread_mutex_t xdg_data_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+static struct wayland_surface *wayland_surface_lock_xdg(struct xdg_surface *xdg_surface)
+{
+    struct wayland_surface *surface;
+
+    pthread_mutex_lock(&xdg_data_mutex);
+    surface = xdg_surface_get_user_data(xdg_surface);
+    if (surface) pthread_mutex_lock(&surface->mutex);
+    pthread_mutex_unlock(&xdg_data_mutex);
+
+    return surface;
+}
+
 static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface,
                                         uint32_t serial)
 {
-    struct wayland_surface *surface = data;
+    struct wayland_surface *surface;

    TRACE("serial=%u\n", serial);

-    pthread_mutex_lock(&surface->mutex);
+    if (!(surface = wayland_surface_lock_xdg(xdg_surface))) return;
+
    /* Handle this event only if wayland_surface is still associated with
     * the target xdg_surface. */
    if (surface->xdg_surface == xdg_surface)
        xdg_surface_ack_configure(xdg_surface, serial);
+
    pthread_mutex_unlock(&surface->mutex);
 }

@@ -92,6 +109,7 @@ err:
 */
 void wayland_surface_destroy(struct wayland_surface *surface)
 {
+    pthread_mutex_lock(&xdg_data_mutex);
    pthread_mutex_lock(&surface->mutex);

    if (surface->xdg_toplevel)
@@ -102,6 +120,7 @@ void wayland_surface_destroy(struct wayland_surface *surface)

    if (surface->xdg_surface)
    {
+        xdg_surface_set_user_data(surface->xdg_surface, NULL);
        xdg_surface_destroy(surface->xdg_surface);
        surface->xdg_surface = NULL;
    }
@@ -113,6 +132,7 @@ void wayland_surface_destroy(struct wayland_surface *surface)
    }

    pthread_mutex_unlock(&surface->mutex);
+    pthread_mutex_unlock(&xdg_data_mutex);

    wl_display_flush(process_wayland.wl_display);