| 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 |
| |