From 9c3f51e99b787279fc71840c3f5b33fc823d66ef Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Wed, 22 Sep 2021 16:42:19 +0000 Subject: [PATCH] crimson/os/alienstore: fix nullptr deref in OnCommit::finish(). `seastar::engine()` is available only for Seastar's threads; it shouldn't be called outside of a reactor thread. Unfortunately, this assumption is violated in `AlienStore` where `OnCommit::finish()`, executed from a finisher thread of `BlueStore`, calls `alien()` on `seastar::engine()`. The net effect are crashes like the following one: ``` INFO 2021-09-22 14:26:33,214 [shard 0] osd - operator() writing superblock cluster_fsid 1d8f7908-2ebf-4a91-ae70-f445668c126b osd_fsid 4da9fe9a-1da5-4ea9-aa79-a1178165ede5 [381/1839] Segmentation fault. Backtrace: 0# print_backtrace(std::basic_string_view >) at /home/rzarzynski/ceph1/build/../src/crimson/common/fatal_signal.cc:80 1# FatalSignal::signaled(int, siginfo_t const&) at /opt/rh/gcc-toolset-9/root/usr/include/c++/9/ostream:570 2# FatalSignal::install_oneshot_signal_handler<11>()::{lambda(int, siginfo_t*, void*)#1}::_FUN(int, siginfo_t*, void*) at /home/rzarzynski/ceph1/build/../src/crimson/common/fatal_signal.cc: 62 3# 0x00007F16BBA13B30 in /lib64/libpthread.so.0 4# (anonymous namespace)::OnCommit::finish(int) at /home/rzarzynski/ceph1/build/../src/crimson/os/alienstore/alien_store.cc:53 5# Context::complete(int) at /home/rzarzynski/ceph1/build/../src/include/Context.h:100 6# Finisher::finisher_thread_entry() at /home/rzarzynski/ceph1/build/../src/common/Finisher.cc:65 7# 0x00007F16BBA0915A in /lib64/libpthread.so.0 8# clone in /lib64/libc.so.6 Dump of siginfo: ... si_addr: 0x10 ``` Signed-off-by: Radoslaw Zarzynski --- src/crimson/os/alienstore/alien_store.cc | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc index c9dd14501d8a4..719a3d1b63fd5 100644 --- a/src/crimson/os/alienstore/alien_store.cc +++ b/src/crimson/os/alienstore/alien_store.cc @@ -39,18 +39,23 @@ class OnCommit final: public Context { const int cpuid; Context *oncommit; + seastar::alien::instance &alien; seastar::promise<> &alien_done; public: OnCommit( int id, seastar::promise<> &done, Context *oncommit, + seastar::alien::instance &alien, ceph::os::Transaction& txn) - : cpuid(id), oncommit(oncommit), - alien_done(done) {} + : cpuid(id), + oncommit(oncommit), + alien(alien), + alien_done(done) { + } void finish(int) final { - return seastar::alien::submit_to(seastar::engine().alien(), cpuid, [this] { + return seastar::alien::submit_to(alien, cpuid, [this] { if (oncommit) { oncommit->complete(0); } @@ -425,9 +430,9 @@ seastar::future<> AlienStore::do_transaction(CollectionRef ch, ceph::os::Transaction::collect_all_contexts(txn); assert(tp); return tp->submit(ch->get_cid().hash_to_shard(tp->size()), - [this, ch, id, crimson_wrapper, &txn, &done] { + [this, ch, id, crimson_wrapper, &txn, &done, &alien=seastar::engine().alien()] { txn.register_on_commit(new OnCommit(id, done, crimson_wrapper, - txn)); + alien, txn)); auto c = static_cast(ch.get()); return store->queue_transaction(c->collection, std::move(txn)); }); -- 2.47.3