crimson/onode-staged-tree: validate node size where possible

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h b/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
index 9180ad868a083c231c885d74afb55f1022d532d0..7f835de474b08404fde4d220b4f2c055ef6fdd92 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
@@ -66,6 +66,11 @@ using node_offset_t = uint16_t;
 constexpr node_offset_t DISK_BLOCK_SIZE = 1u << 12;
 constexpr auto MAX_NODE_SIZE =
     (extent_len_t)std::numeric_limits<node_offset_t>::max() + 1;
+inline bool is_valid_node_size(extent_len_t node_size) {
+  return (node_size > 0 &&
+          node_size <= MAX_NODE_SIZE &&
+          node_size % DISK_BLOCK_SIZE == 0);
+}
 
 using string_size_t = uint16_t;
 

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
index b2e2d74990195277bfa18be8c44c4b401ad91979..afa695fa128bcbbd6ef0e138c9634796ef7056f9 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
@@ -83,7 +83,6 @@ class DeltaRecorderT final: public DeltaRecorder {
     ceph::encode(new_addr, encoded);
     int node_offset = reinterpret_cast<const char*>(p_addr) - p_node_start;
     assert(node_offset > 0 && node_offset < (int)node_size);
-    assert(node_offset < (int)MAX_NODE_SIZE);
     ceph::encode(static_cast<node_offset_t>(node_offset), encoded);
   }
 
@@ -287,6 +286,7 @@ class NodeExtentAccessorT {
       : extent{extent},
         node_stage{reinterpret_cast<const FieldType*>(extent->get_read()),
                    extent->get_length()} {
+    assert(is_valid_node_size(extent->get_length()));
     if (extent->is_initial_pending()) {
       state = nextent_state_t::FRESH;
       mut.emplace(extent->get_mutable());
@@ -314,7 +314,7 @@ class NodeExtentAccessorT {
     auto ref_recorder = recorder_t::create_for_replay();
     test_recorder = static_cast<recorder_t*>(ref_recorder.get());
     test_extent = TestReplayExtent::create(
-        extent->get_length(), std::move(ref_recorder));
+        get_length(), std::move(ref_recorder));
 #endif
   }
   ~NodeExtentAccessorT() = default;
@@ -325,7 +325,11 @@ class NodeExtentAccessorT {
 
   const node_stage_t& read() const { return node_stage; }
   laddr_t get_laddr() const { return extent->get_laddr(); }
-  extent_len_t get_length() const { return extent->get_length(); }
+  extent_len_t get_length() const {
+    auto len = extent->get_length();
+    assert(is_valid_node_size(len));
+    return len;
+  }
   nextent_state_t get_state() const {
     assert(!is_retired());
     // we cannot rely on the underlying extent state because
@@ -355,7 +359,7 @@ class NodeExtentAccessorT {
       state = nextent_state_t::MUTATION_PENDING;
       assert(extent->is_mutation_pending());
       node_stage = node_stage_t(reinterpret_cast<const FieldType*>(extent->get_read()),
-                                extent->get_length());
+                                get_length());
       assert(recorder == static_cast<recorder_t*>(extent->get_recorder()));
       mut.emplace(extent->get_mutable());
     }
@@ -500,7 +504,7 @@ class NodeExtentAccessorT {
 
   void test_copy_to(NodeExtentMutable& to) const {
     assert(extent->get_length() == to.get_length());
-    std::memcpy(to.get_write(), extent->get_read(), extent->get_length());
+    std::memcpy(to.get_write(), extent->get_read(), get_length());
   }
 
   eagain_future<NodeExtentMutable> rebuild(context_t c) {
@@ -512,7 +516,7 @@ class NodeExtentAccessorT {
       return eagain_ertr::make_ready_future<NodeExtentMutable>(*mut);
     }
     assert(!extent->is_initial_pending());
-    auto alloc_size = extent->get_length();
+    auto alloc_size = get_length();
     return c.nm.alloc_extent(c.t, alloc_size
     ).handle_error(
       eagain_ertr::pass_further{},
@@ -527,14 +531,14 @@ class NodeExtentAccessorT {
              c.t, extent->get_laddr(), fresh_extent->get_laddr());
       assert(fresh_extent->is_initial_pending());
       assert(fresh_extent->get_recorder() == nullptr);
-      assert(extent->get_length() == fresh_extent->get_length());
+      assert(get_length() == fresh_extent->get_length());
       auto fresh_mut = fresh_extent->get_mutable();
-      std::memcpy(fresh_mut.get_write(), extent->get_read(), extent->get_length());
+      std::memcpy(fresh_mut.get_write(), extent->get_read(), get_length());
       NodeExtentRef to_discard = extent;
 
       extent = fresh_extent;
       node_stage = node_stage_t(reinterpret_cast<const FieldType*>(extent->get_read()),
-                                extent->get_length());
+                                get_length());
       state = nextent_state_t::FRESH;
       mut.emplace(fresh_mut);
       recorder = nullptr;

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h
index 5082361740fda27a300b62248a715362db59094c..6f92ca9edf389420d12ab259acde5b745b6d9177 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h
@@ -68,7 +68,14 @@ class NodeExtentMutable {
 
   const char* get_read() const { return p_start; }
   char* get_write() { return p_start; }
-  extent_len_t get_length() const { return length; }
+  extent_len_t get_length() const {
+#ifndef NDEBUG
+    if (node_offset == 0) {
+      assert(is_valid_node_size(length));
+    }
+#endif
+    return length;
+  }
   node_offset_t get_node_offset() const { return node_offset; }
 
   NodeExtentMutable get_mutable_absolute(const void* dst, node_offset_t len) const {
@@ -77,6 +84,7 @@ class NodeExtentMutable {
     assert((const char*)dst != get_read());
     auto ret = *this;
     node_offset_t offset = (const char*)dst - get_read();
+    assert(offset != 0);
     ret.p_start += offset;
     ret.length = len;
     ret.node_offset = offset;

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/item_iterator_stage.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/item_iterator_stage.h
index 8def28b26fa953bcf45e4611cbb23a08d5c4e983..248ed606537aa125838cb5ab824512a515619d90 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/item_iterator_stage.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/item_iterator_stage.h
@@ -42,6 +42,7 @@ class item_iterator_t {
       : node_size{range.node_size},
         p_items_start(range.range.p_start),
         p_items_end(range.range.p_end) {
+    assert(is_valid_node_size(node_size));
     assert(p_items_start < p_items_end);
     next_item_range(p_items_end);
   }

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/node_stage.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/node_stage.h
index 5e05c678bbe0e6d5fed75e285be7ca190df086a2..e10577f06db869772978d8f6363bcc80f40d5459 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/node_stage.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/node_stage.h
@@ -36,8 +36,7 @@ class node_extent_t {
 
   node_extent_t(const FieldType* p_fields, extent_len_t node_size)
       : p_fields{p_fields}, node_size{node_size} {
-    assert(node_size <= MAX_NODE_SIZE);
-    assert(node_size % DISK_BLOCK_SIZE == 0);
+    assert(is_valid_node_size(node_size));
     validate(*p_fields);
   }
 

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/sub_items_stage.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/sub_items_stage.h
index f2b791ed5a7d216abe1ac0f2706d372f5ea1d4fb..1e01c033185fbb3433ef8491980ce917afb0044a 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/sub_items_stage.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/sub_items_stage.h
@@ -44,6 +44,7 @@ class internal_sub_items_t {
 
   internal_sub_items_t(const container_range_t& _range)
       : node_size{_range.node_size} {
+    assert(is_valid_node_size(node_size));
     auto& range = _range.range;
     assert(range.p_start < range.p_end);
     assert((range.p_end - range.p_start) % sizeof(internal_sub_item_t) == 0);
@@ -170,6 +171,7 @@ class leaf_sub_items_t {
 
   leaf_sub_items_t(const container_range_t& _range)
       : node_size{_range.node_size} {
+    assert(is_valid_node_size(node_size));
     auto& range = _range.range;
     assert(range.p_start < range.p_end);
     auto _p_num_keys = range.p_end - sizeof(num_keys_t);

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
index bfaba92b4dcd9453be3620f7a9b7639134aee7f1..2b4ec61c8ef9594874dbc45620b2f40c31a24972 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
@@ -111,10 +111,8 @@ void validate_tree_config(const tree_conf_t& conf)
               string_key_view_t::VALID_UPPER_BOUND);
   ceph_assert(conf.max_oid_size <
               string_key_view_t::VALID_UPPER_BOUND);
-  ceph_assert(conf.internal_node_size <= MAX_NODE_SIZE);
-  ceph_assert(conf.internal_node_size % DISK_BLOCK_SIZE == 0);
-  ceph_assert(conf.leaf_node_size <= MAX_NODE_SIZE);
-  ceph_assert(conf.leaf_node_size % DISK_BLOCK_SIZE == 0);
+  ceph_assert(is_valid_node_size(conf.internal_node_size));
+  ceph_assert(is_valid_node_size(conf.leaf_node_size));
 
   if (conf.do_split_check) {
     // In hope to comply with 3 * (oid + ns) + 2 * value < node