xserver: Branch 'master' - 4 commits

Keith Packard keithp at kemper.freedesktop.org
Mon Dec 8 18:08:08 PST 2014


 glx/glxcmds.c |    4 +++-
 glx/glxext.c  |    8 ++++++--
 glx/glxext.h  |    1 -
 3 files changed, 9 insertions(+), 4 deletions(-)

New commits:
commit 3e7218a6c23354d66f508b18164cac98a346b3ee
Merge: 6f4c398 bc71081
Author: Keith Packard <keithp at keithp.com>
Date:   Mon Dec 8 18:07:55 2014 -0800

    Merge remote-tracking branch 'jturney/indirect-glx-fixes'

commit bc71081f0e3d8ce3aecf2cb168431dbc9fe6a87b
Author: Jon TURNEY <jon.turney at dronecode.org.uk>
Date:   Fri Apr 18 12:17:06 2014 +0100

    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 <jon.turney at dronecode.org.uk>
    Reviewed-by: Adam Jackson <ajax at redhat.com>

diff --git a/glx/glxext.c b/glx/glxext.c
index 978d271..e41b881 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 3f2dee6..cde0e15 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);
commit 5c606c0a89e74fa223a99864be11cc3be60a159b
Author: Jon TURNEY <jon.turney at dronecode.org.uk>
Date:   Fri Apr 18 12:17:05 2014 +0100

    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 <jon.turney at dronecode.org.uk>
    Reviewed-by: Adam Jackson <ajax at redhat.com>

diff --git a/glx/glxext.c b/glx/glxext.c
index c2de3ce..978d271 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;
commit 437b27494f127854d75e59b4e2aac264e9f913e9
Author: Jon TURNEY <jon.turney at dronecode.org.uk>
Date:   Fri Apr 18 12:17:04 2014 +0100

    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 <jon.turney at dronecode.org.uk>
    Reviewed-by: Adam Jackson <ajax at redhat.com>

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 8d3fa9f..009fd9b 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;
 }


More information about the xorg-commit mailing list