Skip to content

Commit

Permalink
memtx: fix heap-use-after-free of tuple stories caused by space alter
Browse files Browse the repository at this point in the history
When a space is altered, we abort all in-progress transactions and delete
all stories related to that space: the problem is we don't delete the
stories' read gaps, which are also linked to the stories' transactions,
which get cleaned up on transaction destruction — this, in turn, results in
heap-use-after-free. To fix this, clean up stories' read gap in
`memtx_on_space_delete` — we don't do this in `memtx_tx_story_delete` since
it expects the story to not have any read gaps (see
`memtx_tx_story_gc_step`).

Tested this patch manually against Nick Shirokovskiy's experimental
small-ASAN integration branch.

Closes #8781

NO_DOC=bugfix
NO_TEST=<already covered by existing tests, but was not detectable by ASAN>
  • Loading branch information
CuriousGeorgiy authored and alyapunov committed Jul 3, 2023
1 parent 9041d7e commit e1ed31b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
## bugfix/memtx

* Fixed a heap-use-after-free bug in the transaction manager, which could occur
when performing a DDL operation concurrently with a transaction on the same
space (gh-8781).
15 changes: 14 additions & 1 deletion src/box/memtx_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -946,15 +946,18 @@ memtx_tx_story_new(struct space *space, struct tuple *tuple)
return story;
}

/**
* Deletes a story. Expects the story to be fully unlinked.
*/
static void
memtx_tx_story_delete(struct memtx_story *story)
{
/* Expecting to delete fully unlinked story. */
assert(story->add_stmt == NULL);
assert(story->del_stmt == NULL);
for (uint32_t i = 0; i < story->index_count; i++) {
assert(story->link[i].newer_story == NULL);
assert(story->link[i].older_story == NULL);
assert(rlist_empty(&story->link[i].read_gaps));
}

memtx_tx_stats_discard(&txm.story_stats[story->status],
Expand Down Expand Up @@ -2943,6 +2946,16 @@ memtx_tx_on_space_delete(struct space *space)
if (story->del_stmt != NULL)
memtx_tx_history_remove_stmt(story->del_stmt);
memtx_tx_story_full_unlink_on_space_delete(story);
for (uint32_t i = 0; i < story->index_count; i++) {
struct rlist *read_gaps = &story->link[i].read_gaps;
while (!rlist_empty(&story->link[i].read_gaps)) {
struct gap_item_base *item =
rlist_first_entry(read_gaps,
struct gap_item_base,
in_read_gaps);
memtx_tx_delete_gap(item);
}
}
memtx_tx_story_delete(story);
}
}
Expand Down

0 comments on commit e1ed31b

Please sign in to comment.