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>
This commit is contained in:
Ilya Dryomov 2022-09-15 09:33:01 +02:00
parent 744379b8c6
commit abded6eeda
3 changed files with 4 additions and 104 deletions

View File

@ -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;
}

View File

@ -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();

View File

@ -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();