--- /dev/null
+From 6e3d1880fa78a3a965cb7eb51ee12b1f785f84bb Mon Sep 17 00:00:00 2001
+From: "Steinar H. Gunderson" <sesse@google.com>
+Date: Tue, 1 Dec 2015 22:05:11 +0100
+Subject: [PATCH] Fix locking of GLsync objects.
+
+GLsync objects had a race condition when used from multiple threads
+(which is the main point of the extension, really); it could be
+validated as a sync object at the beginning of the function, and then
+deleted by another thread before use, causing crashes. Fix this by
+changing all casts from GLsync to struct gl_sync_object to a new
+function _mesa_get_sync() that validates and increases the refcount.
+
+In a similar vein, validation itself uses _mesa_set_search(), which
+requires synchronization -- it was called without a mutex held, causing
+spurious error returns and other issues. Since _mesa_get_sync() now
+takes the shared context mutex, this problem is also resolved.
+
+Signed-off-by: Steinar H. Gunderson <sesse@google.com>
+---
+ src/mesa/main/objectlabel.c | 11 ++++--
+ src/mesa/main/shared.c | 2 +-
+ src/mesa/main/syncobj.c | 89 ++++++++++++++++++++++++++-------------------
+ src/mesa/main/syncobj.h | 11 ++----
+ 4 files changed, 64 insertions(+), 49 deletions(-)
+
+diff --git a/src/mesa/main/objectlabel.c b/src/mesa/main/objectlabel.c
+index 41f370c..b083c43 100644
+--- a/src/mesa/main/objectlabel.c
++++ b/src/mesa/main/objectlabel.c
+@@ -288,7 +288,7 @@ void GLAPIENTRY
+ _mesa_ObjectPtrLabel(const void *ptr, GLsizei length, const GLchar *label)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) ptr;
++ struct gl_sync_object *syncObj = _mesa_get_sync(ctx, sync, true);
+ const char *callerstr;
+ char **labelPtr;
+
+@@ -297,7 +297,7 @@ _mesa_ObjectPtrLabel(const void *ptr, GLsizei length, const GLchar *label)
+ else
+ callerstr = "glObjectPtrLabelKHR";
+
+- if (!_mesa_validate_sync(ctx, syncObj)) {
++ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s (not a valid sync object)",
+ callerstr);
+ return;
+@@ -306,6 +306,7 @@ _mesa_ObjectPtrLabel(const void *ptr, GLsizei length, const GLchar *label)
+ labelPtr = &syncObj->Label;
+
+ set_label(ctx, labelPtr, label, length, callerstr);
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ }
+
+ void GLAPIENTRY
+@@ -313,7 +314,7 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
+ GLchar *label)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) ptr;
++ struct gl_sync_object *syncObj;
+ const char *callerstr;
+ char **labelPtr;
+
+@@ -328,7 +329,8 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
+ return;
+ }
+
+- if (!_mesa_validate_sync(ctx, syncObj)) {
++ syncObj = _mesa_get_sync(ctx, sync, true);
++ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s (not a valid sync object)",
+ callerstr);
+ return;
+@@ -337,4 +339,5 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
+ labelPtr = &syncObj->Label;
+
+ copy_label(*labelPtr, label, length, bufSize);
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ }
+diff --git a/src/mesa/main/shared.c b/src/mesa/main/shared.c
+index c37b31d..b9f7bb6 100644
+--- a/src/mesa/main/shared.c
++++ b/src/mesa/main/shared.c
+@@ -338,7 +338,7 @@ free_shared_state(struct gl_context *ctx, struct gl_shared_state *shared)
+ struct set_entry *entry;
+
+ set_foreach(shared->SyncObjects, entry) {
+- _mesa_unref_sync_object(ctx, (struct gl_sync_object *) entry->key);
++ _mesa_unref_sync_object(ctx, (struct gl_sync_object *) entry->key, 1);
+ }
+ }
+ _mesa_set_destroy(shared->SyncObjects, NULL);
+diff --git a/src/mesa/main/syncobj.c b/src/mesa/main/syncobj.c
+index c1b2d3b..d1c6c06 100644
+--- a/src/mesa/main/syncobj.c
++++ b/src/mesa/main/syncobj.c
+@@ -167,34 +167,42 @@ _mesa_free_sync_data(struct gl_context *ctx)
+ * - not in sync objects hash table
+ * - type is GL_SYNC_FENCE
+ * - not marked as deleted
++ *
++ * Returns the internal gl_sync_object pointer if the sync object is valid
++ * or NULL if it isn't.
++ *
++ * If "incRefCount" is true, the reference count is incremented, which is
++ * normally what you want; otherwise, a glDeleteSync from another thread
++ * could delete the sync object while you are still working on it.
+ */
+-bool
+-_mesa_validate_sync(struct gl_context *ctx,
+- const struct gl_sync_object *syncObj)
++struct gl_sync_object *
++_mesa_get_sync(struct gl_context *ctx, GLsync sync, bool incRefCount)
+ {
+- return (syncObj != NULL)
++ struct gl_sync_object *syncObj = (struct gl_sync_object *) sync;
++ mtx_lock(&ctx->Shared->Mutex);
++ if (syncObj != NULL
+ && _mesa_set_search(ctx->Shared->SyncObjects, syncObj) != NULL
+ && (syncObj->Type == GL_SYNC_FENCE)
+- && !syncObj->DeletePending;
+-}
+-
+-
+-void
+-_mesa_ref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj)
+-{
+- mtx_lock(&ctx->Shared->Mutex);
+- syncObj->RefCount++;
++ && !syncObj->DeletePending) {
++ if (incRefCount) {
++ syncObj->RefCount++;
++ }
++ } else {
++ syncObj = NULL;
++ }
+ mtx_unlock(&ctx->Shared->Mutex);
++ return syncObj;
+ }
+
+
+ void
+-_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj)
++_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj,
++ int amount)
+ {
+ struct set_entry *entry;
+
+ mtx_lock(&ctx->Shared->Mutex);
+- syncObj->RefCount--;
++ syncObj->RefCount -= amount;
+ if (syncObj->RefCount == 0) {
+ entry = _mesa_set_search(ctx->Shared->SyncObjects, syncObj);
+ assert (entry != NULL);
+@@ -212,10 +220,9 @@ GLboolean GLAPIENTRY
+ _mesa_IsSync(GLsync sync)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
+ ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_FALSE);
+
+- return _mesa_validate_sync(ctx, syncObj) ? GL_TRUE : GL_FALSE;
++ return _mesa_get_sync(ctx, sync, false) ? GL_TRUE : GL_FALSE;
+ }
+
+
+@@ -223,7 +230,7 @@ void GLAPIENTRY
+ _mesa_DeleteSync(GLsync sync)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
++ struct gl_sync_object *syncObj;
+
+ /* From the GL_ARB_sync spec:
+ *
+@@ -235,16 +242,19 @@ _mesa_DeleteSync(GLsync sync)
+ return;
+ }
+
+- if (!_mesa_validate_sync(ctx, syncObj)) {
++ syncObj = _mesa_get_sync(ctx, sync, true);
++ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glDeleteSync (not a valid sync object)");
+ return;
+ }
+
+ /* If there are no client-waits or server-waits pending on this sync, delete
+- * the underlying object.
++ * the underlying object. Note that we double-unref the object, as _mesa_get_sync
++ * above took an extra refcount to make sure the pointer is valid for us to
++ * manipulate.
+ */
+ syncObj->DeletePending = GL_TRUE;
+- _mesa_unref_sync_object(ctx, syncObj);
++ _mesa_unref_sync_object(ctx, syncObj, 2);
+ }
+
+
+@@ -299,21 +309,20 @@ GLenum GLAPIENTRY
+ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
++ struct gl_sync_object *syncObj;
+ GLenum ret;
+ ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_WAIT_FAILED);
+
+- if (!_mesa_validate_sync(ctx, syncObj)) {
+- _mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync (not a valid sync object)");
+- return GL_WAIT_FAILED;
+- }
+-
+ if ((flags & ~GL_SYNC_FLUSH_COMMANDS_BIT) != 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync(flags=0x%x)", flags);
+ return GL_WAIT_FAILED;
+ }
+
+- _mesa_ref_sync_object(ctx, syncObj);
++ syncObj = _mesa_get_sync(ctx, sync, true);
++ if (!syncObj) {
++ _mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync (not a valid sync object)");
++ return GL_WAIT_FAILED;
++ }
+
+ /* From the GL_ARB_sync spec:
+ *
+@@ -335,7 +344,7 @@ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
+ }
+ }
+
+- _mesa_unref_sync_object(ctx, syncObj);
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ return ret;
+ }
+
+@@ -344,12 +353,7 @@ void GLAPIENTRY
+ _mesa_WaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
+-
+- if (!_mesa_validate_sync(ctx, syncObj)) {
+- _mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync (not a valid sync object)");
+- return;
+- }
++ struct gl_sync_object *syncObj;
+
+ if (flags != 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync(flags=0x%x)", flags);
+@@ -362,7 +366,14 @@ _mesa_WaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
+ return;
+ }
+
++ syncObj = _mesa_get_sync(ctx, sync, true);
++ if (!syncObj) {
++ _mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync (not a valid sync object)");
++ return;
++ }
++
+ ctx->Driver.ServerWaitSync(ctx, syncObj, flags, timeout);
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ }
+
+
+@@ -371,11 +382,12 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
+ GLint *values)
+ {
+ GET_CURRENT_CONTEXT(ctx);
+- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
++ struct gl_sync_object *syncObj;
+ GLsizei size = 0;
+ GLint v[1];
+
+- if (!_mesa_validate_sync(ctx, syncObj)) {
++ syncObj = _mesa_get_sync(ctx, sync, true);
++ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glGetSynciv (not a valid sync object)");
+ return;
+ }
+@@ -409,6 +421,7 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
+
+ default:
+ _mesa_error(ctx, GL_INVALID_ENUM, "glGetSynciv(pname=0x%x)\n", pname);
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ return;
+ }
+
+@@ -421,4 +434,6 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
+ if (length != NULL) {
+ *length = size;
+ }
++
++ _mesa_unref_sync_object(ctx, syncObj, 1);
+ }
+diff --git a/src/mesa/main/syncobj.h b/src/mesa/main/syncobj.h
+index 5d510e8..e8dbded 100644
+--- a/src/mesa/main/syncobj.h
++++ b/src/mesa/main/syncobj.h
+@@ -47,15 +47,12 @@ _mesa_init_sync(struct gl_context *);
+ extern void
+ _mesa_free_sync_data(struct gl_context *);
+
+-extern void
+-_mesa_ref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj);
++struct gl_sync_object *
++_mesa_get_sync(struct gl_context *ctx, GLsync sync, bool incRefCount);
+
+ extern void
+-_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj);
+-
+-extern bool
+-_mesa_validate_sync(struct gl_context *ctx,
+- const struct gl_sync_object *syncObj);
++_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj,
++ int amount);
+
+ extern GLboolean GLAPIENTRY
+ _mesa_IsSync(GLsync sync);
+--
+2.6.2
projects / nageru / blobdiff