From d63ace5628ed3db9bc406efde1a4172c66e5eb6c Mon Sep 17 00:00:00 2001
From: Samuel Just <sjust@redhat.com>
Date: Fri, 20 Nov 2020 19:07:36 -0800
Subject: [PATCH] crimson/os/seastore/journal: validate header crc on read

Signed-off-by: Samuel Just <sjust@redhat.com>
---
 src/crimson/os/seastore/journal.cc | 49 ++++++++++++++++++++----------
 src/crimson/os/seastore/journal.h  | 30 ++++++++++++------
 2 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc
index 06cc5a888df..5a4b132f952 100644
--- a/src/crimson/os/seastore/journal.cc
+++ b/src/crimson/os/seastore/journal.cc
@@ -404,19 +404,20 @@ Journal::find_replay_segments_fut Journal::find_replay_segments()
     });
 }
 
-Journal::read_record_metadata_ret Journal::read_record_metadata(
-  paddr_t start)
+Journal::read_validate_record_metadata_ret Journal::read_validate_record_metadata(
+  paddr_t start,
+  segment_nonce_t nonce)
 {
   if (start.offset + block_size > (int64_t)segment_manager.get_segment_size()) {
-    return read_record_metadata_ret(
-      read_record_metadata_ertr::ready_future_marker{},
+    return read_validate_record_metadata_ret(
+      read_validate_record_metadata_ertr::ready_future_marker{},
       std::nullopt);
   }
   return segment_manager.read(start, block_size
   ).safe_then(
-    [this, start](bufferptr bptr) mutable
-    -> read_record_metadata_ret {
-      logger().debug("read_record_metadata: reading {}", start);
+    [=](bufferptr bptr) mutable
+    -> read_validate_record_metadata_ret {
+      logger().debug("read_validate_record_metadata: reading {}", start);
       bufferlist bl;
       bl.append(bptr);
       auto bp = bl.cbegin();
@@ -424,8 +425,13 @@ Journal::read_record_metadata_ret Journal::read_record_metadata(
       try {
 	decode(header, bp);
       } catch (ceph::buffer::error &e) {
-	return read_record_metadata_ret(
-	  read_record_metadata_ertr::ready_future_marker{},
+	return read_validate_record_metadata_ret(
+	  read_validate_record_metadata_ertr::ready_future_marker{},
+	  std::nullopt);
+      }
+      if (header.segment_nonce != nonce) {
+	return read_validate_record_metadata_ret(
+	  read_validate_record_metadata_ertr::ready_future_marker{},
 	  std::nullopt);
       }
       if (header.mdlength > block_size) {
@@ -439,15 +445,26 @@ Journal::read_record_metadata_ret Journal::read_record_metadata(
 	    [header=std::move(header), bl=std::move(bl)](
 	      auto &&bptail) mutable {
 	      bl.push_back(bptail);
-	      return read_record_metadata_ret(
-		read_record_metadata_ertr::ready_future_marker{},
+	      return read_validate_record_metadata_ret(
+		read_validate_record_metadata_ertr::ready_future_marker{},
 		std::make_pair(std::move(header), std::move(bl)));
 	    });
       } else {
-	  return read_record_metadata_ret(
-	    read_record_metadata_ertr::ready_future_marker{},
-	    std::make_pair(std::move(header), std::move(bl))
-	  );
+	return read_validate_record_metadata_ret(
+	  read_validate_record_metadata_ertr::ready_future_marker{},
+	  std::make_pair(std::move(header), std::move(bl))
+	);
+      }
+    }).safe_then([=](auto p) {
+      if (p && validate_metadata(p->second)) {
+	return read_validate_record_metadata_ret(
+	  read_validate_record_metadata_ertr::ready_future_marker{},
+	  std::move(*p)
+	);
+      } else {
+	return read_validate_record_metadata_ret(
+	  read_validate_record_metadata_ertr::ready_future_marker{},
+	  std::nullopt);
       }
     });
 }
@@ -594,7 +611,7 @@ Journal::scan_segment_ret Journal::scan_segment(
     [=](paddr_t &current) {
       return crimson::do_until(
 	[=, &current]() -> scan_segment_ertr::future<bool> {
-	  return read_record_metadata(current).safe_then
+	  return read_validate_record_metadata(current, nonce).safe_then
 	    ([=, &current](auto p)
 	     -> scan_segment_ertr::future<bool> {
 	      if (!p.has_value()) {
diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h
index b8fe1351a2b..2a55fc2fcd0 100644
--- a/src/crimson/os/seastore/journal.h
+++ b/src/crimson/os/seastore/journal.h
@@ -272,9 +272,19 @@ private:
     record_size_t rsize,
     record_t &&record);
 
-  /// validate metadata
+  /// validate embedded metadata checksum
   static bool validate_metadata(const bufferlist &bl);
 
+  /// read and validate data
+  using read_validate_data_ertr = SegmentManager::read_ertr;
+  using read_validate_data_ret = read_validate_data_ertr::future<bool>;
+  read_validate_data_ret read_validate_data(
+    paddr_t record_base,
+    const record_header_t &header  ///< caller must ensure lifetime through
+                                   ///  future resolution
+  );
+
+
   /// do record write
   using write_record_ertr = crimson::errorator<
     crimson::ct_error::input_output_error>;
@@ -310,14 +320,6 @@ private:
     replay_segments_t>;
   find_replay_segments_fut find_replay_segments();
 
-  /// read record metadata for record starting at start
-  using read_record_metadata_ertr = replay_ertr;
-  using read_record_metadata_ret = read_record_metadata_ertr::future<
-    std::optional<std::pair<record_header_t, bufferlist>>
-    >;
-  read_record_metadata_ret read_record_metadata(
-    paddr_t start);
-
   /// attempts to decode deltas from bl, return nullopt if unsuccessful
   std::optional<std::vector<delta_info_t>> try_decode_deltas(
     record_header_t header,
@@ -328,6 +330,16 @@ private:
     record_header_t header,
     bufferlist &bl);
 
+  /// read record metadata for record starting at start
+  using read_validate_record_metadata_ertr = replay_ertr;
+  using read_validate_record_metadata_ret =
+    read_validate_record_metadata_ertr::future<
+      std::optional<std::pair<record_header_t, bufferlist>>
+    >;
+  read_validate_record_metadata_ret read_validate_record_metadata(
+    paddr_t start,
+    segment_nonce_t nonce);
+
   /**
    * scan_segment
    *
-- 
2.39.5