Skip to content

Commit 0ea8570

Browse files
committed
mds: use available CInode* for uninline data
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 <[email protected]>
1 parent e1f7b2c commit 0ea8570

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

src/mds/MDCache.cc

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13487,7 +13487,7 @@ class C_MDC_DataUninlinedSubmitted : public MDCacheLogContext {
1348713487

1348813488
void finish(int r) {
1348913489
auto mds = get_mds(); // to keep dout happy
13490-
auto in = mds->server->rdlock_path_pin_ref(mdr, true);
13490+
auto in = mdr->in[0];
1349113491

1349213492
ceph_assert(in != nullptr);
1349313493

@@ -13504,6 +13504,7 @@ class C_MDC_DataUninlinedSubmitted : public MDCacheLogContext {
1350413504
h->record_uninline_passed();
1350513505
in->uninline_finished();
1350613506
mdr->apply();
13507+
in->auth_unpin(this); // for uninline data
1350713508
mds->server->respond_to_request(mdr, r);
1350813509
}
1350913510
};
@@ -13520,7 +13521,9 @@ struct C_IO_DataUninlined : public MDSIOContext {
1352013521

1352113522
void finish(int r) override {
1352213523
auto mds = get_mds(); // to keep dout/derr happy
13523-
auto in = mds->server->rdlock_path_pin_ref(mdr, true);
13524+
auto in = mdr->in[0];
13525+
13526+
ceph_assert(in != nullptr);
1352413527

1352513528
// return faster if operation has failed (non-zero) status
1352613529
if (r) {
@@ -13534,6 +13537,7 @@ struct C_IO_DataUninlined : public MDSIOContext {
1353413537
in->make_path_string(path);
1353513538
h->record_uninline_status(in->ino(), r, path);
1353613539
in->uninline_finished();
13540+
in->auth_unpin(this); // for uninline data
1353713541
mds->server->respond_to_request(mdr, r);
1353813542
return;
1353913543
}
@@ -13575,11 +13579,9 @@ struct C_IO_DataUninlined : public MDSIOContext {
1357513579

1357613580
void MDCache::uninline_data_work(MDRequestRef mdr)
1357713581
{
13578-
CInode *in = mds->server->rdlock_path_pin_ref(mdr, true);
13582+
CInode *in = mdr->in[0];
1357913583

13580-
if (!in) {
13581-
return;
13582-
}
13584+
ceph_assert(in != nullptr);
1358313585

1358413586
MutationImpl::LockOpVec lov;
1358513587
lov.add_xlock(&in->authlock);
@@ -13594,6 +13596,7 @@ void MDCache::uninline_data_work(MDRequestRef mdr)
1359413596
if (!in->has_inline_data()) {
1359513597
dout(20) << "(uninline_data) inode doesn't have inline data anymore " << *in << dendl;
1359613598
in->uninline_finished();
13599+
in->auth_unpin(this); // for uninline_data
1359713600
mds->server->respond_to_request(mdr, 0);
1359813601
return;
1359913602
}

src/mds/ScrubStack.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,7 +1382,9 @@ void ScrubStack::uninline_data(CInode *in, Context *fin)
13821382
mdr->snapid = CEPH_NOSNAP;
13831383
mdr->no_early_reply = true;
13841384
mdr->internal_op_finish = fin;
1385+
mdr->in[0] = in;
13851386

1387+
in->auth_pin(this);
13861388
in->mdcache->dispatch_request(mdr);
13871389
}
13881390

0 commit comments

Comments
 (0)