From c0c7b2f2776154ffb130e08e5a6c4e1da8d758d0 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 21 May 2018 16:39:47 -0500 Subject: [PATCH] osd/ReplicatedBackend: fix use-after-free on InProgressOp - op in flight to disk... - on_change() clears the InProgressOp - C_OSD_OnOpCommit calls op_commit() w/ bare pointer - crash! Fix by refcounting InProgressOp and clearing on_commit when it is canceled. Fixes: http://tracker.ceph.com/issues/24219 Signed-off-by: Sage Weil (cherry picked from commit e84c2d097440aea5980fba2a2ef065769dbf1271) --- src/osd/ReplicatedBackend.cc | 18 ++++++++++++------ src/osd/ReplicatedBackend.h | 10 ++++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc index d6d1f731053a0..d5be66694233e 100644 --- a/src/osd/ReplicatedBackend.cc +++ b/src/osd/ReplicatedBackend.cc @@ -236,7 +236,8 @@ void ReplicatedBackend::on_change() { dout(10) << __func__ << dendl; for (auto& op : in_progress_ops) { - delete op.second.on_commit; + delete op.second->on_commit; + op.second->on_commit = nullptr; } in_progress_ops.clear(); clear_recovery_state(); @@ -264,7 +265,7 @@ void ReplicatedBackend::objects_read_async( class C_OSD_OnOpCommit : public Context { ReplicatedBackend *pg; - ReplicatedBackend::InProgressOp *op; + ReplicatedBackend::InProgressOpRef op; public: C_OSD_OnOpCommit(ReplicatedBackend *pg, ReplicatedBackend::InProgressOp *op) : pg(pg), op(op) {} @@ -453,13 +454,13 @@ void ReplicatedBackend::submit_transaction( auto insert_res = in_progress_ops.insert( make_pair( tid, - InProgressOp( + new InProgressOp( tid, on_all_commit, orig_op, at_version) ) ); assert(insert_res.second); - InProgressOp &op = insert_res.first->second; + InProgressOp &op = *insert_res.first->second; op.waiting_for_commit.insert( parent->get_acting_recovery_backfill_shards().begin(), @@ -504,8 +505,13 @@ void ReplicatedBackend::submit_transaction( } void ReplicatedBackend::op_commit( - InProgressOp *op) + InProgressOpRef& op) { + if (op->on_commit == nullptr) { + // aborted + return; + } + FUNCTRACE(cct); OID_EVENT_TRACE_WITH_MSG((op && op->op) ? op->op->get_req() : NULL, "OP_COMMIT_BEGIN", true); dout(10) << __func__ << ": " << op->tid << dendl; @@ -538,7 +544,7 @@ void ReplicatedBackend::do_repop_reply(OpRequestRef op) auto iter = in_progress_ops.find(rep_tid); if (iter != in_progress_ops.end()) { - InProgressOp &ip_op = iter->second; + InProgressOp &ip_op = *iter->second; const MOSDOp *m = NULL; if (ip_op.op) m = static_cast(ip_op.op->get_req()); diff --git a/src/osd/ReplicatedBackend.h b/src/osd/ReplicatedBackend.h index a54f19933cac7..adfbaa1b794ab 100644 --- a/src/osd/ReplicatedBackend.h +++ b/src/osd/ReplicatedBackend.h @@ -327,7 +327,7 @@ private: /** * Client IO */ - struct InProgressOp { + struct InProgressOp : public RefCountedObject { ceph_tid_t tid; set waiting_for_commit; Context *on_commit; @@ -336,13 +336,15 @@ private: InProgressOp( ceph_tid_t tid, Context *on_commit, OpRequestRef op, eversion_t v) - : tid(tid), on_commit(on_commit), + : RefCountedObject(nullptr, 0), + tid(tid), on_commit(on_commit), op(op), v(v) {} bool done() const { return waiting_for_commit.empty(); } }; - map in_progress_ops; + typedef boost::intrusive_ptr InProgressOpRef; + map in_progress_ops; public: friend class C_OSD_OnOpCommit; @@ -395,7 +397,7 @@ private: boost::optional &hset_history, InProgressOp *op, ObjectStore::Transaction &op_t); - void op_commit(InProgressOp *op); + void op_commit(InProgressOpRef& op); void do_repop_reply(OpRequestRef op); void do_repop(OpRequestRef op); -- 2.39.5