From 8ad6b379137504bab3d181386922bdf0de53649e Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 25 Apr 2020 02:03:26 +0300 Subject: [PATCH 1/2] Proper StorageDistributed shutdown to avoid UAF in DistributedMonitor StorageDistributed::shutdown() does not acquire the lock, that controls access to the cluster_nodes_data, thus it does not synced with the requireDirectoryMonitor(), hence some monitors can be untracked that will trigger UAF (use-after-free) after DROP TABLE dist: This is for the SIGSEGV from the DirectoryMonitor (with already destroyed storage): 0 0x0000000008e9f760 in std::__1::__cxx_atomic_load (__order=std::__1::memory_order::seq_cst, __a=0x0) 1 std::__1::__atomic_base::load (__m=std::__1::memory_order::seq_cst, this=0x0) <-- this is nullptr 2 std::__1::__atomic_base::operator int (this=0x0) 3 DB::ActionBlocker::isCancelled (this=0x7f85e31c9bb8) at ../src/Common/ActionBlocker.h:18 4 DB::StorageDistributedDirectoryMonitor::run (this=0x7f85f93b2a00) at ../src/Storages/Distributed/DirectoryMonitor.cpp:140 --- src/Storages/StorageDistributed.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Storages/StorageDistributed.cpp b/src/Storages/StorageDistributed.cpp index 9c557ad5c8a..2bd0eda2d3d 100644 --- a/src/Storages/StorageDistributed.cpp +++ b/src/Storages/StorageDistributed.cpp @@ -570,6 +570,7 @@ void StorageDistributed::startup() void StorageDistributed::shutdown() { + std::lock_guard lock(cluster_nodes_mutex); cluster_nodes_data.clear(); } From 747a74215f4938bc5265d38e877675d45122e52a Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 25 Apr 2020 02:03:27 +0300 Subject: [PATCH 2/2] Avoid processing all batches before Distributed shutdown --- src/Storages/StorageDistributed.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Storages/StorageDistributed.cpp b/src/Storages/StorageDistributed.cpp index 2bd0eda2d3d..84d6cf8cc0e 100644 --- a/src/Storages/StorageDistributed.cpp +++ b/src/Storages/StorageDistributed.cpp @@ -570,6 +570,8 @@ void StorageDistributed::startup() void StorageDistributed::shutdown() { + monitors_blocker.cancelForever(); + std::lock_guard lock(cluster_nodes_mutex); cluster_nodes_data.clear(); }