From: myoungwon oh <ohmyoungwon@gmail.com>
Date: Tue, 15 Dec 2020 01:07:21 +0000 (+0900)
Subject: osd: prevent accessing deleted reference
X-Git-Tag: v16.1.0~171^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b839203e2dddadac37fe71fc8a66292d108608aa;p=ceph.git

osd: prevent accessing deleted reference

cb in C_SetManifestRefCountDone() was freed
during cancel_manifest_ops(). After that,
~C_SetManifestRefCountDone() try to free the cb
because the cb is not nullptr.
This commit prevents calling a deleted reference.

Fixes: https://tracker.ceph.com/issues/48599

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>
---

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 8cb6495a51a3..5ddf9a30e364 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -3256,28 +3256,24 @@ struct SetManifestFinisher : public PrimaryLogPG::OpFinisher {
 };
 
 struct C_SetManifestRefCountDone : public Context {
-  RefCountCallback* cb;
+  PrimaryLogPGRef pg;
+  PrimaryLogPG::ManifestOpRef mop;
   hobject_t soid;
-  C_SetManifestRefCountDone(
-    RefCountCallback* cb, hobject_t soid) : cb(cb), soid(soid) {}
+  C_SetManifestRefCountDone(PrimaryLogPG *p,
+    PrimaryLogPG::ManifestOpRef mop, hobject_t soid) : 
+    pg(p), mop(mop), soid(soid) {}
   void finish(int r) override {
     if (r == -ECANCELED)
       return;
-    auto pg = cb->ctx->pg;
     std::scoped_lock locker{*pg};
     auto it = pg->manifest_ops.find(soid);
     if (it == pg->manifest_ops.end()) {
       // raced with cancel_manifest_ops
       return;
     }
+    it->second->cb->complete(r);
     pg->manifest_ops.erase(it);
-    cb->complete(r);
-    cb = nullptr;
-  }
-  ~C_SetManifestRefCountDone() {
-    if (cb) {
-      delete cb;
-    }
+    mop.reset();
   }
 };
 
@@ -3403,12 +3399,12 @@ bool PrimaryLogPG::inc_refcount_by_set(OpContext* ctx, object_manifest_t& set_ch
        * the reference the targe object has prior to update object_manifest in object_info_t.
        * So, call directly refcount_manifest.
        */
-      C_SetManifestRefCountDone* fin = new C_SetManifestRefCountDone(
-			  new RefCountCallback(ctx, osd_op), 
-			  ctx->obs->oi.soid);
+      ManifestOpRef mop = std::make_shared<ManifestOp>(new RefCountCallback(ctx, osd_op));
+      C_SetManifestRefCountDone* fin = new C_SetManifestRefCountDone(this, mop, ctx->obs->oi.soid);
       ceph_tid_t tid = refcount_manifest(ctx->obs->oi.soid, p->first,
-			refcount_t::INCREMENT_REF, fin, std::nullopt);
-      manifest_ops[ctx->obs->oi.soid] = std::make_shared<ManifestOp>(fin->cb, tid);
+					  refcount_t::INCREMENT_REF, fin, std::nullopt);
+      mop->objecter_tid = tid;
+      manifest_ops[ctx->obs->oi.soid] = mop;
       ctx->obc->start_block();
       return true;
     } else if (inc_ref_count < 0) {
@@ -6870,12 +6866,12 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  // start
 	  ctx->op_finishers[ctx->current_osd_subop_num].reset(
 	    new SetManifestFinisher(osd_op));
-	  C_SetManifestRefCountDone* fin = new C_SetManifestRefCountDone(
-			      new RefCountCallback(ctx, osd_op), 
-			      soid);
+	  ManifestOpRef mop = std::make_shared<ManifestOp>(new RefCountCallback(ctx, osd_op));
+	  C_SetManifestRefCountDone* fin = new C_SetManifestRefCountDone(this, mop, soid);
 	  ceph_tid_t tid = refcount_manifest(soid, target, 
-			    refcount_t::INCREMENT_REF, fin, std::nullopt);
-	  manifest_ops[soid] = std::make_shared<ManifestOp>(fin->cb, tid);
+					      refcount_t::INCREMENT_REF, fin, std::nullopt);
+	  mop->objecter_tid = tid;
+	  manifest_ops[soid] = mop;
 	  ctx->obc->start_block();
 	  result = -EINPROGRESS;
 	} else {
@@ -10084,7 +10080,7 @@ int PrimaryLogPG::start_dedup(OpRequestRef op, ObjectContextRef obc)
    * The operations to make dedup chunks are tracked by a ManifestOp.
    * This op will be finished if all the operations are completed.
    */
-  ManifestOpRef mop(std::make_shared<ManifestOp>(nullptr, 0));
+  ManifestOpRef mop(std::make_shared<ManifestOp>(nullptr));
 
   // cdc
   std::map<uint64_t, bufferlist> chunks; 
diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index 5a8c9f405d6c..432a7f9c3366 100644
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -269,8 +269,8 @@ public:
     object_manifest_t new_manifest;
     
 
-    ManifestOp(RefCountCallback* cb, ceph_tid_t tid)
-      : cb(cb), objecter_tid(tid) {}
+    ManifestOp(RefCountCallback* cb)
+      : cb(cb), objecter_tid(0) {}
   };
   typedef std::shared_ptr<ManifestOp> ManifestOpRef;
   std::map<hobject_t, ManifestOpRef> manifest_ops;