From e76cea690a6f67a09f8b8174d81723b06903657b Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Mon, 22 Oct 2018 14:38:48 -0500
Subject: [PATCH] os/bluestore: fix race between SharedBlobSet::lookup and
 SharedBlob::put

A                             B
SharedBlobSet::lookup()
  takes lock
  nref is not 0
                              SharedBlob::put()
                                --nref
returns SharedBlobRef,
  ++nref
                                takes cache lock
                                SharedBlobSet::remove
                                  takes lock
                                  removes
                                deletes SharedBlob

-> A ends up with a ref to deleted SharedBlob

Fix by verifying that nref is still zero in SharedBlobSet::remove(),
while we are holding the SharedBlobSet::lock.  The lock ensures that we
have increased the ref for the lookup before entering remove, so we can
verify that nref is still zero before removing it.  If not, we have
raced, and put() bails out and does nothing.

Fixes: http://tracker.ceph.com/issues/36526
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 020bd7b5f38a82d9eef5e25e6f4a4dd12b066915)

Conflicts:
	src/os/bluestore/BlueStore.h
---
 src/os/bluestore/BlueStore.cc | 6 ++++--
 src/os/bluestore/BlueStore.h  | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 0c4fbc0bf9e..e3b4105e2c2 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1677,8 +1677,10 @@ void BlueStore::SharedBlob::put()
       if (coll_snap != coll) {
 	goto again;
       }
-      coll_snap->shared_blob_set.remove(this);
-
+      if (!coll_snap->shared_blob_set.remove(this, true)) {
+	// race with lookup
+	return;
+      }
       bc._clear(coll_snap->cache);
       coll_snap->cache->rm_blob();
     }
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 01e436cdcf4..216c914e35c 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -457,15 +457,19 @@ public:
       sb->coll = coll;
     }
 
-    void remove(SharedBlob *sb) {
+    bool remove(SharedBlob *sb, bool verify_nref_is_zero=false) {
       std::lock_guard<std::mutex> l(lock);
       ceph_assert(sb->get_parent() == this);
+      if (verify_nref_is_zero && sb->nref != 0) {
+	return false;
+      }
       // only remove if it still points to us
       auto p = sb_map.find(sb->get_sbid());
       if (p != sb_map.end() &&
 	  p->second == sb) {
 	sb_map.erase(p);
       }
+      return true;
     }
 
     bool empty() {
-- 
2.47.3