From 437b27494f127854d75e59b4e2aac264e9f913e9 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Fri, 18 Apr 2014 12:17:04 +0100 Subject: [PATCH 1/3] Revert "glx: Simplify glXDestroyContext" This reverts commit 7f5adf73a0f9a951a6df201532b4031d38054369. This seems to miss the whole point of the idExists flag, as it makes the lifetime of that being true the same as the lifetime of the Context resource. The previously current context tag is always given in a MakeContextCurrent request, even if that context tag is no longer valid (for example, the context has been deleted), so this leads to BadContextTag errors. See fd.o bug #30089 for the makecurrenttest.c testcase, and some discussion of previous manifestations of this bug. Signed-off-by: Jon TURNEY Reviewed-by: Adam Jackson --- glx/glxcmds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 8d3fa9ff7..009fd9be0 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -413,7 +413,9 @@ __glXDisp_DestroyContext(__GLXclientState * cl, GLbyte * pc) &glxc, &err)) return err; - FreeResourceByType(req->context, __glXContextRes, FALSE); + glxc->idExists = GL_FALSE; + if (!glxc->currentClient) + FreeResourceByType(req->context, __glXContextRes, FALSE); return Success; } From 5c606c0a89e74fa223a99864be11cc3be60a159b Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Fri, 18 Apr 2014 12:17:05 +0100 Subject: [PATCH 2/3] glx: Flush context which is being made non-current due to drawable going away Some sequences of glean tests fail with GLXBadCurrentWindow when using indirect rendering, e.g. glean -t 'fpexceptions getString'. Flush a context which is being made non-current due to the drawable on which is it is current going away. Waiting until another context is made current is too late, as the drawable no longer exists. v2: Rewrite for direct GL dispatch v3: Inline FlushContext(), doesn't need to be a separate function e.g. LIBGL_ALWAYS_INDIRECT=1 ./glean -r results -o --quick -t "fpexceptions getString" fails with a BadContextTag error. Signed-off-by: Jon TURNEY Reviewed-by: Adam Jackson --- glx/glxext.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/glx/glxext.c b/glx/glxext.c index c2de3cedd..978d27112 100644 --- a/glx/glxext.c +++ b/glx/glxext.c @@ -133,6 +133,9 @@ DrawableGone(__GLXdrawable * glxPriv, XID xid) next = c->next; if (c->currentClient && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) { + /* flush the context */ + glFlush(); + c->hasUnflushedCommands = GL_FALSE; /* just force a re-bind the next time through */ (*c->loseCurrent) (c); lastGLContext = NULL; From bc71081f0e3d8ce3aecf2cb168431dbc9fe6a87b Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Fri, 18 Apr 2014 12:17:06 +0100 Subject: [PATCH 3/3] glx: Fix crash when a client exits without deleting GL contexts With the previous patches applied, we now have crash due to use-after-free when a client exits without deleting all it's GL contexts On client exit, CloseDownClient first calls glxClientCallback() with ClientStateGone, which calls __glXFreeContext() directly. Subsequently CloseDownClient() frees all the clients resources, which leads to ContextGone() being called for a context resource where the context has already been freed. Fix this by modifiying glxClientCallback() to free the context resource. Also make __glXFreeContext() static, as calling it directly leads to this problem, instead the context resource should be released. With the previous patches applied, this can be demonstrated with e.g. glxinfo, which doesn't delete it's context before exit. Signed-off-by: Jon TURNEY Reviewed-by: Adam Jackson --- glx/glxext.c | 5 +++-- glx/glxext.h | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/glx/glxext.c b/glx/glxext.c index 978d27112..e41b881f2 100644 --- a/glx/glxext.c +++ b/glx/glxext.c @@ -66,6 +66,7 @@ static DevPrivateKeyRec glxClientPrivateKeyRec; ** Forward declarations. */ static int __glXDispatch(ClientPtr); +static GLboolean __glXFreeContext(__GLXcontext * cx); /* ** Called when the extension is reset. @@ -189,7 +190,7 @@ __glXRemoveFromContextList(__GLXcontext * cx) /* ** Free a context. */ -GLboolean +static GLboolean __glXFreeContext(__GLXcontext * cx) { if (cx->idExists || cx->currentClient) @@ -294,7 +295,7 @@ glxClientCallback(CallbackListPtr *list, void *closure, void *data) c->loseCurrent(c); lastGLContext = NULL; c->currentClient = NULL; - __glXFreeContext(c); + FreeResourceByType(c->id, __glXContextRes, FALSE); } } diff --git a/glx/glxext.h b/glx/glxext.h index 3f2dee696..cde0e1519 100644 --- a/glx/glxext.h +++ b/glx/glxext.h @@ -51,7 +51,6 @@ #define GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT 0x20B1 #endif -extern GLboolean __glXFreeContext(__GLXcontext * glxc); extern void __glXFlushContextCache(void); extern Bool __glXAddContext(__GLXcontext * cx);