[Mesa-dev] [PATCH 4/4] glx/dri2: Try and detect invalid DRI buffers

[George Barrett]

DRI routines in X drivers can return null buffers if they're unable to
allocate them as requested (for example, [1]) but the DRI 2 protocol
doesn't expose a mechanism to communicate this explicitly. Instead, we
assume that zeroed out buffer info structs are invalid and go from
there.

This commit also corrects a minor bug wherein dri2_bind_context
(dri2_glx.c) inadvertently indicates success upon the failure of the
provided dri2_screen->bindContext method.

[1]: https://cgit.freedesktop.org/xorg/driver/xf86-video-ati/tree/src/radeon_glamor.c?id=5cdd334b3402c2431deb3a87a8d04ef590da53ee#n201
---
 src/glx/dri2_glx.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 0f44635725..cc3b43146b 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -152,7 +152,9 @@ dri2_bind_context(struct glx_context *context, struct glx_context *old,
       return GLXBadDrawable;
 
    if (!(*psc->core->bindContext) (pcp->driContext, dri_draw, dri_read))
-      return GLXBadContext;
+      /* GLXBadContext has the same value as Success, so return something
+       * similar instead. */
+      return GLXBadContextState;
 
    /* If the server doesn't send invalidate events, we may miss a
     * resize before the rendering starts.  Invalidate the buffers now
@@ -719,16 +721,27 @@ dri2DestroyScreen(struct glx_screen *base)
  * Processes the list of buffers received in a reply from the server to either
  * \c DRI2GetBuffers or \c DRI2GetBuffersWithFormat.
  */
-static void
+static Bool
 process_buffers(struct dri2_drawable * pdraw, DRI2Buffer * buffers,
                 unsigned count)
 {
    int i;
+   /* The buffer we would expect if a pixmap allocator within X returned
+    * NullPixmap to a DRI routine. */
+   static const DRI2Buffer NullBuffer = {};
 
    pdraw->bufferCount = count;
    pdraw->have_fake_front = 0;
    pdraw->have_back = 0;
 
+   /* Attempt to test the validity of the buffer array we've been handed. A
+    * zero-valued pitch value should be indicative, but we call memcmp anyway
+    * to try and minimise false-positives. */
+   if (count == 1 && memcmp(buffers, &NullBuffer, sizeof(DRI2Buffer)) == 0) {
+       pdraw->bufferCount = 0;
+       return GL_FALSE;
+   }
+
    /* This assumes the DRI2 buffer attachment tokens matches the
     * __DRIbuffer tokens. */
    for (i = 0; i < count; i++) {
@@ -743,6 +756,7 @@ process_buffers(struct dri2_drawable * pdraw, DRI2Buffer * buffers,
          pdraw->have_back = 1;
    }
 
+   return GL_TRUE;
 }
 
 unsigned dri2GetSwapEventType(Display* dpy, XID drawable)
@@ -873,6 +887,7 @@ dri2GetBuffers(__DRIdrawable * driDrawable,
 {
    struct dri2_drawable *pdraw = loaderPrivate;
    DRI2Buffer *buffers;
+   int buffers_valid;
 
    buffers = DRI2GetBuffers(pdraw->base.psc->dpy, pdraw->base.xDrawable,
                             width, height, attachments, count, out_count);
@@ -881,11 +896,11 @@ dri2GetBuffers(__DRIdrawable * driDrawable,
 
    pdraw->width = *width;
    pdraw->height = *height;
-   process_buffers(pdraw, buffers, *out_count);
+   buffers_valid = process_buffers(pdraw, buffers, *out_count);
 
    free(buffers);
 
-   return pdraw->buffers;
+   return buffers_valid ? pdraw->buffers : NULL;
 }
 
 static __DRIbuffer *
@@ -896,6 +911,7 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
 {
    struct dri2_drawable *pdraw = loaderPrivate;
    DRI2Buffer *buffers;
+   int buffers_valid;
 
    buffers = DRI2GetBuffersWithFormat(pdraw->base.psc->dpy,
                                       pdraw->base.xDrawable,
@@ -906,11 +922,11 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
 
    pdraw->width = *width;
    pdraw->height = *height;
-   process_buffers(pdraw, buffers, *out_count);
+   buffers_valid = process_buffers(pdraw, buffers, *out_count);
 
    free(buffers);
 
-   return pdraw->buffers;
+   return buffers_valid ? pdraw->buffers : NULL;
 }
 
 static int
-- 
2.14.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171121/bdf00afd/attachment.sig>