[RFC PATCH xserver] xwayland: Monitor client states to destroy callbacks

Olivier Fourdan ofourdan at redhat.com
Tue Mar 7 09:11:13 UTC 2017


Client resources can survive the client itself, in which case we
may end up in our sync callback trying to access client's data after
it's been freed/reclaimed.

Add a ClientStateCallback handler to monitor the client state changes
and clear the sync callback set up by the glamor drm code if any.

Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
---
 As discussed with Michel on irc, it's possible that widnows survive the
 client itself, meanign that we could still end up trying to access freed
 memory in our glamor sync handler.

 hw/xwayland/xwayland-glamor.c | 50 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 65c3c00..cc04a33 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -38,6 +38,9 @@
 #include <dri3.h>
 #include "drm-client-protocol.h"
 
+static DevPrivateKeyRec xwlGlamorPrivateKeyRec;
+#define xwlGlamorPrivateKey (&xwlGlamorPrivateKeyRec)
+
 struct xwl_pixmap {
     struct wl_buffer *buffer;
     struct gbm_bo *bo;
@@ -429,9 +432,39 @@ glamor_egl_dri3_fd_name_from_tex(ScreenPtr screen,
 struct xwl_auth_state {
     int fd;
     ClientPtr client;
+    struct wl_callback *callback;
 };
 
 static void
+free_xwl_auth_state(ClientPtr pClient, struct xwl_auth_state *state)
+{
+    dixSetPrivate(&(pClient)->devPrivates, xwlGlamorPrivateKey, NULL);
+
+    if (state) {
+        wl_callback_destroy(state->callback);
+        free(state);
+    }
+}
+
+static void
+xwl_auth_state_client_callback(CallbackListPtr *pcbl, void *unused, void *data)
+{
+    NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
+    ClientPtr pClient = clientinfo->client;
+    struct xwl_auth_state *state;
+
+    switch (pClient->clientState) {
+    case ClientStateGone:
+    case ClientStateRetained:
+        state = dixLookupPrivate(&(pClient)->devPrivates, xwlGlamorPrivateKey);
+        free_xwl_auth_state (pClient, state);
+        break;
+    default:
+        break;
+    }
+}
+
+static void
 sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
 {
     struct xwl_auth_state *state = data;
@@ -441,8 +474,7 @@ sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
         dri3_send_open_reply(client, state->fd);
         AttendClient(client);
     }
-    free(state);
-    wl_callback_destroy(callback);
+    free_xwl_auth_state (client, state);
 }
 
 static const struct wl_callback_listener sync_listener = {
@@ -485,8 +517,9 @@ xwl_dri3_open_client(ClientPtr client,
     }
 
     wl_drm_authenticate(xwl_screen->drm, magic);
-    callback = wl_display_sync(xwl_screen->display);
+    state->callback = wl_display_sync(xwl_screen->display);
     wl_callback_add_listener(callback, &sync_listener, state);
+    dixSetPrivate(&client->devPrivates, xwlGlamorPrivateKey, state);
 
     IgnoreClient(client);
 
@@ -568,6 +601,17 @@ xwl_glamor_init(struct xwl_screen *xwl_screen)
         return FALSE;
     }
 
+    if (!AddCallback(&ClientStateCallback, xwl_auth_state_client_callback, NULL)) {
+        ErrorF("Failed to add client state callback\n");
+        return FALSE;
+    }
+
+    if (!dixRegisterPrivateKey(xwlGlamorPrivateKey, PRIVATE_CLIENT, 0)) {
+        ErrorF("Failed to register private key\n");
+        DeleteCallback(&ClientStateCallback, xwl_auth_state_client_callback, NULL);
+        return FALSE;
+    }
+
     xwl_screen->CreateScreenResources = screen->CreateScreenResources;
     screen->CreateScreenResources = xwl_glamor_create_screen_resources;
     screen->CreatePixmap = xwl_glamor_create_pixmap;
-- 
2.9.3



More information about the xorg-devel mailing list