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 <idryomov@gmail.com>

diff --git a/src/librbd/crypto/luks/FlattenRequest.cc b/src/librbd/crypto/luks/FlattenRequest.cc
index 8ec8505f9d5d3597d8b49a628f6f0f5bff6e9e68..edc7e3c475adc2eb1b769c16539b3090c8279602 100644 (file)
--- 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<I>::FlattenRequest(
 
 template <typename I>
 void FlattenRequest<I>::send() {
-  shutdown_crypto();
-}
-
-template <typename I>
-void FlattenRequest<I>::shutdown_crypto() {
-  auto ctx = create_context_callback<
-        FlattenRequest<I>, &FlattenRequest<I>::handle_shutdown_crypto>(this);
-
-  auto *req = ShutDownCryptoRequest<I>::create(
-          m_image_ctx, &m_encryption_format, ctx);
-  req->send();
-}
-
-template <typename I>
-void FlattenRequest<I>::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 <typename I>
 void FlattenRequest<I>::read_header() {
   auto ctx = create_context_callback<
         FlattenRequest<I>, &FlattenRequest<I>::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 <typename I>
 void FlattenRequest<I>::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 e277825175b4d396cc4e3c8395104e822cc0a3e6..a1432f505b444e3b7c9e67881904c66803a97d86 100644 (file)
--- a/src/librbd/crypto/luks/FlattenRequest.h
+++ b/src/librbd/crypto/luks/FlattenRequest.h
@@ -30,9 +30,6 @@ private:
    * <start>
    *    |
    *    v
-   * SHUTDOWN_CRYPTO
-   *    |
-   *    v
    * READ_HEADER
    *    |
    *    v
@@ -42,17 +39,14 @@ private:
    * FLUSH
    *    |
    *    v
-   * <finish> (+ RESTORE_CRYPTO)
+   * <finish>
    *
    * @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 72dc985f7b66093132f9ea360813b7aa5c55d16c..bc615bcf76e00dced7185bcdb676c682c5877634 100644 (file)
--- 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<MockEncryptionFormat> encryption_format) {
-  image_ctx->encryption_format = std::move(encryption_format);
-}
-
-} // namespace util
-
-template <>
-struct ShutDownCryptoRequest<MockTestImageCtx> {
-  Context *on_finish = nullptr;
-  std::unique_ptr<MockEncryptionFormat>* format;
-  static ShutDownCryptoRequest *s_instance;
-  static ShutDownCryptoRequest *create(
-          MockTestImageCtx *image_ctx,
-          std::unique_ptr<MockEncryptionFormat>* 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<MockTestImageCtx> *ShutDownCryptoRequest<
-        MockTestImageCtx>::s_instance = nullptr;
-
 namespace luks {
 
 using ::testing::_;
@@ -75,7 +39,6 @@ using ::testing::Return;
 
 struct TestMockCryptoLuksFlattenRequest : public TestMockFixture {
   typedef FlattenRequest<MockTestImageCtx> MockFlattenRequest;
-  typedef ShutDownCryptoRequest<MockTestImageCtx> 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();