Skip to content
Snippets Groups Projects
Commit 8cbe42eb authored by Andrey Saranchin's avatar Andrey Saranchin Committed by Serge Petrenko
Browse files

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)
parent 74584869
No related branches found
No related tags found
No related merge requests found
......@@ -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);
}
}
......
......@@ -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
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment