From 8cbe42eb332aa9bf50d80174dc6bd85242d662ff Mon Sep 17 00:00:00 2001
From: Andrey Saranchin <Andrey22102001@gmail.com>
Date: Wed, 28 Aug 2024 12:28:46 +0300
Subject: [PATCH] memtx: fix use-after-free in mvcc on ddl
When space is being altered, `memtx_tx_space_on_delete` is called - it
deletes all the stories associated with the old schema. However, before
deleting a story, its `reader_list` member is not unlinked from the list
so other nodes can still access this memory. The commit fixes this
problem and adds an assertion that checks if story is always unlinked
from reader list when is being deleted.
Part of #10146
NO_CHANGELOG=later
NO_DOC=bugfix
(cherry picked from commit a32f56dfbb4b56b410ac376fce079613cac0ccb6)
---
src/box/memtx_tx.c | 13 ++++++++
test/box-luatest/mvcc_ddl_test.lua | 51 ++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c
index 52963c49ef..f9e32cbc11 100644
--- a/src/box/memtx_tx.c
+++ b/src/box/memtx_tx.c
@@ -956,6 +956,7 @@ memtx_tx_story_delete(struct memtx_story *story)
{
assert(story->add_stmt == NULL);
assert(story->del_stmt == NULL);
+ assert(rlist_empty(&story->reader_list));
for (uint32_t i = 0; i < story->index_count; i++) {
assert(story->link[i].newer_story == NULL);
assert(story->link[i].older_story == NULL);
@@ -2991,6 +2992,18 @@ memtx_tx_on_space_delete(struct space *space)
memtx_tx_delete_gap(item);
}
}
+ /*
+ * Remove all read trackers since they point to the story that
+ * is going to be deleted.
+ */
+ while (!rlist_empty(&story->reader_list)) {
+ struct tx_read_tracker *tracker =
+ rlist_first_entry(&story->reader_list,
+ struct tx_read_tracker,
+ in_reader_list);
+ rlist_del(&tracker->in_reader_list);
+ rlist_del(&tracker->in_read_set);
+ }
memtx_tx_story_delete(story);
}
}
diff --git a/test/box-luatest/mvcc_ddl_test.lua b/test/box-luatest/mvcc_ddl_test.lua
index 24fd45c7fc..0b6b1080fc 100644
--- a/test/box-luatest/mvcc_ddl_test.lua
+++ b/test/box-luatest/mvcc_ddl_test.lua
@@ -86,3 +86,54 @@ g.test_background_build = function(cg)
box.internal.memtx_tx_gc(1000)
end)
end
+
+-- The test covers a crash when transaction that is being deleted removes
+-- itself from reader list of a deleted story that leads to use-after-free
+g.test_reader_list_use_after_free = function(cg)
+ cg.server:exec(function()
+ local txn_proxy = require("test.box.lua.txn_proxy")
+
+ -- Create space with tuples
+ local s = box.schema.space.create('test')
+ s:create_index('pk')
+
+ box.begin()
+ for i = 1, 10000 do
+ s:replace{i}
+ end
+ box.commit()
+
+ -- Create a transaction that reads every tuple so it's
+ -- inserted to reader list of every story
+ local tx = txn_proxy.new()
+ tx:begin()
+ for i = 1, 10000 do
+ tx('box.space.test:get{' .. i .. '}')
+ end
+
+ -- Create a new index
+ -- Firstly, we need it so that all the stories will be deleted
+ -- due to DDL
+ -- Secondly, we need to create a new index so that layout of stories
+ -- will be changed and use-after-free on rlist link will trash another
+ -- field (for example, pointer to tuple) and that's will definitely lead
+ -- to crash
+ box.space.test:create_index('sk')
+
+ -- Open a read-view so that stories for all tuples from the space
+ -- are created
+ local rv = txn_proxy.new()
+ rv:begin()
+ rv('box.space.test:select{}')
+
+ -- Rollback the first reader so that it will delete itself from reader
+ -- lists of all stories and that will lead to use-after-free
+ tx:rollback()
+
+ -- Read all the tuples, Tarantool is most likely to crash here if
+ -- use-after-free broke something
+ for i = 1, 10000 do
+ s:get{i}
+ end
+ end)
+end
--
GitLab