compaction: make a local copy of owned ranges

table_resharding_compaction_task_impl::run() performs the forbidden action of copying a lw_shared_ptr (_owned_ranges_ptr) on a remote shard, which is a data race that can cause a use-after-free, typically manifesting as allocator corruption. Content of _owned_ranges_ptr is copied to local lw_shared_ptrs. Fixes scylladb#14475 Fixes scylladb#14618
Deexie · Jul 11, 2023 · 1ba4561 · 1ba4561
1 parent 2de168e
commit 1ba4561
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 10 deletions.
diff --git a/compaction/task_manager_module.cc b/compaction/task_manager_module.cc
@@ -534,7 +534,12 @@ future<> table_resharding_compaction_task_impl::run() {
     co_await _db.invoke_on_all(coroutine::lambda([&] (replica::database& db) -> future<> {
         tasks::task_info parent_info{_status.id, _status.shard};
         auto& compaction_module = _db.local().get_compaction_manager().get_task_manager_module();
-        auto task = co_await compaction_module.make_and_start_task<shard_resharding_compaction_task_impl>(parent_info, _status.keyspace, _status.table, _status.id, _dir, db, _creator, _owned_ranges_ptr, destinations);
+        // make shard-local copy of owned_ranges
+        compaction::owned_ranges_ptr local_owned_ranges_ptr;
+        if (_owned_ranges_ptr) {
+            local_owned_ranges_ptr = make_lw_shared<const dht::token_range_vector>(*_owned_ranges_ptr);
+        }
+        auto task = co_await compaction_module.make_and_start_task<shard_resharding_compaction_task_impl>(parent_info, _status.keyspace, _status.table, _status.id, _dir, db, _creator, std::move(local_owned_ranges_ptr), destinations);
         co_await task->done();
     }));
 
@@ -549,12 +554,7 @@ tasks::is_internal shard_resharding_compaction_task_impl::is_internal() const no
 future<> shard_resharding_compaction_task_impl::run() {
     auto& table = _db.find_column_family(_status.keyspace, _status.table);
     auto info_vec = std::move(_destinations[this_shard_id()].info_vec);
-    // make shard-local copy of owned_ranges
-    compaction::owned_ranges_ptr local_owned_ranges_ptr;
-    if (_owned_ranges_ptr) {
-        local_owned_ranges_ptr = make_lw_shared<const dht::token_range_vector>(*_owned_ranges_ptr);
-    }
-    co_await reshard(_dir.local(), std::move(info_vec), table, _creator, std::move(local_owned_ranges_ptr));
+    co_await reshard(_dir.local(), std::move(info_vec), table, _creator, std::move(_local_owned_ranges_ptr));
     co_await _dir.local().move_foreign_sstables(_dir);
 }
 

diff --git a/compaction/task_manager_module.hh b/compaction/task_manager_module.hh
@@ -606,7 +606,7 @@ private:
     sharded<sstables::sstable_directory>& _dir;
     replica::database& _db;
     sstables::compaction_sstable_creator_fn _creator;
-    compaction::owned_ranges_ptr _owned_ranges_ptr;
+    compaction::owned_ranges_ptr _local_owned_ranges_ptr;
     std::vector<replica::reshard_shard_descriptor>& _destinations;
 public:
     shard_resharding_compaction_task_impl(tasks::task_manager::module_ptr module,
@@ -616,13 +616,13 @@ public:
             sharded<sstables::sstable_directory>& dir,
             replica::database& db,
             sstables::compaction_sstable_creator_fn creator,
-            compaction::owned_ranges_ptr owned_ranges_ptr,
+            compaction::owned_ranges_ptr local_owned_ranges_ptr,
             std::vector<replica::reshard_shard_descriptor>& destinations) noexcept
         : resharding_compaction_task_impl(module, tasks::task_id::create_random_id(), 0, std::move(keyspace), std::move(table), "", parent_id)
         , _dir(dir)
         , _db(db)
         , _creator(std::move(creator))
-        , _owned_ranges_ptr(std::move(owned_ranges_ptr))
+        , _local_owned_ranges_ptr(std::move(local_owned_ranges_ptr))
         , _destinations(destinations)
     {}