[gegl] buffer: fix potential race condition during tile-swap entry destruction



commit 4241b0fd5b39cee9bd9e84cdbc30a31e50ff8bf9
Author: Ell <ell_se yahoo com>
Date:   Sat Aug 11 08:37:22 2018 -0400

    buffer: fix potential race condition during tile-swap entry destruction
    
    In gegl_tile_backend_swap_entry_destroy(), don't free the entry in
    the calling thread when it has a pending op in the queue, but no
    allocated storage, since the test for whether the entry has
    allocated storage introduces a race condition between the calling
    thread and the writer thread, where storage allocation happens.
    Instead, always convert the existing op to an OP_DESTROY.
    
    In particular, this could lead to a use-after-free and/or swap-
    space leak, if a TILE_SET command is issued for a tile while it is
    being written to the swap by the writer thread (after the
    corresponding OP_WRITE has been dequeued).  In this case, another
    OP_WRITE is pushed to the queue.  If the corresponding entry
    doesn't have storage allocated yet, and is then destroyed, the
    entry will be freed while the writer thread is still potentially
    using it while serving the earlier OP_WRITE, and the storage
    subsequently allocated for it will never be reclaimed.

 gegl/buffer/gegl-tile-backend-swap.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)
---
diff --git a/gegl/buffer/gegl-tile-backend-swap.c b/gegl/buffer/gegl-tile-backend-swap.c
index 6eb32224f..563e12455 100644
--- a/gegl/buffer/gegl-tile-backend-swap.c
+++ b/gegl/buffer/gegl-tile-backend-swap.c
@@ -326,10 +326,12 @@ gegl_tile_backend_swap_destroy (ThreadParams *params)
   start = params->entry->offset;
   end = start + params->length;
 
-  g_assert (start >= 0);
-
   g_slice_free (SwapEntry, params->entry);
 
+  /* storage for entry not allocated yet.  nothing more to do. */
+  if (start < 0)
+    return;
+
   if ((hlink = gap_list))
     while (hlink)
       {
@@ -626,26 +628,8 @@ gegl_tile_backend_swap_entry_destroy (GeglTileBackendSwap *self,
           queued_op->tile = NULL;
         }
 
-      if (entry->offset >= 0)
-        {
-          /* the queued op's entry already has storage allocated to it, which
-           * needs to be reclaimed.  change it to an OP_DESTROY, and move it to
-           * the top.
-           */
-          queued_op->operation = OP_DESTROY;
-
-          g_queue_unlink (queue, link);
-          g_queue_push_head_link (queue, link);
-        }
-      else
-        {
-          /* the queued op's entry doesn't have storage allocated to it, so we
-           * can simply free it here.
-           */
-          g_queue_delete_link (queue, link);
-          g_slice_free (ThreadParams, queued_op);
-          g_slice_free (SwapEntry, entry);
-        }
+      /* reuse the queued op, changing it to an OP_DESTROY. */
+      queued_op->operation = OP_DESTROY;
 
       return;
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]