Skip to content

Commit

Permalink
librbd: don't temporarily shut down crypto when flattening
Browse files Browse the repository at this point in the history 
(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 <[email protected]>
  • Loading branch information
@idryomov
idryomov committed Dec 4, 2022
1 parent 744379b commit abded6e
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 104 deletions.
37 changes: 3 additions & 34 deletions src/librbd/crypto/luks/FlattenRequest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -35,46 +34,21 @@ 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();
}

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();
Expand Down Expand Up @@ -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;
}
Expand Down
8 changes: 1 addition & 7 deletions src/librbd/crypto/luks/FlattenRequest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ class FlattenRequest {
* <start>
* |
* v
* SHUTDOWN_CRYPTO
* |
* v
* READ_HEADER
* |
* v
Expand All @@ -42,17 +39,14 @@ class FlattenRequest {
* 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();
Expand Down
63 changes: 0 additions & 63 deletions src/test/librbd/crypto/luks/test_mock_FlattenRequest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::_;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit abded6e

Please sign in to comment.