[PATCH v3 xserver] xwayland: Don't block in DRI2 authentication mechanism

Tiago Vignatti tiago.vignatti at intel.com
Wed Feb 6 05:09:18 PST 2013


Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
---
v2: - added AuthMagic3, for passing ClientPtr
    - fixed server internal ABI issue, by maintaining DRI2Authenticate intact
    - fixed multiple clients accesses to DRI2 authenticate, using Wayland
      callback mechanism to ensure they all go atomically

v3: - fixed AuthMagic and LegacyAuthMagic semantics when adding the new
      DRI2AuthMagic3ProcPtr type
    - unexported DRI2Authenticate
    - fixed DRI2SendAuthReply to be more strict about its args ("Bool status")
    - changed the new nonblocking dri2 authentication to process only one
      request at time

 hw/xfree86/dri2/dri2.c                 |   18 ++++++----
 hw/xfree86/dri2/dri2.h                 |   17 ++++++----
 hw/xfree86/dri2/dri2ext.c              |   25 ++++++++++----
 hw/xfree86/xwayland/xwayland-drm.c     |   58 +++++++++++++++++++++++++++-----
 hw/xfree86/xwayland/xwayland-private.h |    1 +
 hw/xfree86/xwayland/xwayland.c         |    2 ++
 hw/xfree86/xwayland/xwayland.h         |    2 +-
 7 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index b08618a..142619c 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -106,8 +106,9 @@ typedef struct _DRI2Screen {
     DRI2ScheduleSwapProcPtr ScheduleSwap;
     DRI2GetMSCProcPtr GetMSC;
     DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
-    DRI2AuthMagic2ProcPtr AuthMagic;
     DRI2AuthMagicProcPtr LegacyAuthMagic;
+    DRI2AuthMagic2ProcPtr LegacyAuthMagic2;
+    DRI2AuthMagic3ProcPtr AuthMagic;
     DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
     DRI2SwapLimitValidateProcPtr SwapLimitValidate;
 
@@ -1123,11 +1124,11 @@ DRI2AuthMagic (ScreenPtr pScreen, uint32_t magic)
 }
 
 Bool
-DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
+DRI2Authenticate(ClientPtr client, ScreenPtr pScreen, uint32_t magic)
 {
     DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
 
-    if (ds == NULL || (*ds->AuthMagic) (pScreen, magic))
+    if (ds == NULL || (*ds->AuthMagic) (client, pScreen, magic))
         return FALSE;
 
     return TRUE;
@@ -1222,12 +1223,15 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
      *
      * If a driver is built without xwayland support, we'll die before we get
      * here. If a driver is built with xwayland support, it'll support
-     * AuthMagic2, or crash; we don't care about xwayland ABI yet.
+     * AuthMagic3, or crash; we don't care about xwayland ABI yet.
      */
     if (xorgWayland) {
-        ds->AuthMagic = info->AuthMagic2;
+        ds->AuthMagic = info->AuthMagic3;
+    }
+
+    if (info->version >= 8) {
+        ds->LegacyAuthMagic2 = info->AuthMagic2;
     }
-    
     if (info->version >= 5) {
         ds->LegacyAuthMagic = info->AuthMagic;
     }
@@ -1247,7 +1251,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
          * If the driver doesn't provide an AuthMagic function
          * it relies on the old method (using libdrm) or fails
          */
-        if (!ds->LegacyAuthMagic)
+        if (!ds->LegacyAuthMagic2 && !ds->LegacyAuthMagic)
 #ifdef WITH_LIBDRM
             ds->LegacyAuthMagic = drmAuthMagic;
 #else
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index cec9634..617cbfb 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -65,6 +65,8 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw,
 typedef void (*DRI2WaitProcPtr) (WindowPtr pWin, unsigned int sequence);
 typedef int (*DRI2AuthMagicProcPtr) (int fd, uint32_t magic);
 typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, uint32_t magic);
+typedef int (*DRI2AuthMagic3ProcPtr) (ClientPtr client,
+                                      ScreenPtr pScreen, uint32_t magic);
 
 /**
  * Schedule a buffer swap
@@ -213,15 +215,14 @@ typedef struct {
     DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
     DRI2SwapLimitValidateProcPtr SwapLimitValidate;
 
-    /* added in version 8 AND in xwayland branch */
-    /* 
-     * MERGE NOTE: when merging xwayland to master, the commit adding this
-     * is unnecessary, but it's slightly different in master.
-     */
+    /* added in version 8 */
     /* AuthMagic callback which passes extra context */
     /* If this is NULL the AuthMagic callback is used */
     /* If this is non-NULL the AuthMagic callback is ignored */
     DRI2AuthMagic2ProcPtr AuthMagic2;
+
+    /* MERGE NOTE: added in xwayland branch */
+    DRI2AuthMagic3ProcPtr AuthMagic3;
 } DRI2InfoRec, *DRI2InfoPtr;
 
 extern _X_EXPORT int DRI2EventBase;
@@ -238,7 +239,11 @@ extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen,
                                   const char **driverName,
                                   const char **deviceName);
 
-extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, uint32_t magic);
+extern Bool DRI2Authenticate(ClientPtr client,
+                             ScreenPtr pScreen,
+                             uint32_t magic);
+
+extern _X_EXPORT void DRI2SendAuthReply(ClientPtr client, Bool status);
 
 extern _X_EXPORT int DRI2CreateDrawable(ClientPtr client,
                                         DrawablePtr pDraw,
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 2579a5c..246f55e 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -133,25 +133,36 @@ ProcDRI2Connect(ClientPtr client)
     return Success;
 }
 
+void
+DRI2SendAuthReply(ClientPtr client, Bool status)
+{
+    xDRI2AuthenticateReply rep;
+
+    REQUEST_SIZE_MATCH(xDRI2AuthenticateReq);
+    rep.type = X_Reply;
+    rep.sequenceNumber = client->sequence;
+    rep.length = 0;
+    rep.authenticated = status;
+
+    WriteToClient(client, sizeof(xDRI2AuthenticateReply), &rep);
+}
+
 static int
 ProcDRI2Authenticate(ClientPtr client)
 {
     REQUEST(xDRI2AuthenticateReq);
-    xDRI2AuthenticateReply rep;
     DrawablePtr pDraw;
     int status;
 
-    REQUEST_SIZE_MATCH(xDRI2AuthenticateReq);
     if (!validDrawable(client, stuff->window, DixGetAttrAccess,
                        &pDraw, &status))
         return status;
 
-    rep.type = X_Reply;
-    rep.sequenceNumber = client->sequence;
-    rep.length = 0;
-    rep.authenticated = DRI2Authenticate(pDraw->pScreen, stuff->magic);
-    WriteToClient(client, sizeof(xDRI2AuthenticateReply), &rep);
+    if (DRI2Authenticate(client, pDraw->pScreen, stuff->magic))
+        return Success;
 
+    /* BadAccess or BadDrawable */
+    DRI2SendAuthReply(client, FALSE);
     return Success;
 }
 
diff --git a/hw/xfree86/xwayland/xwayland-drm.c b/hw/xfree86/xwayland/xwayland-drm.c
index c19eb84..26a5448 100644
--- a/hw/xfree86/xwayland/xwayland-drm.c
+++ b/hw/xfree86/xwayland/xwayland-drm.c
@@ -45,6 +45,15 @@
 
 #include "xwayland.h"
 #include "xwayland-private.h"
+#include "../dri2/dri2.h"
+
+struct xwl_auth_req {
+    struct xorg_list link;
+
+    ClientPtr client;
+    struct xwl_screen *xwl_screen;
+    uint32_t magic;
+};
 
 static void
 drm_handle_device (void *data, struct wl_drm *drm, const char *device)
@@ -63,8 +72,31 @@ static void
 drm_handle_authenticated (void *data, struct wl_drm *drm)
 {
     struct xwl_screen *xwl_screen = data;
+    struct xwl_auth_req *req;
 
     xwl_screen->authenticated = 1;
+
+    /* it does one authentication transaction at a time, so if there's an
+     * element in the list, we call DRI2SendAuthReply for that client, remove
+     * the head and free the struct. If there are still elements in the list,
+     * it means that we have one or more clients waiting to be authenticated
+     * and we send out a wl_drm authenticate request for the first client in
+     * the list */
+    if (xorg_list_is_empty(&xwl_screen->authenticate_client_list))
+	return;
+
+    req = xorg_list_first_entry(&xwl_screen->authenticate_client_list,
+	                        struct xwl_auth_req, link);
+    DRI2SendAuthReply(req->client, TRUE);
+    AttendClient(req->client);
+    xorg_list_del(&req->link);
+    free(req);
+
+    xorg_list_for_each_entry(req, &xwl_screen->authenticate_client_list,
+	                     link) {
+	wl_drm_authenticate (xwl_screen->drm, req->magic);
+	return;
+    }
 }
 
 static const struct wl_drm_listener xwl_drm_listener =
@@ -143,21 +175,29 @@ int xwl_screen_get_drm_fd(struct xwl_screen *xwl_screen)
     return xwl_screen->drm_fd;
 }
 
-int xwl_drm_authenticate(struct xwl_screen *xwl_screen,
+int xwl_drm_authenticate(ClientPtr client, struct xwl_screen *xwl_screen,
 			    uint32_t magic)
 {
-    int ret;
+    struct xwl_auth_req *req;
 
-    xwl_screen->authenticated = 0;
+    if (!xwl_screen->drm)
+	return BadAccess;
 
-    if (xwl_screen->drm)
+    req = malloc (sizeof *req);
+    if (req == NULL)
+	return BadAlloc;
+
+    req->client = client;
+    req->xwl_screen = xwl_screen;
+    req->magic = magic;
+
+    if (xorg_list_is_empty(&xwl_screen->authenticate_client_list))
 	wl_drm_authenticate (xwl_screen->drm, magic);
 
-    ret = wl_display_roundtrip(xwl_screen->display);
-    if (ret == -1)
-	return BadAlloc;
-    if (!xwl_screen->authenticated)
-	return BadAlloc;
+    xorg_list_add(&req->link, &xwl_screen->authenticate_client_list);
+
+    IgnoreClient(req->client);
+    xwl_screen->authenticated = 0;
 
     return Success;
 }
diff --git a/hw/xfree86/xwayland/xwayland-private.h b/hw/xfree86/xwayland/xwayland-private.h
index 23263d0..5b602d7 100644
--- a/hw/xfree86/xwayland/xwayland-private.h
+++ b/hw/xfree86/xwayland/xwayland-private.h
@@ -61,6 +61,7 @@ struct xwl_screen {
     struct xorg_list		 seat_list;
     struct xorg_list		 damage_window_list;
     struct xorg_list		 window_list;
+    struct xorg_list		 authenticate_client_list;
     uint32_t			 serial;
 
     CreateWindowProcPtr		 CreateWindow;
diff --git a/hw/xfree86/xwayland/xwayland.c b/hw/xfree86/xwayland/xwayland.c
index 98e036e..d97f4ee 100644
--- a/hw/xfree86/xwayland/xwayland.c
+++ b/hw/xfree86/xwayland/xwayland.c
@@ -233,6 +233,7 @@ xwl_screen_pre_init(ScrnInfoPtr scrninfo, struct xwl_screen *xwl_screen,
     xorg_list_init(&xwl_screen->seat_list);
     xorg_list_init(&xwl_screen->damage_window_list);
     xorg_list_init(&xwl_screen->window_list);
+    xorg_list_init(&xwl_screen->authenticate_client_list);
     xwl_screen->scrninfo = scrninfo;
     xwl_screen->driver = driver;
     xwl_screen->flags = flags;
@@ -307,6 +308,7 @@ void xwl_screen_close(struct xwl_screen *xwl_screen)
     xorg_list_init(&xwl_screen->seat_list);
     xorg_list_init(&xwl_screen->damage_window_list);
     xorg_list_init(&xwl_screen->window_list);
+    xorg_list_init(&xwl_screen->authenticate_client_list);
 
     wl_display_roundtrip(xwl_screen->display);
 }
diff --git a/hw/xfree86/xwayland/xwayland.h b/hw/xfree86/xwayland/xwayland.h
index e536c42..f268366 100644
--- a/hw/xfree86/xwayland/xwayland.h
+++ b/hw/xfree86/xwayland/xwayland.h
@@ -69,7 +69,7 @@ extern _X_EXPORT void
 xwl_screen_post_damage(struct xwl_screen *xwl_screen);
 
 extern _X_EXPORT int
-xwl_drm_authenticate(struct xwl_screen *xwl_screen,
+xwl_drm_authenticate(ClientPtr client, struct xwl_screen *xwl_screen,
 		     uint32_t magic);
 
 extern _X_EXPORT int
-- 
1.7.9.5



More information about the wayland-devel mailing list