Skip to content

Commit cdeb0ef

Browse files
committed
librbd: make diff-iterate in fast-diff mode aware of encryption
diff-iterate wasn't updated when librbd was being prepared to support encryption in commit 8d6a479 ("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] ceph#37935 (comment) Fixes: https://tracker.ceph.com/issues/66570 Signed-off-by: Ilya Dryomov <[email protected]>
1 parent da69d1f commit cdeb0ef

File tree

2 files changed

+161
-36
lines changed

2 files changed

+161
-36
lines changed

src/librbd/api/DiffIterate.cc

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "librbd/internal.h"
1111
#include "librbd/io/AioCompletion.h"
1212
#include "librbd/io/ImageDispatchSpec.h"
13+
#include "librbd/io/Utils.h"
1314
#include "librbd/object_map/DiffRequest.h"
1415
#include "include/rados/librados.hpp"
1516
#include "include/interval_set.h"
@@ -275,15 +276,15 @@ std::pair<uint64_t, uint64_t> DiffIterate<I>::calc_object_diff_range() {
275276
if (first_period_off != last_period_off) {
276277
// map only the tail of the first period and the front of the last
277278
// period instead of the entire range for efficiency
278-
Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout,
279-
m_offset, first_period_off + period - m_offset,
280-
0, 0, &object_extents);
281-
Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout,
282-
last_period_off, m_offset + m_length - last_period_off,
283-
0, 0, &object_extents);
279+
io::util::area_to_object_extents(&m_image_ctx, m_offset,
280+
first_period_off + period - m_offset,
281+
io::ImageArea::DATA, 0, &object_extents);
282+
io::util::area_to_object_extents(&m_image_ctx, last_period_off,
283+
m_offset + m_length - last_period_off,
284+
io::ImageArea::DATA, 0, &object_extents);
284285
} else {
285-
Striper::file_to_extents(m_image_ctx.cct, &m_image_ctx.layout, m_offset,
286-
m_length, 0, 0, &object_extents);
286+
io::util::area_to_object_extents(&m_image_ctx, m_offset, m_length,
287+
io::ImageArea::DATA, 0, &object_extents);
287288
}
288289
return {object_extents.front().object_no, object_extents.back().object_no + 1};
289290
}
@@ -380,47 +381,43 @@ int DiffIterate<I>::execute() {
380381
uint64_t read_len = std::min(period_off + period - off, left);
381382

382383
if (fast_diff_enabled) {
383-
// map to extents
384-
std::map<object_t,std::vector<ObjectExtent> > object_extents;
385-
Striper::file_to_extents(cct, m_image_ctx.format_string,
386-
&m_image_ctx.layout, off, read_len, 0,
387-
object_extents, 0);
384+
// map to objects (there would be one extent per object)
385+
striper::LightweightObjectExtents object_extents;
386+
io::util::area_to_object_extents(&m_image_ctx, off, read_len,
387+
io::ImageArea::DATA, 0, &object_extents);
388388

389389
// get diff info for each object and merge adjacent stripe units
390390
// into an aggregate (this also sorts them)
391391
io::SparseExtents aggregate_sparse_extents;
392-
for (auto& [object, extents] : object_extents) {
393-
const uint64_t object_no = extents.front().objectno;
394-
ceph_assert(object_no >= start_object_no && object_no < end_object_no);
395-
uint8_t diff_state = object_diff_state[object_no - start_object_no];
396-
ldout(cct, 20) << "object " << object << ": diff_state="
397-
<< (int)diff_state << dendl;
392+
for (const auto& oe : object_extents) {
393+
ceph_assert(oe.object_no >= start_object_no &&
394+
oe.object_no < end_object_no);
395+
uint8_t diff_state = object_diff_state[oe.object_no - start_object_no];
396+
ldout(cct, 20) << "object "
397+
<< util::data_object_name(&m_image_ctx, oe.object_no)
398+
<< ": diff_state=" << (int)diff_state << dendl;
398399

399400
if (diff_state == object_map::DIFF_STATE_HOLE &&
400401
from_snap_id == 0 && !parent_diff.empty()) {
401402
// no data in child object -- report parent diff instead
402-
for (auto& oe : extents) {
403-
for (auto& be : oe.buffer_extents) {
404-
interval_set<uint64_t> o;
405-
o.insert(off + be.first, be.second);
406-
o.intersection_of(parent_diff);
407-
ldout(cct, 20) << " reporting parent overlap " << o << dendl;
408-
for (auto e = o.begin(); e != o.end(); ++e) {
409-
aggregate_sparse_extents.insert(e.get_start(), e.get_len(),
410-
{io::SPARSE_EXTENT_STATE_DATA,
411-
e.get_len()});
412-
}
403+
for (const auto& be : oe.buffer_extents) {
404+
interval_set<uint64_t> o;
405+
o.insert(off + be.first, be.second);
406+
o.intersection_of(parent_diff);
407+
ldout(cct, 20) << " reporting parent overlap " << o << dendl;
408+
for (auto e = o.begin(); e != o.end(); ++e) {
409+
aggregate_sparse_extents.insert(e.get_start(), e.get_len(),
410+
{io::SPARSE_EXTENT_STATE_DATA,
411+
e.get_len()});
413412
}
414413
}
415414
} else if (diff_state == object_map::DIFF_STATE_HOLE_UPDATED ||
416415
diff_state == object_map::DIFF_STATE_DATA_UPDATED) {
417416
auto state = (diff_state == object_map::DIFF_STATE_HOLE_UPDATED ?
418417
io::SPARSE_EXTENT_STATE_ZEROED : io::SPARSE_EXTENT_STATE_DATA);
419-
for (auto& oe : extents) {
420-
for (auto& be : oe.buffer_extents) {
421-
aggregate_sparse_extents.insert(off + be.first, be.second,
422-
{state, be.second});
423-
}
418+
for (const auto& be : oe.buffer_extents) {
419+
aggregate_sparse_extents.insert(off + be.first, be.second,
420+
{state, be.second});
424421
}
425422
}
426423
}

src/test/librbd/test_librbd.cc

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3795,7 +3795,7 @@ TYPED_TEST(EncryptedFlattenTest, ZeroOverlap)
37953795
}
37963796
}
37973797

3798-
#endif
3798+
#endif // HAVE_LIBCRYPTSETUP
37993799

38003800
TEST_F(TestLibRBD, TestIOWithIOHint)
38013801
{
@@ -7487,6 +7487,82 @@ class DiffIterateTest : public TestLibRBD {
74877487
test_deterministic_pp(image, object_off, len, 1);
74887488
}
74897489

7490+
#ifdef HAVE_LIBCRYPTSETUP
7491+
7492+
void test_deterministic_luks1(uint64_t object_off, uint64_t len) {
7493+
rados_ioctx_t ioctx;
7494+
ASSERT_EQ(0, rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx));
7495+
7496+
rbd_image_t image;
7497+
int order = 22;
7498+
std::string name = this->get_temp_image_name();
7499+
ASSERT_EQ(0, create_image(ioctx, name.c_str(), 24 << 20, &order));
7500+
ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
7501+
rbd_encryption_luks1_format_options_t fopts = {
7502+
RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase", 15};
7503+
ASSERT_EQ(0, rbd_encryption_format(image, RBD_ENCRYPTION_FORMAT_LUKS1,
7504+
&fopts, sizeof(fopts)));
7505+
test_deterministic(image, object_off, len, 512);
7506+
7507+
ASSERT_EQ(0, rbd_close(image));
7508+
rados_ioctx_destroy(ioctx);
7509+
}
7510+
7511+
void test_deterministic_luks1_pp(uint64_t object_off, uint64_t len) {
7512+
librados::IoCtx ioctx;
7513+
ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));
7514+
7515+
librbd::RBD rbd;
7516+
librbd::Image image;
7517+
int order = 22;
7518+
std::string name = this->get_temp_image_name();
7519+
ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), 24 << 20, &order));
7520+
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
7521+
librbd::encryption_luks1_format_options_t fopts = {
7522+
RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase"};
7523+
ASSERT_EQ(0, image.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS1, &fopts,
7524+
sizeof(fopts)));
7525+
test_deterministic_pp(image, object_off, len, 512);
7526+
}
7527+
7528+
void test_deterministic_luks2(uint64_t object_off, uint64_t len) {
7529+
rados_ioctx_t ioctx;
7530+
ASSERT_EQ(0, rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx));
7531+
7532+
rbd_image_t image;
7533+
int order = 22;
7534+
std::string name = this->get_temp_image_name();
7535+
ASSERT_EQ(0, create_image(ioctx, name.c_str(), 36 << 20, &order));
7536+
ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
7537+
rbd_encryption_luks2_format_options_t fopts = {
7538+
RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase", 15};
7539+
ASSERT_EQ(0, rbd_encryption_format(image, RBD_ENCRYPTION_FORMAT_LUKS2,
7540+
&fopts, sizeof(fopts)));
7541+
test_deterministic(image, object_off, len, 4096);
7542+
7543+
ASSERT_EQ(0, rbd_close(image));
7544+
rados_ioctx_destroy(ioctx);
7545+
}
7546+
7547+
void test_deterministic_luks2_pp(uint64_t object_off, uint64_t len) {
7548+
librados::IoCtx ioctx;
7549+
ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));
7550+
7551+
librbd::RBD rbd;
7552+
librbd::Image image;
7553+
int order = 22;
7554+
std::string name = this->get_temp_image_name();
7555+
ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), 36 << 20, &order));
7556+
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
7557+
librbd::encryption_luks2_format_options_t fopts = {
7558+
RBD_ENCRYPTION_ALGORITHM_AES256, "some passphrase"};
7559+
ASSERT_EQ(0, image.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS2, &fopts,
7560+
sizeof(fopts)));
7561+
test_deterministic_pp(image, object_off, len, 4096);
7562+
}
7563+
7564+
#endif // HAVE_LIBCRYPTSETUP
7565+
74907566
private:
74917567
void test_deterministic(rbd_image_t image, uint64_t object_off,
74927568
uint64_t len, uint64_t block_size) {
@@ -7894,6 +7970,58 @@ TYPED_TEST(DiffIterateTest, DiffIterateDeterministicPP)
78947970
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_pp((4 << 20) - 2, 2));
78957971
}
78967972

7973+
#ifdef HAVE_LIBCRYPTSETUP
7974+
7975+
TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS1)
7976+
{
7977+
REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
7978+
REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
7979+
7980+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1(0, 256));
7981+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((1 << 20) - 256, 256));
7982+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((1 << 20) - 128, 256));
7983+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1(1 << 20, 256));
7984+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1((4 << 20) - 256, 256));
7985+
}
7986+
7987+
TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS1PP)
7988+
{
7989+
REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
7990+
REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
7991+
7992+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp(0, 2));
7993+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((3 << 20) - 2, 2));
7994+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((3 << 20) - 1, 2));
7995+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp(3 << 20, 2));
7996+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks1_pp((4 << 20) - 2, 2));
7997+
}
7998+
7999+
TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS2)
8000+
{
8001+
REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
8002+
REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
8003+
8004+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2(0, 256));
8005+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((1 << 20) - 256, 256));
8006+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((1 << 20) - 128, 256));
8007+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2(1 << 20, 256));
8008+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2((4 << 20) - 256, 256));
8009+
}
8010+
8011+
TYPED_TEST(DiffIterateTest, DiffIterateDeterministicLUKS2PP)
8012+
{
8013+
REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));
8014+
REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));
8015+
8016+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp(0, 2));
8017+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((3 << 20) - 2, 2));
8018+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((3 << 20) - 1, 2));
8019+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp(3 << 20, 2));
8020+
EXPECT_NO_FATAL_FAILURE(this->test_deterministic_luks2_pp((4 << 20) - 2, 2));
8021+
}
8022+
8023+
#endif // HAVE_LIBCRYPTSETUP
8024+
78978025
TYPED_TEST(DiffIterateTest, DiffIterateDiscard)
78988026
{
78998027
REQUIRE(!is_feature_enabled(RBD_FEATURE_STRIPINGV2));

0 commit comments

Comments
 (0)