From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 13 Apr 2016 02:29:50 +0000 (-0400)
Subject: journal: potential for double-free of context on shut down
X-Git-Tag: ses3-milestone4~17^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=760863f47e829cee8d0e5284079e5b59830d9d36;p=ceph.git

journal: potential for double-free of context on shut down

The context associated with a scheduled watch might be freed
by two ObjectPlayers.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/journal/JournalPlayer.cc b/src/journal/JournalPlayer.cc
index 0244d9ad37da..aee87165d86a 100644
--- a/src/journal/JournalPlayer.cc
+++ b/src/journal/JournalPlayer.cc
@@ -552,9 +552,14 @@ void JournalPlayer::schedule_watch() {
 
 void JournalPlayer::handle_watch(int r) {
   ldout(m_cct, 10) << __func__ << ": r=" << r << dendl;
+  if (r == -ECANCELED) {
+    // unwatch of object player(s)
+    return;
+  }
 
   Mutex::Locker locker(m_lock);
   m_watch_scheduled = false;
+
   std::set<uint64_t> object_numbers;
   for (auto &players : m_object_players) {
     object_numbers.insert(
diff --git a/src/journal/JournalPlayer.h b/src/journal/JournalPlayer.h
index 476b49b5acb4..80a9ff7c1f08 100644
--- a/src/journal/JournalPlayer.h
+++ b/src/journal/JournalPlayer.h
@@ -70,24 +70,26 @@ private:
 
   struct C_Watch : public Context {
     JournalPlayer *player;
+    Mutex lock;
     uint8_t pending_fetches = 1;
     int ret_val = 0;
 
-    C_Watch(JournalPlayer *player) : player(player) {
+    C_Watch(JournalPlayer *player)
+      : player(player), lock("JournalPlayer::C_Watch::lock") {
     }
 
     virtual void complete(int r) override {
-      player->m_lock.Lock();
+      lock.Lock();
       if (ret_val == 0 && r < 0) {
         ret_val = r;
       }
 
       assert(pending_fetches > 0);
       if (--pending_fetches == 0) {
-        player->m_lock.Unlock();
+        lock.Unlock();
         Context::complete(ret_val);
       } else {
-        player->m_lock.Unlock();
+        lock.Unlock();
       }
     }
 
diff --git a/src/journal/ObjectPlayer.cc b/src/journal/ObjectPlayer.cc
index c299792f276e..b135d39bfac5 100644
--- a/src/journal/ObjectPlayer.cc
+++ b/src/journal/ObjectPlayer.cc
@@ -70,18 +70,20 @@ void ObjectPlayer::watch(Context *on_fetch, double interval) {
 
 void ObjectPlayer::unwatch() {
   ldout(m_cct, 20) << __func__ << ": " << m_oid << " unwatch" << dendl;
-  Mutex::Locker timer_locker(m_timer_lock);
+  Context *watch_ctx = nullptr;
+  {
+    Mutex::Locker timer_locker(m_timer_lock);
 
-  cancel_watch();
+    cancel_watch();
 
-  Context *watch_ctx = nullptr;
-  std::swap(watch_ctx, m_watch_ctx);
-  if (watch_ctx != nullptr) {
-    delete watch_ctx;
+    std::swap(watch_ctx, m_watch_ctx);
+    while (m_watch_in_progress) {
+      m_watch_in_progress_cond.Wait(m_timer_lock);
+    }
   }
 
-  while (m_watch_in_progress) {
-    m_watch_in_progress_cond.Wait(m_timer_lock);
+  if (watch_ctx != nullptr) {
+    watch_ctx->complete(-ECANCELED);
   }
 }