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>

3 files changed, 4 insertions, 104 deletions

diff --git a/src/librbd/crypto/luks/FlattenRequest.cc b/src/librbd/crypto/luks/FlattenRequest.cc
index 8ec8505f9d5..edc7e3c475a 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<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 e277825175b..a1432f505b4 100644
--- 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 72dc985f7b6..bc615bcf76e 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<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();