From: Hualong Feng Date: Fri, 2 Jul 2021 01:49:39 +0000 (+0800) Subject: librbd/cache/pwl/ssd: fix use-after-free on C_BlockIORequest X-Git-Tag: v16.2.7~50^2~38 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=18549e347d6041e452affa8ea609020f538c5619;p=ceph.git librbd/cache/pwl/ssd: fix use-after-free on C_BlockIORequest In setup_schedule_append() function, its first expression will cause the req to be deleted, and subsequent use of the variable req becomes an illegal operation. And due to delete, rep->m_image_ctx will be empty, so it lead to segfault in AbstractWriteLog::get_context(). So pass the `req` into `schedule_append()` function. Fixes: https://tracker.ceph.com/issues/50951 Signed-off-by: Hualong Feng (cherry picked from commit 2dc3b8881290f1e12c536a232bea37547a73a45b) --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 518628b95285..67a3f845067b 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -1257,19 +1257,19 @@ void AbstractWriteLog::append_scheduled(GenericLogOperations &ops, bool &ops_ } template -void AbstractWriteLog::schedule_append(GenericLogOperationsVector &ops) +void AbstractWriteLog::schedule_append(GenericLogOperationsVector &ops, C_BlockIORequestT *req) { GenericLogOperations to_append(ops.begin(), ops.end()); - schedule_append_ops(to_append); + schedule_append_ops(to_append, req); } template -void AbstractWriteLog::schedule_append(GenericLogOperationSharedPtr op) +void AbstractWriteLog::schedule_append(GenericLogOperationSharedPtr op, C_BlockIORequestT *req) { GenericLogOperations to_append { op }; - schedule_append_ops(to_append); + schedule_append_ops(to_append, req); } /* diff --git a/src/librbd/cache/pwl/AbstractWriteLog.h b/src/librbd/cache/pwl/AbstractWriteLog.h index 09c20d4683ba..e61f44967b82 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.h +++ b/src/librbd/cache/pwl/AbstractWriteLog.h @@ -125,8 +125,8 @@ public: virtual void setup_schedule_append( pwl::GenericLogOperationsVector &ops, bool do_early_flush, C_BlockIORequestT *req) = 0; - void schedule_append(pwl::GenericLogOperationsVector &ops); - void schedule_append(pwl::GenericLogOperationSharedPtr op); + void schedule_append(pwl::GenericLogOperationsVector &ops, C_BlockIORequestT *req = nullptr); + void schedule_append(pwl::GenericLogOperationSharedPtr op, C_BlockIORequestT *req = nullptr); void flush_new_sync_point(C_FlushRequestT *flush_req, pwl::DeferredContexts &later); @@ -360,7 +360,7 @@ protected: virtual void process_work() = 0; virtual void append_scheduled_ops(void) = 0; - virtual void schedule_append_ops(pwl::GenericLogOperations &ops) = 0; + virtual void schedule_append_ops(pwl::GenericLogOperations &ops, C_BlockIORequestT *req) = 0; virtual void remove_pool_file() = 0; virtual void initialize_pool(Context *on_finish, pwl::DeferredContexts &later) = 0; diff --git a/src/librbd/cache/pwl/rwl/WriteLog.cc b/src/librbd/cache/pwl/rwl/WriteLog.cc index 3b8c1f4a104a..20b5e342b0af 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.cc +++ b/src/librbd/cache/pwl/rwl/WriteLog.cc @@ -624,7 +624,7 @@ void WriteLog::flush_then_append_scheduled_ops(void) * get to the log message append step. */ if (ops.size()) { flush_pmem_buffer(ops); - schedule_append_ops(ops); + schedule_append_ops(ops, nullptr); } } while (ops_remain); append_scheduled_ops(); @@ -694,7 +694,7 @@ void WriteLog::setup_schedule_append( * all prior log entries are persisted everywhere. */ template -void WriteLog::schedule_append_ops(GenericLogOperations &ops) +void WriteLog::schedule_append_ops(GenericLogOperations &ops, C_BlockIORequestT *req) { bool need_finisher; GenericLogOperationsVector appending; diff --git a/src/librbd/cache/pwl/rwl/WriteLog.h b/src/librbd/cache/pwl/rwl/WriteLog.h index d5c132e44630..a72e668dc3ec 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.h +++ b/src/librbd/cache/pwl/rwl/WriteLog.h @@ -82,7 +82,7 @@ protected: using AbstractWriteLog::m_first_valid_entry; void process_work() override; - void schedule_append_ops(pwl::GenericLogOperations &ops) override; + void schedule_append_ops(pwl::GenericLogOperations &ops, C_BlockIORequestT *req) override; void append_scheduled_ops(void) override; void reserve_cache(C_BlockIORequestT *req, bool &alloc_succeeds, bool &no_space) override; diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc index eec483f8da77..977586295dbe 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -353,7 +353,7 @@ void WriteLog::enlist_op_appender() { * all prior log entries are persisted everywhere. */ template -void WriteLog::schedule_append_ops(GenericLogOperations &ops) { +void WriteLog::schedule_append_ops(GenericLogOperations &ops, C_BlockIORequestT *req) { bool need_finisher = false; GenericLogOperationsVector appending; @@ -371,6 +371,17 @@ void WriteLog::schedule_append_ops(GenericLogOperations &ops) { need_finisher = has_sync_point_logs(ops); } this->m_ops_to_append.splice(this->m_ops_to_append.end(), ops); + + // To preserve the order of overlapping IOs, release_cell() may be + // called only after the ops are added to m_ops_to_append. + // As soon as m_lock is released, the appended ops can be picked up + // by append_scheduled_ops() in another thread and req can be freed. + if (req != nullptr) { + if (persist_on_flush) { + req->complete_user_request(0); + } + req->release_cell(); + } } if (need_finisher) { @@ -386,11 +397,7 @@ template void WriteLog::setup_schedule_append(pwl::GenericLogOperationsVector &ops, bool do_early_flush, C_BlockIORequestT *req) { - this->schedule_append(ops); - if (this->get_persist_on_flush()) { - req->complete_user_request(0); - } - req->release_cell(); + this->schedule_append(ops, req); } template diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h index 30f8bb23bf0e..9c3a2bbd6a8d 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.h +++ b/src/librbd/cache/pwl/ssd/WriteLog.h @@ -67,7 +67,7 @@ protected: pwl::DeferredContexts &later) override; void process_work() override; void append_scheduled_ops(void) override; - void schedule_append_ops(pwl::GenericLogOperations &ops) override; + void schedule_append_ops(pwl::GenericLogOperations &ops, C_BlockIORequestT *req) override; void remove_pool_file() override; void release_ram(std::shared_ptr log_entry) override;