blob: c337550e4e00debfbf976f075d20c03eb40768f5 [file] [log] [blame]
From 37093bb245ccd57b7ed65e71163ea5b65e949521 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri, 10 Dec 2010 11:30:34 +0000
Subject: [PATCH] glx: Refcnt the GLXDrawable to avoid use after free with multiple FreeResource
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Although there may be more than one resource handles pointing to the
Drawable, we only want to destroy it once and only reference the
resource which may have just been deleted on the first instance.
v2: Apply fixes and combine with another bug fix from Michel Dänzer,
https://bugs.freedesktop.org/show_bug.cgi?id=28181
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kristian Høgsberg <krh@bitplanet.net>
Cc: Michel Dänzer <daenzer@vmware.com>
---
glx/glxcmds.c | 24 +++++++++++++++---------
glx/glxdrawable.h | 3 +++
glx/glxext.c | 19 ++++++++++---------
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index de9c3f0..b3ea784 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -507,6 +507,7 @@ __glXGetDrawable(__GLXcontext * glxc, GLXDrawable drawId, ClientPtr client,
*error = BadAlloc;
return NULL;
}
+ pGlxDraw->refcnt++;
return pGlxDraw;
}
@@ -1127,8 +1128,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;
}
@@ -1158,15 +1161,18 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen * pGlxScreen,
pGlxDraw->destroy(pGlxDraw);
return BadAlloc;
}
-
- /*
- * Windows aren't refcounted, so track both the X and the GLX window
- * so we get called regardless of destruction order.
- */
- if (drawableId != glxDrawableId && type == GLX_DRAWABLE_WINDOW &&
- !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
- pGlxDraw->destroy(pGlxDraw);
- return BadAlloc;
+ pGlxDraw->refcnt++;
+
+ if (drawableId != glxDrawableId && type == GLX_DRAWABLE_WINDOW) {
+ /* 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
@@ -123,17 +123,18 @@ DrawableGone(__GLXdrawable * glxPriv, XID xid)
{
__GLXcontext *c, *next;
- if (glxPriv->type == GLX_DRAWABLE_WINDOW) {
- /* If this was created by glXCreateWindow, free the matching resource */
- if (glxPriv->drawId != glxPriv->pDraw->id) {
- if (xid == glxPriv->drawId)
- FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
- else
- FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
- }
- /* otherwise this window was implicitly created by MakeCurrent */
+ 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)) {
--
1.7.7.3