librbd/cache/pwl: cancel periodic_stats timer before perf_stop()

AbstractWriteLog::shut_down() calls perf_stop(), which deletes
m_perfcounter, but the 5s periodic_stats timer is only canceled later
in the destructor. If it fires in between, periodic_stats ->
update_image_cache_state dereferences the freed m_perfcounter. Cancel
the timer under m_timer_lock first.

Destroying an AbstractWriteLog that was init()'ed without a matching
shut_down() is illegal, so the cancel_event() in the destructor is now
redundant and is dropped.

Fixes: https://tracker.ceph.com/issues/77501
Signed-off-by: Sun Yuechi <sunyuechi@iscas.ac.cn>

diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc
index 70d8d7418a13f5e55e784c6d87fcfb9fe858b511..44ea8271b505ddc8968aa8b6725d3dc4c4a6d858 100644 (file)
--- a/src/librbd/cache/pwl/AbstractWriteLog.cc
+++ b/src/librbd/cache/pwl/AbstractWriteLog.cc
@@ -85,9 +85,7 @@ template <typename I>
 AbstractWriteLog<I>::~AbstractWriteLog() {
   ldout(m_image_ctx.cct, 15) << "enter" << dendl;
   {
-    std::lock_guard timer_locker(*m_timer_lock);
     std::lock_guard locker(m_lock);
-    m_timer->cancel_event(m_timer_ctx);
     m_thread_pool.stop();
     ceph_assert(m_deferred_ios.size() == 0);
     ceph_assert(m_ops_to_flush.size() == 0);
@@ -649,6 +647,11 @@ void AbstractWriteLog<I>::shut_down(Context *on_finish) {
 
   Context *ctx = new LambdaContext(
     [this, on_finish](int r) {
+      {
+        std::lock_guard timer_locker(*m_timer_lock);
+        m_timer->cancel_event(m_timer_ctx);
+        m_timer_ctx = nullptr;
+      }
       if (m_perfcounter) {
         perf_stop();
       }