New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
memtx: fix use-after-free of successor in tree_iterator_start
#7759
memtx: fix use-after-free of successor in tree_iterator_start
#7759
Conversation
1344779
to
a92f029
Compare
a92f029
to
d72d7c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The patch solves the set problems, but I am concerned about the current development vector - some methods categorically cannot call GC, and some methods on the contrary can do it implicitly (for example, gap trackers now should not call GC, but all other trackers can), and most importantly - during the process of development and support of transaction manager we must ensure that methods without GC do not call methods with it. I believe we need to redesign the transaction manager garbage collection system. The easiest option is to make ALL GC calls explicit.
d72d7c5
to
a81a8b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created an issue with my concern - this patch is OK.
We assumed that the successor tuple's story could not get garbage collected on clarify of result tuple in `tree_iterator_start`, since they coincide in case of regular iterators. But this is not the case for reverse iterators: the result tuple is of-by-one from the successor, which means the successor's story can get garbage collected along with the tuple itself getting deleted, leading to use-after-free of successor: remove garbage collection from `memtx_tx_tuple_clarify` and call it manually. The crash in tarantool#7756 revealed that the `put` in transaction manager's story hash table was performed incorrectly: fix it and add an assertion that nothing was replaced. Closes tarantool#7755 Closes tarantool#7756 NO_DOC=bugfix
a81a8b0
to
8d3f75b
Compare
Backported to 2.10. |
We assumed that the successor tuple's story could not get garbage collected on clarify of result tuple in
tree_iterator_start
, since they coincide in case of regular iterators. But this is not the case for reverse iterators: the result tuple is of-by-one from the successor, which means the successor's story can get garbage collected along with the tuple itself getting deleted, leading to use-after-free of successor: remove garbage collection frommemtx_tx_tuple_clarify
and call it manually.The crash in #7756 revealed that the
put
in transaction manager's story hash table was performed incorrectly: fix it and add an assertion that nothing was replaced.Closes #7755
Closes #7756