Attachment 41618 Details for Bug 32822 – Refcnt the drawable

[patch] Refcnt the drawable

0001-glx-Refcnt-the-GLXDrawable-to-avoid-use-after-free-w.patch (text/plain), 4.06 KB, created by Chris Wilson on 2011-01-04 05:11:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2011-01-04 05:11:06 UTC

Size: 4.06 KB

patch

obsolete

>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     |   23 +++++++++++++++--------
> glx/glxdrawable.h |    3 +++
> glx/glxext.c      |   15 ++++++++++-----
> 3 files changed, 28 insertions(+), 13 deletions(-)
>
>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)) {
>-- 
>1.7.2.3
>

Actions: View | Splinter Review

Attachments on bug 32822: 41618

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.