From bf4695f1da893faf648a5d0e08c269efd91c04cd Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Fri, 21 May 2021 15:27:31 +0200 Subject: [PATCH] librbd/cache/pwl/ssd: avoid corrupting m_first_free_entry In append_op_log_entries(), new_first_free_entry is read after append_ops() returns. This can result in accessing freed memory because all I/Os may complete and append_ctx callback may run by the time new_first_free_entry is read. Garbage value gets written to m_first_free_entry and depending on the circumstances it may allow AbstractWriteLog code to accept more dirty user data than we have space for. Luckily we usually crash before then. Fixes: https://tracker.ceph.com/issues/50832 Signed-off-by: Ilya Dryomov (cherry picked from commit d83a0f6db8ff26eeb2c817b1bd192fb357f715df) --- src/librbd/cache/pwl/ssd/WriteLog.cc | 25 ++++++++++++------------- src/librbd/cache/pwl/ssd/WriteLog.h | 3 +-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc index df25cd1c36a9e..1a31004983f28 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -455,14 +455,7 @@ void WriteLog::append_op_log_entries(GenericLogOperations &ops) { } }); // Append logs and update first_free_update - uint64_t bytes_allocated_updated; - append_ops(ops, append_ctx, new_first_free_entry, bytes_allocated_updated); - - { - std::lock_guard locker1(m_lock); - m_first_free_entry = *new_first_free_entry; - m_bytes_allocated -= bytes_allocated_updated; - } + append_ops(ops, append_ctx, new_first_free_entry); if (ops.size()) { this->dispatch_deferred_writes(); @@ -738,12 +731,11 @@ bool WriteLog::retire_entries(const unsigned long int frees_per_tx) { template void WriteLog::append_ops(GenericLogOperations &ops, Context *ctx, - uint64_t* new_first_free_entry, - uint64_t &bytes_allocated) { + uint64_t* new_first_free_entry) { GenericLogEntriesVector log_entries; CephContext *cct = m_image_ctx.cct; uint64_t span_payload_len = 0; - bytes_allocated = 0; + uint64_t bytes_to_free = 0; ldout(cct, 20) << "Appending " << ops.size() << " log entries." << dendl; AioTransContext* aio = new AioTransContext(cct, ctx); @@ -756,7 +748,7 @@ void WriteLog::append_ops(GenericLogOperations &ops, Context *ctx, if (log_entries.size() == CONTROL_BLOCK_MAX_LOG_ENTRIES || span_payload_len >= SPAN_MAX_DATA_LEN) { if (log_entries.size() > 1) { - bytes_allocated += (log_entries.size() - 1) * MIN_WRITE_ALLOC_SSD_SIZE; + bytes_to_free += (log_entries.size() - 1) * MIN_WRITE_ALLOC_SSD_SIZE; } write_log_entries(log_entries, aio); log_entries.clear(); @@ -767,10 +759,17 @@ void WriteLog::append_ops(GenericLogOperations &ops, Context *ctx, } if (!span_payload_len || !log_entries.empty()) { if (log_entries.size() > 1) { - bytes_allocated += (log_entries.size() - 1) * MIN_WRITE_ALLOC_SSD_SIZE; + bytes_to_free += (log_entries.size() - 1) * MIN_WRITE_ALLOC_SSD_SIZE; } write_log_entries(log_entries, aio); } + + { + std::lock_guard locker1(m_lock); + m_first_free_entry = *new_first_free_entry; + m_bytes_allocated -= bytes_to_free; + } + bdev->aio_submit(&aio->ioc); *new_first_free_entry = pool_root.first_free_entry; } diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h index 83633dabf425e..30f8bb23bf0ea 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.h +++ b/src/librbd/cache/pwl/ssd/WriteLog.h @@ -123,8 +123,7 @@ private: Context* construct_flush_entry_ctx( std::shared_ptr log_entry); void append_ops(GenericLogOperations &ops, Context *ctx, - uint64_t* new_first_free_entry, - uint64_t &bytes_allocated); + uint64_t* new_first_free_entry); void write_log_entries(GenericLogEntriesVector log_entries, AioTransContext *aio); void schedule_update_root(std::shared_ptr root, -- 2.39.5