From 860ddba0f00a1d0e60c7322bf18eace328065d4a Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Fri, 18 Jun 2021 16:33:00 +0800 Subject: [PATCH] crimson/onode-staged-tree: validate node header when load Add logs to detect corruptions when load nodes. assert() is not informative enough to understand the context. Signed-off-by: Yingxin Cheng --- .../onode_manager/staged-fltree/node.cc | 36 +++++++++++++++---- .../staged-fltree/node_extent_manager.cc | 12 ------- .../staged-fltree/node_extent_manager.h | 7 ++-- .../node_extent_manager/seastore.cc | 16 ++++++--- .../onode_manager/staged-fltree/node_impl.cc | 20 +++++------ .../onode_manager/staged-fltree/node_impl.h | 4 +-- .../onode_manager/staged-fltree/node_layout.h | 3 +- 7 files changed, 59 insertions(+), 39 deletions(-) diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc index 859e82e91689f..6eea71f4c0864 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc @@ -684,21 +684,40 @@ eagain_future> Node::load( c.t, addr, expect_is_level_tail); ceph_abort("fatal error"); }) - ).safe_then([FNAME, c, expect_is_level_tail](auto extent) { - auto [node_type, field_type] = extent->get_types(); + ).safe_then([FNAME, c, addr, expect_is_level_tail](auto extent) { + auto header = extent->get_header(); + auto field_type = header.get_field_type(); + if (!field_type) { + ERRORT("load addr={:x}, is_level_tail={} error, " + "got invalid header -- {}", + c.t, addr, expect_is_level_tail, extent); + ceph_abort("fatal error"); + } + if (header.get_is_level_tail() != expect_is_level_tail) { + ERRORT("load addr={:x}, is_level_tail={} error, " + "is_level_tail mismatch -- {}", + c.t, addr, expect_is_level_tail, extent); + ceph_abort("fatal error"); + } + + auto node_type = header.get_node_type(); if (node_type == node_type_t::LEAF) { if (extent->get_length() != c.vb.get_leaf_node_size()) { - ERRORT("leaf length mismatch -- {}", c.t, extent); + ERRORT("load addr={:x}, is_level_tail={} error, " + "leaf length mismatch -- {}", + c.t, addr, expect_is_level_tail, extent); ceph_abort("fatal error"); } - auto impl = LeafNodeImpl::load(extent, field_type, expect_is_level_tail); + auto impl = LeafNodeImpl::load(extent, *field_type); return Ref(new LeafNode(impl.get(), std::move(impl))); } else if (node_type == node_type_t::INTERNAL) { if (extent->get_length() != c.vb.get_internal_node_size()) { - ERRORT("internal length mismatch -- {}", c.t, extent); + ERRORT("load addr={:x}, is_level_tail={} error, " + "internal length mismatch -- {}", + c.t, addr, expect_is_level_tail, extent); ceph_abort("fatal error"); } - auto impl = InternalNodeImpl::load(extent, field_type, expect_is_level_tail); + auto impl = InternalNodeImpl::load(extent, *field_type); return Ref(new InternalNode(impl.get(), std::move(impl))); } else { ceph_abort("impossible path"); @@ -1518,6 +1537,11 @@ eagain_future> InternalNode::get_or_track_child( ).safe_then([this, position, c, FNAME] (auto child) { TRACET("loaded child untracked {}", c.t, child->get_name()); + if (child->level() + 1 != level()) { + ERRORT("loaded child {} error from parent {} at pos({}), level mismatch", + c.t, child->get_name(), get_name(), position); + ceph_abort("fatal error"); + } child->as_child(position, this); return child; }); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc index e0c83bf524e55..e8f6fbe5e54ac 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc @@ -5,21 +5,9 @@ #include "node_extent_manager/dummy.h" #include "node_extent_manager/seastore.h" -#include "stages/node_stage_layout.h" namespace crimson::os::seastore::onode { -std::pair NodeExtent::get_types() const -{ - const auto header = reinterpret_cast(get_read()); - auto node_type = header->get_node_type(); - auto field_type = header->get_field_type(); - if (!field_type.has_value()) { - throw std::runtime_error("load failed: bad field type"); - } - return {node_type, *field_type}; -} - NodeExtentManagerURef NodeExtentManager::create_dummy(bool is_sync) { if (is_sync) { diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h index ac3b7179cf8b5..924d6ee8770a1 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h @@ -8,9 +8,10 @@ #include "crimson/os/seastore/transaction_manager.h" #include "fwd.h" -#include "super.h" #include "node_extent_mutable.h" #include "node_types.h" +#include "stages/node_stage_layout.h" +#include "super.h" /** * node_extent_manager.h @@ -24,7 +25,9 @@ using crimson::os::seastore::LogicalCachedExtent; class NodeExtent : public LogicalCachedExtent { public: virtual ~NodeExtent() = default; - std::pair get_types() const; + const node_header_t& get_header() const { + return *reinterpret_cast(get_read()); + } const char* get_read() const { return get_bptr().c_str(); } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc index 528acb8f13157..ba6eb63d7cfdf 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc @@ -62,13 +62,19 @@ void SeastoreNodeExtent::apply_delta(const ceph::bufferlist& bl) { DEBUG("replay {:#x} ...", get_laddr()); if (!recorder) { - auto [node_type, field_type] = get_types(); - recorder = create_replay_recorder(node_type, field_type); + auto header = get_header(); + auto field_type = header.get_field_type(); + if (!field_type.has_value()) { + ERROR("replay got invalid node -- {}", *this); + ceph_abort("fatal error"); + } + auto node_type = header.get_node_type(); + recorder = create_replay_recorder(node_type, *field_type); } else { #ifndef NDEBUG - auto [node_type, field_type] = get_types(); - assert(recorder->node_type() == node_type); - assert(recorder->field_type() == field_type); + auto header = get_header(); + assert(recorder->node_type() == header.get_node_type()); + assert(recorder->field_type() == *header.get_field_type()); #endif } auto node = do_get_mutable(); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc index e8fc618da4498..219dbb861374c 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc @@ -46,32 +46,32 @@ LeafNodeImpl::allocate( } InternalNodeImplURef InternalNodeImpl::load( - NodeExtentRef extent, field_type_t type, bool expect_is_level_tail) + NodeExtentRef extent, field_type_t type) { if (type == field_type_t::N0) { - return InternalNode0::load(extent, expect_is_level_tail); + return InternalNode0::load(extent); } else if (type == field_type_t::N1) { - return InternalNode1::load(extent, expect_is_level_tail); + return InternalNode1::load(extent); } else if (type == field_type_t::N2) { - return InternalNode2::load(extent, expect_is_level_tail); + return InternalNode2::load(extent); } else if (type == field_type_t::N3) { - return InternalNode3::load(extent, expect_is_level_tail); + return InternalNode3::load(extent); } else { ceph_abort("impossible path"); } } LeafNodeImplURef LeafNodeImpl::load( - NodeExtentRef extent, field_type_t type, bool expect_is_level_tail) + NodeExtentRef extent, field_type_t type) { if (type == field_type_t::N0) { - return LeafNode0::load(extent, expect_is_level_tail); + return LeafNode0::load(extent); } else if (type == field_type_t::N1) { - return LeafNode1::load(extent, expect_is_level_tail); + return LeafNode1::load(extent); } else if (type == field_type_t::N2) { - return LeafNode2::load(extent, expect_is_level_tail); + return LeafNode2::load(extent); } else if (type == field_type_t::N3) { - return LeafNode3::load(extent, expect_is_level_tail); + return LeafNode3::load(extent); } else { ceph_abort("impossible path"); } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h index 9a0ad143e22ba..94655686faef9 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h @@ -181,7 +181,7 @@ class InternalNodeImpl : public NodeImpl { }; static eagain_future allocate(context_t, field_type_t, bool, level_t); - static InternalNodeImplURef load(NodeExtentRef, field_type_t, bool); + static InternalNodeImplURef load(NodeExtentRef, field_type_t); protected: InternalNodeImpl() = default; @@ -261,7 +261,7 @@ class LeafNodeImpl : public NodeImpl { }; static eagain_future allocate(context_t, field_type_t, bool); - static LeafNodeImplURef load(NodeExtentRef, field_type_t, bool); + static LeafNodeImplURef load(NodeExtentRef, field_type_t); protected: LeafNodeImpl() = default; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h index fadb4a509b0cd..d474e86954c16 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h @@ -60,9 +60,8 @@ class NodeLayoutT final : public InternalNodeImpl, public LeafNodeImpl { NodeLayoutT& operator=(NodeLayoutT&&) = delete; ~NodeLayoutT() override = default; - static URef load(NodeExtentRef extent, bool expect_is_level_tail) { + static URef load(NodeExtentRef extent) { std::unique_ptr ret(new NodeLayoutT(extent)); - assert(ret->is_level_tail() == expect_is_level_tail); return ret; } -- 2.39.5