os/bluestore: Added asserts for allocator regions

Functions release/init_add_free/init_rm_free did not check its input against device size.
It is incorrect and had been a problem when you shrink device.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
(cherry picked from commit 9bdb2e3c3105bec2041f282fb7d5104f7d69a8c4)

diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc
index f92821f0c853beb4c696742f0a59d655b9d40a31..e132cd49442f5c5966f28c272d384ff64dbee769 100644 (file)
--- a/src/os/bluestore/Allocator.cc
+++ b/src/os/bluestore/Allocator.cc
@@ -109,7 +109,7 @@ public:
 Allocator::Allocator(const std::string& name,
                      int64_t _capacity,
                      int64_t _block_size)
-  : capacity(_capacity), block_size(_block_size)
+  : device_size(_capacity), block_size(_block_size)
 {
   asok_hook = new SocketHook(this, name);
 }
@@ -129,7 +129,7 @@ Allocator *Allocator::create(CephContext* cct, string type,
 {
   Allocator* alloc = nullptr;
   if (type == "stupid") {
-    alloc = new StupidAllocator(cct, name, size, block_size);
+    alloc = new StupidAllocator(cct, size, block_size, name);
   } else if (type == "bitmap") {
     alloc = new BitmapAllocator(cct, size, block_size, name);
   } else if (type == "avl") {

diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h
index 38ea4f997388b1a963cee076c85b12ad4ecfcdbe..915e9db0ade8c35af2822e8ef4094bf9a99e8643 100644 (file)
--- a/src/os/bluestore/Allocator.h
+++ b/src/os/bluestore/Allocator.h
@@ -80,7 +80,7 @@ public:
   const string& get_name() const;
   int64_t get_capacity() const
   {
-    return capacity;
+    return device_size;
   }
   int64_t get_block_size() const
   {
@@ -90,9 +90,9 @@ public:
 private:
   class SocketHook;
   SocketHook* asok_hook = nullptr;
-
-  int64_t capacity = 0;
-  int64_t block_size = 0;
+protected:
+  const int64_t device_size = 0;
+  const int64_t block_size = 0;
 };
 
 #endif

diff --git a/src/os/bluestore/AvlAllocator.cc b/src/os/bluestore/AvlAllocator.cc
index 5f17a3689b18f21c905453d8652d8ce0ff524d2e..9152c0e32a8f57cf1677aea341c7dea07080587a 100644 (file)
--- a/src/os/bluestore/AvlAllocator.cc
+++ b/src/os/bluestore/AvlAllocator.cc
@@ -304,6 +304,7 @@ void AvlAllocator::_release(const interval_set<uint64_t>& release_set)
   for (auto p = release_set.begin(); p != release_set.end(); ++p) {
     const auto offset = p.get_start();
     const auto length = p.get_len();
+    ceph_assert(offset + length <= uint64_t(num_total));
     ldout(cct, 10) << __func__ << std::hex
       << " offset 0x" << offset
       << " length 0x" << length
@@ -448,6 +449,7 @@ void AvlAllocator::init_add_free(uint64_t offset, uint64_t length)
   if (!length)
     return;
   std::lock_guard l(lock);
+  ceph_assert(offset + length <= uint64_t(num_total));
   ldout(cct, 10) << __func__ << std::hex
                  << " offset 0x" << offset
                  << " length 0x" << length
@@ -460,6 +462,7 @@ void AvlAllocator::init_rm_free(uint64_t offset, uint64_t length)
   if (!length)
     return;
   std::lock_guard l(lock);
+  ceph_assert(offset + length <= uint64_t(num_total));
   ldout(cct, 10) << __func__ << std::hex
                  << " offset 0x" << offset
                  << " length 0x" << length

diff --git a/src/os/bluestore/BitmapAllocator.cc b/src/os/bluestore/BitmapAllocator.cc
index 0e15c41729fe6542c4889292941e71452e30d680..5a3967f5e625dc4ef996a0331188882edc0de692 100644 (file)
--- a/src/os/bluestore/BitmapAllocator.cc
+++ b/src/os/bluestore/BitmapAllocator.cc
@@ -52,9 +52,10 @@ void BitmapAllocator::release(
   const interval_set<uint64_t>& release_set)
 {
   if (cct->_conf->subsys.should_gather<dout_subsys, 10>()) {
-    for (auto r : release_set) {
-      ldout(cct, 10) << __func__ << " 0x" << std::hex << r.first << "~" << r.second
-                   << std::dec << dendl;
+    for (auto& [offset, len] : release_set) {
+      ldout(cct, 10) << __func__ << " 0x" << std::hex << offset << "~" << len
+                     << std::dec << dendl;
+      ceph_assert(offset + len <= (uint64_t)device_size);
     }
   }
   _free_l2(release_set);
@@ -70,6 +71,7 @@ void BitmapAllocator::init_add_free(uint64_t offset, uint64_t length)
   auto mas = get_min_alloc_size();
   uint64_t offs = round_up_to(offset, mas);
   uint64_t l = p2align(offset + length - offs, mas);
+  ceph_assert(offs + l <= (uint64_t)device_size);
 
   _mark_free(offs, l);
   ldout(cct, 10) << __func__ << " done" << dendl;
@@ -81,6 +83,7 @@ void BitmapAllocator::init_rm_free(uint64_t offset, uint64_t length)
   auto mas = get_min_alloc_size();
   uint64_t offs = round_up_to(offset, mas);
   uint64_t l = p2align(offset + length - offs, mas);
+  ceph_assert(offs + l <= (uint64_t)device_size);
   _mark_allocated(offs, l);
   ldout(cct, 10) << __func__ << " done" << dendl;
 }

diff --git a/src/os/bluestore/BitmapAllocator.h b/src/os/bluestore/BitmapAllocator.h
index bb6fa73a1a53a4af6d1b8bb80af3c5f5545bb224..90bcd956f11c868648e437eaf37404ad45bcf4d4 100644 (file)
--- a/src/os/bluestore/BitmapAllocator.h
+++ b/src/os/bluestore/BitmapAllocator.h
@@ -15,7 +15,6 @@
 class BitmapAllocator : public Allocator,
   public AllocatorLevel02<AllocatorLevel01Loose> {
   CephContext* cct;
-
 public:
   BitmapAllocator(CephContext* _cct, int64_t capacity, int64_t alloc_unit, const std::string& name);
   ~BitmapAllocator() override

diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc
index 805a51fb090bed07722aa61b37475e56c78d06c9..2c0ca7a43119207fc3929dc16f1d1a6444220c2d 100644 (file)
--- a/src/os/bluestore/StupidAllocator.cc
+++ b/src/os/bluestore/StupidAllocator.cc
@@ -11,14 +11,15 @@
 #define dout_prefix *_dout << "stupidalloc 0x" << this << " "
 
 StupidAllocator::StupidAllocator(CephContext* cct,
-                                 const std::string& name,
-                                 int64_t _size,
-                                 int64_t _block_size)
-  : Allocator(name, _size, _block_size), cct(cct), num_free(0),
+                                 int64_t capacity,
+                                 int64_t _block_size,
+                                 const std::string& name)
+  : Allocator(name, capacity, _block_size),
+    cct(cct), num_free(0),
     free(10)
 {
   ceph_assert(cct != nullptr);
-  bdev_block_size = cct->_conf->bdev_block_size;
+  ceph_assert(block_size > 0);
 }
 
 StupidAllocator::~StupidAllocator()
@@ -27,8 +28,7 @@ StupidAllocator::~StupidAllocator()
 
 unsigned StupidAllocator::_choose_bin(uint64_t orig_len)
 {
-  ceph_assert(bdev_block_size > 0);
-  uint64_t len = orig_len / bdev_block_size;
+  uint64_t len = orig_len / block_size;
   int bin = std::min((int)cbits(len), (int)free.size() - 1);
   ldout(cct, 30) << __func__ << " len 0x" << std::hex << orig_len
                 << std::dec << " -> " << bin << dendl;

diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h
index e1fc101e0879f7bf1e64f273489c01c00b5e2d09..1ad377a39066f8f1cd68b421080fd2be7d5b3092 100644 (file)
--- a/src/os/bluestore/StupidAllocator.h
+++ b/src/os/bluestore/StupidAllocator.h
@@ -18,7 +18,6 @@ class StupidAllocator : public Allocator {
   ceph::mutex lock = ceph::make_mutex("StupidAllocator::lock");
 
   int64_t num_free;     ///< total bytes in freelist
-  uint64_t bdev_block_size;
 
   template <typename K, typename V> using allocator_t =
     mempool::bluestore_alloc::pool_allocator<std::pair<const K, V>>;
@@ -38,9 +37,9 @@ class StupidAllocator : public Allocator {
 
 public:
   StupidAllocator(CephContext* cct,
-                  const std::string& name,
                   int64_t size,
-                  int64_t block_size);
+                  int64_t block_size,
+                 const std::string& name);
   ~StupidAllocator() override;
   const char* get_type() const override
   {