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

[Alexandre Julliard]

Module: wine
Branch: master
Commit: a7ec328fa8020d48e77c6e39630edc4c320aebc8
URL:    https://gitlab.winehq.org/wine/wine/-/commit/a7ec328fa8020d48e77c6e39630edc4...

Author: Alexandros Frantzis 
Date:   Mon May 29 18:13:16 2023 +0300

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

---

dlls/winewayland.drv/wayland_surface.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c
index 9ee0a41f02c..d679492b94a 100644
--- 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);