]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
librbd: make diff-iterate in fast-diff mode aware of encryption 58345/head
authorIlya Dryomov <idryomov@gmail.com>
Thu, 20 Jun 2024 19:13:56 +0000 (21:13 +0200)
committerIlya Dryomov <idryomov@gmail.com>
Mon, 8 Jul 2024 14:59:03 +0000 (16:59 +0200)
diff-iterate wasn't updated when librbd was being prepared to support
encryption in commit 8d6a47933269 ("librbd: add crypto image dispatch
layer").  This is even noted in [1]:

> The two places I skipped for now are DiffIterate and TrimRequest.

CryptoImageDispatch has since been removed, but diff-iterate in
fast-diff mode is still unaware of encryption and just assumes that all
offsets are raw.  This means that the callback gets invoked with
incorrect image offsets when encryption is loaded.  For example, for
a LUKS1-formatted image with some data at offsets 0 and 20971520,
diff-iterate with encryption loaded reports

  0~4194304
  4194304~4194304
  25165824~4194304

instead of

  0~4194304
  20971520~4194304

as "exists".  For any piece of code that is using diff-iterate to
optimize block-by-block processing (e.g. copy an encrypted source image
to a differently-encrypted destination image), this is fatal: it would
skip processing block 20971520 which has data and instead process block
25165824 which doesn't have any data and was to be skipped, producing
a corrupted destination image.

[1] https://github.com/ceph/ceph/pull/37935#issue-735278403

Fixes: https://tracker.ceph.com/issues/66570
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit cdeb0efce3f9f857ad6d5b7ff3965f3292cb571a)

src/librbd/api/DiffIterate.cc
src/test/librbd/test_librbd.cc

index 8ddcabd7c0b231b96e7aa936c22c856df9c38f49..f7dd57504db42e43531d08df7f874fc3dc35183c 100644 (file)
@@ -10,6 +10,7 @@
 #include "librbd/internal.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/ImageDispatchSpec.h"
+#include "librbd/io/Utils.h"
 #include "librbd/object_map/DiffRequest.h"
 #include "include/rados/librados.hpp"
 #include "include/interval_set.h"
@@ -275,15 +276,15 @@ std::pair<uint64_t, uint64_t> DiffIterate<I>::calc_object_diff_range() {
   if (first_period_off != last_period_off) {
     // map only the tail of the first period and the front of the last
     // period instead of the entire range for efficiency
-    Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout,
-                             m_offset, first_period_off + period - m_offset,
-                             0, 0, &object_extents);
-    Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout,
-                        last_period_off, m_offset + m_length - last_period_off,
-                        0, 0, &object_extents);
+    io::util::area_to_object_extents(&m_image_ctx, m_offset,
+                                     first_period_off + period - m_offset,
+                                     io::ImageArea::DATA, 0, &object_extents);
+    io::util::area_to_object_extents(&m_image_ctx, last_period_off,
+                                     m_offset + m_length - last_period_off,
+                                     io::ImageArea::DATA, 0, &object_extents);
   } else {
-    Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout, m_offset,
-                             m_length, 0, 0, &object_extents);
+    io::util::area_to_object_extents(&m_image_ctx, m_offset, m_length,
+                                     io::ImageArea::DATA, 0, &object_extents);
   }
   return {object_extents.front().object_no, object_extents.back().object_no + 1};
 }
@@ -380,47 +381,43 @@ int DiffIterate<I>::execute() {
     uint64_t read_len = std::min(period_off + period - off, left);
 
     if (fast_diff_enabled) {
-      // map to extents
-      std::map<object_t,std::vector<ObjectExtent> > object_extents;
-      Striper::file_to_extents(cct, m_image_ctx.format_string,
-                               &m_image_ctx.layout, off, read_len, 0,
-                               object_extents, 0);
+      // map to objects (there would be one extent per object)
+      striper::LightweightObjectExtents object_extents;
+      io::util::area_to_object_extents(&m_image_ctx, off, read_len,
+                                       io::ImageArea::DATA, 0, &object_extents);
 
       // get diff info for each object and merge adjacent stripe units
       // into an aggregate (this also sorts them)
       io::SparseExtents aggregate_sparse_extents;
-      for (auto& [object, extents] : object_extents) {
-        const uint64_t object_no = extents.front().objectno;
-        ceph_assert(object_no >= start_object_no && object_no < end_object_no);
-        uint8_t diff_state = object_diff_state[object_no - start_object_no];
-        ldout(cct, 20) << "object " << object << ": diff_state="
-                       << (int)diff_state << dendl;
+      for (const auto& oe : object_extents) {
+        ceph_assert(oe.object_no >= start_object_no &&
+                    oe.object_no < end_object_no);
+        uint8_t diff_state = object_diff_state[oe.object_no - start_object_no];
+        ldout(cct, 20) << "object "
+                       << util::data_object_name(&m_image_ctx, oe.object_no)
+                       << ": diff_state=" << (int)diff_state << dendl;
 
         if (diff_state == object_map::DIFF_STATE_HOLE &&
             from_snap_id == 0 && !parent_diff.empty()) {
           // no data in child object -- report parent diff instead
-          for (auto& oe : extents) {
-            for (auto& be : oe.buffer_extents) {
-              interval_set<uint64_t> o;
-              o.insert(off + be.first, be.second);
-              o.intersection_of(parent_diff);
-              ldout(cct, 20) << " reporting parent overlap " << o << dendl;
-              for (auto e = o.begin(); e != o.end(); ++e) {
-                aggregate_sparse_extents.insert(e.get_start(), e.get_len(),
-                                                {io::SPARSE_EXTENT_STATE_DATA,
-                                                 e.get_len()});
-              }
+          for (const auto& be : oe.buffer_extents) {
+            interval_set<uint64_t> o;
+            o.insert(off + be.first, be.second);
+            o.intersection_of(parent_diff);
+            ldout(cct, 20) << " reporting parent overlap " << o << dendl;
+            for (auto e = o.begin(); e != o.end(); ++e) {
+              aggregate_sparse_extents.insert(e.get_start(), e.get_len(),
+                                              {io::SPARSE_EXTENT_STATE_DATA,
+                                               e.get_len()});
             }
           }
         } else if (diff_state == object_map::DIFF_STATE_HOLE_UPDATED ||
                    diff_state == object_map::DIFF_STATE_DATA_UPDATED) {
           auto state = (diff_state == object_map::DIFF_STATE_HOLE_UPDATED ?
               io::SPARSE_EXTENT_STATE_ZEROED : io::SPARSE_EXTENT_STATE_DATA);
-          for (auto& oe : extents) {
-            for (auto& be : oe.buffer_extents) {
-              aggregate_sparse_extents.insert(off + be.first, be.second,
-                                              {state, be.second});
-            }
+          for (const auto& be : oe.buffer_extents) {
+            aggregate_sparse_extents.insert(off + be.first, be.second,
+                                            {state, be.second});
           }
         }
       }
index a13f6d3ac84e9517ed2377cb85e137fd2ced108c..a5b33c80db338c4646e42dc4a2e0bd45207b6647 100644 (file)
@@ -3795,7 +3795,7 @@ TYPED_TEST(EncryptedFlattenTest, ZeroOverlap)
   }
 }
 
-#endif
+#endif // HAVE_LIBCRYPTSETUP
 
 TEST_F(TestLibRBD, TestIOWithIOHint)
 {
@@ -7487,6 +7487,82 @@ public:
     test_deterministic_pp(image, object_off, len, 1);
   }
 
+#ifdef HAVE_LIBCRYPTSETUP
+
+  void test_deterministic_luks1(uint64_t object_off, uint64_t len) {
+    rados_ioctx_t ioctx;
+    ASSERT_EQ(0, rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx));
+
+    rbd_image_t image;
+    int order = 22;
+    std::string name = this->get_temp_image_name();
+    ASSERT_EQ(0, create_image(ioctx, name.c_str(), 24 << 20, &order));
+    ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+    rbd_encryption_luks1_format_options_t fopts = {
+        RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase", 15};
+    ASSERT_EQ(0, rbd_encryption_format(image, RBD_ENCRYPTION_FORMAT_LUKS1,
+                                       &fopts, sizeof(fopts)));
+    test_deterministic(image, object_off, len, 512);
+
+    ASSERT_EQ(0, rbd_close(image));
+    rados_ioctx_destroy(ioctx);
+  }
+
+  void test_deterministic_luks1_pp(uint64_t object_off, uint64_t len) {
+    librados::IoCtx ioctx;
+    ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));
+
+    librbd::RBD rbd;
+    librbd::Image image;
+    int order = 22;
+    std::string name = this->get_temp_image_name();
+    ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), 24 << 20, &order));
+    ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
+    librbd::encryption_luks1_format_options_t fopts = {
+        RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase"};
+    ASSERT_EQ(0, image.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS1, &fopts,
+                                         sizeof(fopts)));
+    test_deterministic_pp(image, object_off, len, 512);
+  }
+
+  void test_deterministic_luks2(uint64_t object_off, uint64_t len) {
+    rados_ioctx_t ioctx;
+    ASSERT_EQ(0, rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx));
+
+    rbd_image_t image;
+    int order = 22;
+    std::string name = this->get_temp_image_name();
+    ASSERT_EQ(0, create_image(ioctx, name.c_str(), 36 << 20, &order));
+    ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+    rbd_encryption_luks2_format_options_t fopts = {
+        RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase", 15};
+    ASSERT_EQ(0, rbd_encryption_format(image, RBD_ENCRYPTION_FORMAT_LUKS2,
+                                       &fopts, sizeof(fopts)));
+    test_deterministic(image, object_off, len, 4096);
+
+    ASSERT_EQ(0, rbd_close(image));
+    rados_ioctx_destroy(ioctx);
+  }
+
+  void test_deterministic_luks2_pp(uint64_t object_off, uint64_t len) {
+    librados::IoCtx ioctx;
+    ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));
+
+    librbd::RBD rbd;
+    librbd::Image image;
+    int order = 22;
+    std::string name = this->get_temp_image_name();
+    ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), 36 << 20, &order));
+    ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
+    librbd::encryption_luks2_format_options_t fopts = {
+        RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase"};
+    ASSERT_EQ(0, image.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS2, &fopts,
+                                         sizeof(fopts)));
+    test_deterministic_pp(image, object_off, len, 4096);
+  }
+
+#endif // HAVE_LIBCRYPTSETUP
+
 private:
   void test_deterministic(rbd_image_t image, uint64_t object_off,
                           uint64_t len, uint64_t block_size) {
@@ -7894,6 +7970,58 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministicPP)
   EXPECT_NO_FATAL_FAILURE(this->test_deterministic_pp((4 << 20) - 2, 2));
 }
 
+#ifdef HAVE_LIBCRYPTSETUP
+
+TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS1)
+{
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
+
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1(0, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((1 << 20) - 256, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((1 << 20) - 128, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1(1 << 20, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((4 << 20) - 256, 256));
+}
+
+TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS1PP)
+{
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
+
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp(0, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((3 << 20) - 2, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((3 << 20) - 1, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp(3 << 20, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((4 << 20) - 2, 2));
+}
+
+TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS2)
+{
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
+
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2(0, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((1 << 20) - 256, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((1 << 20) - 128, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2(1 << 20, 256));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((4 << 20) - 256, 256));
+}
+
+TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS2PP)
+{
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
+  REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
+
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp(0, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((3 << 20) - 2, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((3 << 20) - 1, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp(3 << 20, 2));
+  EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((4 << 20) - 2, 2));
+}
+
+#endif // HAVE_LIBCRYPTSETUP
+
 TYPED_TEST(DiffIterateTest, DiffIterateDiscard)
 {
   REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));