[gegl] buffer: fix potential race condition during tile-swap entry destruction
- From: Ell <ell src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gegl] buffer: fix potential race condition during tile-swap entry destruction
- Date: Sat, 11 Aug 2018 12:58:48 +0000 (UTC)
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]