Skip to content

memtx: fix use-after-free on background index build#10646

Merged
locker merged 1 commit intotarantool:masterfrom
drewdzzz:index_shadow_build_rollback_crash
Nov 1, 2024
Merged

memtx: fix use-after-free on background index build#10646
locker merged 1 commit intotarantool:masterfrom
drewdzzz:index_shadow_build_rollback_crash

Conversation

@drewdzzz
Copy link
Contributor

@drewdzzz drewdzzz commented Oct 4, 2024

When building an index in background, we create on_rollback triggers for tuples inserted concurrently. The problem here is on_rollback trigger has independent from index and memtx_ddl_state lifetime - it can be called after the index was build (and memtx_ddl_state is destroyed) and even after the index was altered. So, in order to avoid use-after-free in on_rollback trigger, let's drop all on_rollback triggers when the DDL is over. It's OK because all owners of triggers are already prepared, hence, in WAL or replication queue (since we build indexes in background only without MVCC so the transactions cannot yield), so if they are rolled back, the same will happen to the DDL.

Closes #10620

@coveralls
Copy link

coveralls commented Oct 4, 2024

Coverage Status

coverage: 87.327% (+0.004%) from 87.323%
when pulling 6d4de19 on drewdzzz:index_shadow_build_rollback_crash
into 6ae595f
on tarantool:master
.

@drewdzzz drewdzzz marked this pull request as ready for review October 4, 2024 10:18
@drewdzzz drewdzzz requested a review from a team as a code owner October 4, 2024 10:18
@drewdzzz drewdzzz requested a review from locker October 4, 2024 10:18
@locker locker assigned drewdzzz and unassigned locker Oct 4, 2024
@drewdzzz drewdzzz requested a review from locker October 29, 2024 14:33
@drewdzzz drewdzzz assigned locker and unassigned drewdzzz Oct 29, 2024
@locker locker assigned drewdzzz and unassigned locker Oct 30, 2024
@drewdzzz drewdzzz requested a review from ligurio as a code owner October 30, 2024 14:08
@drewdzzz drewdzzz requested a review from locker October 30, 2024 14:29
@drewdzzz drewdzzz assigned locker and unassigned drewdzzz Oct 30, 2024
@locker locker assigned drewdzzz and unassigned locker Oct 30, 2024
When building an index in background, we create on_rollback triggers for
tuples inserted concurrently. The problem here is on_rollback trigger
has independent from `index` and `memtx_ddl_state` lifetime - it can be
called after the index was build (and `memtx_ddl_state` is destroyed)
and even after the index was altered. So, in order to avoid
use-after-free in on_rollback trigger, let's drop all on_rollback
triggers when the DDL is over. It's OK because all owners of triggers
are already prepared, hence, in WAL or replication queue (since we
build indexes in background only without MVCC so the transactions cannot
yield), so if they are rolled back, the same will happen to the DDL.

In order to delete on_rollback triggers, we should collect them into a
list in `memtx_ddl_state`. On the other hand, when the DML statement is
over (committed or rolled back), we should delete its trigger from the
list to prevent use-after-free. That's why the commit adds the on_commit
trigger to background build process.

Closes #10620

NO_DOC=bugfix
@drewdzzz drewdzzz requested a review from locker October 31, 2024 10:29
@drewdzzz drewdzzz assigned locker and unassigned drewdzzz Oct 31, 2024
@locker locker assigned drewdzzz and ligurio and unassigned locker Oct 31, 2024
@ligurio ligurio added the full-ci Enables all tests for a pull request label Oct 31, 2024
@ligurio ligurio removed their assignment Oct 31, 2024
@drewdzzz drewdzzz assigned locker and unassigned drewdzzz Oct 31, 2024
@locker locker merged commit d8d82db into tarantool:master Nov 1, 2024
@locker
Copy link
Member

locker commented Nov 1, 2024

Cherry-picked to 2.11 and 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault in index_replace

5 participants