From abceb1652239629ed11187a5fc670a3b1a3a5bb1 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Thu, 16 Nov 2023 09:38:04 +0000 Subject: [PATCH] crimson/osd/osd_operations/snaptrim_event: lifetime fixes ``` // SnapTrimEvent is a background operation, // it's lifetime is not guarnteed since the caller // returned future is being ignored. We should capture // a self reference thourhgout the entire execution // progress (not only on finally() continuations). // See: PG::on_active_actmap() ``` Sanitized backtrace: ``` DEBUG 2023-11-16 08:42:48,441 [shard 0] osd - snaptrim_event(id=21122, detail=SnapTrimEvent(pgid=3.1 snapid=3cb needs_pause=1)): interrupted crimson::common::actingset_changed (acting set changed kernel callstack: #0 0x55e310e0ace7 in seastar::shared_mutex::unlock() (/usr/bin/ceph-osd+0x1edd0ce7) #1 0x55e313325d9c in auto seastar::futurize_invoke::ExitBarrier::BlockingEvent::Trigger >::exit()::{lambda()#1}&>(crimson::OrderedConcurrentPhaseT::ExitBarrier::BlockingEvent::Trigger >::exit()::{lambda()#1}&) (/usr/bin/ceph-osd+0x212ebd9c) #2 0x55e3133260ef in _ZN7seastar20noncopyable_functionIFNS_6futureIvEEvEE17direct_vtable_forIZNS2_4thenIZN7crimson23OrderedConcurrentPhaseTINS7_3osd13SnapTrimEvent9WaitSubopEE11ExitBarrierINSC_13BlockingEvent7TriggerISA_EEE4exitEvEUlvE_S2_EET0_OT_EUlDpOT_E_E4callEPKS4_ (/usr/bin/ceph-osd+0x212ec0ef) 0x61500013365c is located 92 bytes inside of 472-byte region [0x615000133600,0x6150001337d8) freed by thread T2 here: #0 0x7fb345ab73cf in operator delete(void*, unsigned long) (/lib64/libasan.so.6+0xb73cf) #1 0x55e313474863 in crimson::osd::SnapTrimEvent::~SnapTrimEvent() (/usr/bin/ceph-osd+0x2143a863) previously allocated by thread T2 here: #0 0x7fb345ab6367 in operator new(unsigned long) (/lib64/libasan.so.6+0xb6367) #1 0x55e31183ac18 in auto crimson::OperationRegistryI::create_operation(crimson::osd::PG*&&, SnapMapper&, snapid_t const&, bool const&) (/usr/bin/ceph-osd+0x1f800c18) SUMMARY: AddressSanitizer: heap-use-after-free (/usr/bin/ceph-osd+0x1edd0ce7) in seastar::shared_mutex::unlock() ``` Signed-off-by: Matan Breizman --- src/crimson/osd/osd_operations/snaptrim_event.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/crimson/osd/osd_operations/snaptrim_event.cc b/src/crimson/osd/osd_operations/snaptrim_event.cc index f76dd1e36f238..8ae36a5483dd4 100644 --- a/src/crimson/osd/osd_operations/snaptrim_event.cc +++ b/src/crimson/osd/osd_operations/snaptrim_event.cc @@ -90,7 +90,14 @@ SnapTrimEvent::start() { ShardServices &shard_services = pg->get_shard_services(); IRef ref = this; - return interruptor::with_interruption([&shard_services, this] { + return interruptor::with_interruption( + // SnapTrimEvent is a background operation, + // it's lifetime is not guarnteed since the caller + // returned future is being ignored. We should capture + // a self reference thourhgout the entire execution + // progress (not only on finally() continuations). + // See: PG::on_active_actmap() + [&shard_services, this, ref] { return enter_stage( client_pp().wait_for_active ).then_interruptible([this] { @@ -193,7 +200,8 @@ SnapTrimEvent::start() }); }); }); - }, [this](std::exception_ptr eptr) -> snap_trim_ertr::future { + }, [this, ref] + (std::exception_ptr eptr) -> snap_trim_ertr::future { logger().debug("{}: interrupted {}", *this, eptr); return crimson::ct_error::eagain::make(); }, pg).finally([this, ref=std::move(ref)] { -- 2.39.5