On 28.09.2017 18:37, Eric Engestrom wrote:
On Thursday, 2017-09-28 16:10:51 +0000, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haeh...@amd.com>

A tempting alternative fix would be adding a lock/unlock pair in
util_queue_fence_is_signalled. However, that wouldn't actually
improve anything in the semantics of util_queue_fence_is_signalled,
while making that test much more heavy-weight. So this lock/unlock
pair in util_queue_fence_destroy for "flushing out" other threads
that may still be in util_queue_fence_signal looks like the better
fix.

Cc: mesa-sta...@lists.freedesktop.org
---
  src/util/u_queue.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 449da7dc9ab..82a761e8420 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -113,20 +113,33 @@ util_queue_fence_init(struct util_queue_fence *fence)
     memset(fence, 0, sizeof(*fence));
     (void) mtx_init(&fence->mutex, mtx_plain);
     cnd_init(&fence->cond);
     fence->signalled = true;
  }
void
  util_queue_fence_destroy(struct util_queue_fence *fence)
  {
     assert(fence->signalled);
+
+   /* Protect against the following race:
+    *
+    * util_queue_fence_signal (begin)
+    *   fence->signalled = true;
+    *                                             util_queue_fence_is_signalled
+    *                                             util_queue_fence_destroy
+    *   cnd_broadcast(&fence->cond); <-- use-after-free
+    * util_queue_fence_signal (end)
+    */
+   mtx_lock(&fence->mutex);
+   mtx_unlock(&fence->mutex);

How's this protecting anything?

util_queue_fence_signal locks fence->mutex. So the main part of util_queue_fence_destroy that actually destroy stuff will be delayed until after the end of util_queue_fence_signal.


This feels completely wrong...

I agree it's unusual, but it's pretty much explained by the commit message IMO.

Cheers,
Nicolai



+
     cnd_destroy(&fence->cond);
     mtx_destroy(&fence->mutex);
  }
/****************************************************************************
   * util_queue implementation
   */
struct thread_input {
     struct util_queue *queue;
--
2.11.0



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to