From abded6eedaa7d945ccadabfaece3aed5ef9d128f Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 15 Sep 2022 09:33:01 +0200 Subject: [PATCH] librbd: don't temporarily shut down crypto when flattening (Temporarily) shutting down crypto can lead to data corruption in the face of concurrent I/O, especially when flatten operation is proxied to the remote lock owner. This was added to be able to read, optionally modify and write crypto header without it being subjected to remapping and encryption itself. read_header() and write_header() now achieve that by specifying CRYPTO_HEADER area explicitly. Signed-off-by: Ilya Dryomov --- src/librbd/crypto/luks/FlattenRequest.cc | 37 +---------- src/librbd/crypto/luks/FlattenRequest.h | 8 +-- .../crypto/luks/test_mock_FlattenRequest.cc | 63 ------------------- 3 files changed, 4 insertions(+), 104 deletions(-) diff --git a/src/librbd/crypto/luks/FlattenRequest.cc b/src/librbd/crypto/luks/FlattenRequest.cc index 8ec8505f9d5d3..edc7e3c475adc 100644 --- a/src/librbd/crypto/luks/FlattenRequest.cc +++ b/src/librbd/crypto/luks/FlattenRequest.cc @@ -7,7 +7,6 @@ #include "common/errno.h" #include "librbd/Utils.h" #include "librbd/crypto/EncryptionFormat.h" -#include "librbd/crypto/ShutDownCryptoRequest.h" #include "librbd/crypto/Utils.h" #include "librbd/crypto/luks/LoadRequest.h" #include "librbd/crypto/luks/Magic.h" @@ -35,30 +34,6 @@ FlattenRequest::FlattenRequest( template void FlattenRequest::send() { - shutdown_crypto(); -} - -template -void FlattenRequest::shutdown_crypto() { - auto ctx = create_context_callback< - FlattenRequest, &FlattenRequest::handle_shutdown_crypto>(this); - - auto *req = ShutDownCryptoRequest::create( - m_image_ctx, &m_encryption_format, ctx); - req->send(); -} - -template -void FlattenRequest::handle_shutdown_crypto(int r) { - if (r < 0) { - lderr(m_image_ctx->cct) << "error shutting down crypto: " - << cpp_strerror(r) << dendl; - finish(r); - return; - } - - ceph_assert(m_encryption_format.get() != nullptr); - read_header(); } @@ -66,15 +41,14 @@ template void FlattenRequest::read_header() { auto ctx = create_context_callback< FlattenRequest, &FlattenRequest::handle_read_header>(this); - - uint64_t data_offset = m_encryption_format->get_crypto()->get_data_offset(); - auto aio_comp = io::AioCompletion::create_and_start( ctx, librbd::util::get_image_ctx(m_image_ctx), io::AIO_TYPE_READ); + + auto crypto = m_image_ctx->encryption_format->get_crypto(); ZTracer::Trace trace; auto req = io::ImageDispatchSpec::create_read( *m_image_ctx, io::IMAGE_DISPATCH_LAYER_API_START, aio_comp, - {{0, data_offset}}, io::ImageArea::CRYPTO_HEADER, + {{0, crypto->get_data_offset()}}, io::ImageArea::CRYPTO_HEADER, io::ReadResult{&m_bl}, m_image_ctx->get_data_io_context(), 0, 0, trace); req->send(); @@ -169,11 +143,6 @@ template void FlattenRequest::finish(int r) { ldout(m_image_ctx->cct, 20) << "r=" << r << dendl; - // restore crypto to image context - if (m_encryption_format.get() != nullptr) { - util::set_crypto(m_image_ctx, std::move(m_encryption_format)); - } - m_on_finish->complete(r); delete this; } diff --git a/src/librbd/crypto/luks/FlattenRequest.h b/src/librbd/crypto/luks/FlattenRequest.h index e277825175b4d..a1432f505b444 100644 --- a/src/librbd/crypto/luks/FlattenRequest.h +++ b/src/librbd/crypto/luks/FlattenRequest.h @@ -30,9 +30,6 @@ private: * * | * v - * SHUTDOWN_CRYPTO - * | - * v * READ_HEADER * | * v @@ -42,17 +39,14 @@ private: * FLUSH * | * v - * (+ RESTORE_CRYPTO) + * * * @endverbatim */ I* m_image_ctx; Context* m_on_finish; ceph::bufferlist m_bl; - EncryptionFormat m_encryption_format; - void shutdown_crypto(); - void handle_shutdown_crypto(int r); void read_header(); void handle_read_header(int r); void write_header(); diff --git a/src/test/librbd/crypto/luks/test_mock_FlattenRequest.cc b/src/test/librbd/crypto/luks/test_mock_FlattenRequest.cc index 72dc985f7b660..bc615bcf76e00 100644 --- a/src/test/librbd/crypto/luks/test_mock_FlattenRequest.cc +++ b/src/test/librbd/crypto/luks/test_mock_FlattenRequest.cc @@ -31,42 +31,6 @@ inline ImageCtx *get_image_ctx(MockImageCtx *image_ctx) { namespace librbd { namespace crypto { - -namespace util { - -template <> -void set_crypto(MockTestImageCtx *image_ctx, - std::unique_ptr encryption_format) { - image_ctx->encryption_format = std::move(encryption_format); -} - -} // namespace util - -template <> -struct ShutDownCryptoRequest { - Context *on_finish = nullptr; - std::unique_ptr* format; - static ShutDownCryptoRequest *s_instance; - static ShutDownCryptoRequest *create( - MockTestImageCtx *image_ctx, - std::unique_ptr* format, - Context *on_finish) { - ceph_assert(s_instance != nullptr); - s_instance->format = format; - s_instance->on_finish = on_finish; - return s_instance; - } - - MOCK_METHOD0(send, void()); - - ShutDownCryptoRequest() { - s_instance = this; - } -}; - -ShutDownCryptoRequest *ShutDownCryptoRequest< - MockTestImageCtx>::s_instance = nullptr; - namespace luks { using ::testing::_; @@ -75,7 +39,6 @@ using ::testing::Return; struct TestMockCryptoLuksFlattenRequest : public TestMockFixture { typedef FlattenRequest MockFlattenRequest; - typedef ShutDownCryptoRequest MockShutDownCryptoRequest; const size_t OBJECT_SIZE = 4 * 1024 * 1024; const uint64_t DATA_OFFSET = MockCryptoInterface::DATA_OFFSET; @@ -86,7 +49,6 @@ struct TestMockCryptoLuksFlattenRequest : public TestMockFixture { MockFlattenRequest* mock_flatten_request; MockEncryptionFormat* mock_encryption_format; MockCryptoInterface mock_crypto; - MockShutDownCryptoRequest mock_shutdown_crypto_request; C_SaferCond finished_cond; Context *on_finish = &finished_cond; Context* image_read_request; @@ -125,17 +87,6 @@ struct TestMockCryptoLuksFlattenRequest : public TestMockFixture { } } - void expect_shutdown_crypto(int r = 0) { - EXPECT_CALL(mock_shutdown_crypto_request, send()).WillOnce( - Invoke([this, r]() { - if (r == 0) { - *mock_shutdown_crypto_request.format = - std::move(mock_image_ctx->encryption_format); - } - mock_shutdown_crypto_request.on_finish->complete(r); - })); - } - void expect_get_crypto() { EXPECT_CALL(*mock_encryption_format, get_crypto()).WillOnce( Return(&mock_crypto)); @@ -224,7 +175,6 @@ struct TestMockCryptoLuksFlattenRequest : public TestMockFixture { TEST_F(TestMockCryptoLuksFlattenRequest, LUKS1) { generate_header(CRYPT_LUKS1, "aes", 32, "xts-plain64", 512, true); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); @@ -241,7 +191,6 @@ TEST_F(TestMockCryptoLuksFlattenRequest, LUKS1) { TEST_F(TestMockCryptoLuksFlattenRequest, LUKS2) { generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, true); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); @@ -256,17 +205,8 @@ TEST_F(TestMockCryptoLuksFlattenRequest, LUKS2) { ASSERT_EQ(mock_encryption_format, mock_image_ctx->encryption_format.get()); } -TEST_F(TestMockCryptoLuksFlattenRequest, FailShutDownCrypto) { - generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, true); - expect_shutdown_crypto(-EIO); - mock_flatten_request->send(); - ASSERT_EQ(-EIO, finished_cond.wait()); - ASSERT_EQ(mock_encryption_format, mock_image_ctx->encryption_format.get()); -} - TEST_F(TestMockCryptoLuksFlattenRequest, FailedRead) { generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, true); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); @@ -278,7 +218,6 @@ TEST_F(TestMockCryptoLuksFlattenRequest, FailedRead) { TEST_F(TestMockCryptoLuksFlattenRequest, AlreadyFlattened) { generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, false); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); @@ -295,7 +234,6 @@ TEST_F(TestMockCryptoLuksFlattenRequest, AlreadyFlattened) { TEST_F(TestMockCryptoLuksFlattenRequest, FailedWrite) { generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, true); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); @@ -310,7 +248,6 @@ TEST_F(TestMockCryptoLuksFlattenRequest, FailedWrite) { TEST_F(TestMockCryptoLuksFlattenRequest, FailedFlush) { generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, true); - expect_shutdown_crypto(); expect_get_crypto(); expect_image_read(0, DATA_OFFSET); mock_flatten_request->send(); -- 2.39.5