librbd: don't complete ImageUpdateWatchers::shut_down() prematurely

ImageUpdateWatchers::flush() requests aren't tracked with
m_in_flight-like mechanism the way ImageUpdateWatchers::send_notify()
requests are, but in both cases callbacks that represent delayed work
that is very likely to (indirectly) reference ImageCtx are involved.
When the image is getting closed, ImageUpdateWatchers::shut_down() is
called before anything that belongs to ImageCtx is destroyed.  However,
the shutdown can complete prematurely in the face of a pending flush if
one gets sent shortly before CloseRequest is invoked.  The callback for
that flush will then race with CloseRequest and may execute after parts
of or even the entire ImageCtx is destroyed, leading to use-after-free
and various segfaults.

Fixes: https://tracker.ceph.com/issues/75161
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc
index aa3bbecfc79778c5a36e22d6e7bcc6cfafb80206..a9480f2b4bf1f8c27f7e55a02bf2053ff00d6c24 100644 (file)
--- a/src/librbd/ImageState.cc
+++ b/src/librbd/ImageState.cc
@@ -64,6 +64,10 @@ public:
 
   void shut_down(Context *on_finish) {
     ldout(m_cct, 20) << "ImageUpdateWatchers::" << __func__ << dendl;
+    if (m_work_queue != nullptr) {
+      // ensure all pending flush() callbacks execute before completing
+      on_finish = create_async_context_callback(m_work_queue, on_finish);
+    }
     {
       std::lock_guard locker{m_lock};
       ceph_assert(m_on_shut_down_finish == nullptr);
@@ -73,8 +77,6 @@ public:
        return;
       }
     }
-    ldout(m_cct, 20) << "ImageUpdateWatchers::" << __func__
-                    << ": completing shut down" << dendl;
     on_finish->complete(0);
   }
 
@@ -184,8 +186,6 @@ public:
     }
 
     if (on_shut_down_finish != nullptr) {
-      ldout(m_cct, 20) << "ImageUpdateWatchers::" << __func__
-                      << ": completing shut down" << dendl;
       on_shut_down_finish->complete(0);
     }
   }