]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
mds: use available CInode* for uninline data
authorMilind Changire <mchangir@redhat.com>
Thu, 3 Apr 2025 07:54:37 +0000 (13:24 +0530)
committerMilind Changire <mchangir@redhat.com>
Wed, 14 May 2025 16:03:32 +0000 (21:33 +0530)
This is a null pointer dereference issue and it happens as follows:

Uninline Data is not a regular client request ... it is an Internal Request.
So, there's no client request struct allocated and assigned in the mdr to
begin with.

In the scrubbing path, the auth validation is already done in
ScrubStack::kick_off_scrubs() ...  and since Uninline Data path piggybacks
on the scrubbing path, we get the auth validation for free.

rdlock_path_pin_ref(), fails to lock the path if the lock is already taken.
This is what happens in the Uninline Data case. So rdlock_path_pin_ref()
creates a C_MDS_RetryRequest and this causes the request to be re-attempted
in the regular client request path where Server::handle_client_request()
assumes that the mdr->client_request member is valid ...
and hence the null pointer dereference issue.
---
Since the scrub path dequeues the CInode* from the ScrubStack, this
commit attempts to use the already available CInode*.

Fixes: https://tracker.ceph.com/issues/70624
Signed-off-by: Milind Changire <mchangir@redhat.com>
src/mds/MDCache.cc
src/mds/ScrubStack.cc

index 5e7bcd2a376a85170acc3077ab78949b03d30f55..b2ea018cba7f8bd3bbe7cdeaa45e5aa4ce92be7e 100644 (file)
@@ -13487,7 +13487,7 @@ class C_MDC_DataUninlinedSubmitted : public MDCacheLogContext {
 
   void finish(int r) {
     auto mds = get_mds(); // to keep dout happy
-    auto in = mds->server->rdlock_path_pin_ref(mdr, true);
+    auto in = mdr->in[0];
 
     ceph_assert(in != nullptr);
 
@@ -13504,6 +13504,7 @@ class C_MDC_DataUninlinedSubmitted : public MDCacheLogContext {
     h->record_uninline_passed();
     in->uninline_finished();
     mdr->apply();
+    in->auth_unpin(this); // for uninline data
     mds->server->respond_to_request(mdr, r);
   }
 };
@@ -13520,7 +13521,9 @@ struct C_IO_DataUninlined : public MDSIOContext {
 
   void finish(int r) override {
     auto mds = get_mds(); // to keep dout/derr happy
-    auto in = mds->server->rdlock_path_pin_ref(mdr, true);
+    auto in = mdr->in[0];
+
+    ceph_assert(in != nullptr);
 
     // return faster if operation has failed (non-zero) status
     if (r) {
@@ -13534,6 +13537,7 @@ struct C_IO_DataUninlined : public MDSIOContext {
       in->make_path_string(path);
       h->record_uninline_status(in->ino(), r, path);
       in->uninline_finished();
+      in->auth_unpin(this); // for uninline data
       mds->server->respond_to_request(mdr, r);
       return;
     }
@@ -13575,11 +13579,9 @@ struct C_IO_DataUninlined : public MDSIOContext {
 
 void MDCache::uninline_data_work(MDRequestRef mdr)
 {
-  CInode *in = mds->server->rdlock_path_pin_ref(mdr, true);
+  CInode *in = mdr->in[0];
 
-  if (!in) {
-    return;
-  }
+  ceph_assert(in != nullptr);
 
   MutationImpl::LockOpVec lov;
   lov.add_xlock(&in->authlock);
@@ -13594,6 +13596,7 @@ void MDCache::uninline_data_work(MDRequestRef mdr)
   if (!in->has_inline_data()) {
     dout(20) << "(uninline_data) inode doesn't have inline data anymore " << *in << dendl;
     in->uninline_finished();
+    in->auth_unpin(this); // for uninline_data
     mds->server->respond_to_request(mdr, 0);
     return;
   }
index 61cf447c720fa43ab896f8fb632fe0c5939eb7c2..785bd3c34d80ab321fe1c2e22fef693f37aa02a5 100644 (file)
@@ -1382,7 +1382,9 @@ void ScrubStack::uninline_data(CInode *in, Context *fin)
   mdr->snapid = CEPH_NOSNAP;
   mdr->no_early_reply = true;
   mdr->internal_op_finish = fin;
+  mdr->in[0] = in;
 
+  in->auth_pin(this);
   in->mdcache->dispatch_request(mdr);
 }