glx: Refcnt the GLXDrawable to avoid use after free with multiple FreeResource

Commit Message

Chris Wilson Dec. 10, 2010, 1:38 p.m. UTC

None

Patch

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index de9c3f0..b3ea784 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -529,6 +529,7 @@  __glXGetDrawable(__GLXcontext *glxc, GLXDrawable drawId, ClientPtr client,
 	*error = BadAlloc;
 	return NULL;
     }
+    pGlxDraw->refcnt++;
 
     return pGlxDraw;
 }
@@ -1099,8 +1100,10 @@  __glXDrawableInit(__GLXdrawable *drawable,
     drawable->pDraw = pDraw;
     drawable->type = type;
     drawable->drawId = drawId;
+    drawable->otherId = 0;
     drawable->config = config;
     drawable->eventMask = 0;
+    drawable->refcnt = 0;
 
     return GL_TRUE;
 }
@@ -1130,14 +1133,18 @@  DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen,
 	pGlxDraw->destroy (pGlxDraw);
 	return BadAlloc;
     }
+    pGlxDraw->refcnt++;
 
-    /* Add the glx drawable under the XID of the underlying X drawable
-     * too.  That way we'll get a callback in DrawableGone and can
-     * clean up properly when the drawable is destroyed. */
-    if (drawableId != glxDrawableId &&
-	!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
-	pGlxDraw->destroy (pGlxDraw);
-	return BadAlloc;
+    if (drawableId != glxDrawableId) {
+	/* Add the glx drawable under the XID of the underlying X drawable
+	 * too.  That way we'll get a callback in DrawableGone and can
+	 * clean up properly when the drawable is destroyed. */
+	if (!AddResource(drawableId, __glXDrawableRes, pGlxDraw)) {
+	    pGlxDraw->destroy (pGlxDraw);
+	    return BadAlloc;
+	}
+	pGlxDraw->refcnt++;
+	pGlxDraw->otherId = drawableId;
     }
 
     return Success;
diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h
index 2a365c5..80c3234 100644
--- a/glx/glxdrawable.h
+++ b/glx/glxdrawable.h
@@ -51,8 +51,11 @@  struct __GLXdrawable {
     void      (*waitX)(__GLXdrawable *);
     void      (*waitGL)(__GLXdrawable *);
 
+    int refcnt; /* number of resources handles referencing this */
+
     DrawablePtr pDraw;
     XID drawId;
+    XID otherId; /* for glx1.3 we need to track the original Drawable as well */
 
     /*
     ** Either GLX_DRAWABLE_PIXMAP, GLX_DRAWABLE_WINDOW or
diff --git a/glx/glxext.c b/glx/glxext.c
index 4bd5d6b..77db8b0 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -128,13 +128,18 @@  static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
      * constructors, we added it as a glx drawable resource under both
      * its glx drawable ID and it X drawable ID.  Remove the other
      * resource now so we don't a callback for freed memory. */
-    if (glxPriv->drawId != glxPriv->pDraw->id) {
-	if (xid == glxPriv->drawId)
-	    FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
-	else
-	    FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
+    if (glxPriv->otherId) {
+	    XID other = glxPriv->otherId;
+	    glxPriv->otherId = 0;
+	    if (xid == other)
+		    FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
+	    else
+		    FreeResourceByType(other, __glXDrawableRes, TRUE);
     }
 
+    if (--glxPriv->refcnt)
+	    return True;
+
     for (c = glxAllContexts; c; c = next) {
 	next = c->next;
 	if (c->isCurrent && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) {