Skip to content

Commit 15f519e

Browse files
Alex Markuzeidryomov
Alex Markuze
authored and
idryomov
committed
ceph: fix race condition validating r_parent before applying state
Add validation to ensure the cached parent directory inode matches the directory info in MDS replies. This prevents client-side race conditions where concurrent operations (e.g. rename) cause r_parent to become stale between request initiation and reply processing, which could lead to applying state changes to incorrect directory inodes. [ idryomov: folded a kerneldoc fixup and a follow-up fix from Alex to move CEPH_CAP_PIN reference when r_parent is updated: When the parent directory lock is not held, req->r_parent can become stale and is updated to point to the correct inode. However, the associated CEPH_CAP_PIN reference was not being adjusted. The CEPH_CAP_PIN is a reference on an inode that is tracked for accounting purposes. Moving this pin is important to keep the accounting balanced. When the pin was not moved from the old parent to the new one, it created two problems: The reference on the old, stale parent was never released, causing a reference leak. A reference for the new parent was never acquired, creating the risk of a reference underflow later in ceph_mdsc_release_request(). This patch corrects the logic by releasing the pin from the old parent and acquiring it for the new parent when r_parent is switched. This ensures reference accounting stays balanced. ] Cc: [email protected] Signed-off-by: Alex Markuze <[email protected]> Reviewed-by: Viacheslav Dubeyko <[email protected]> Signed-off-by: Ilya Dryomov <[email protected]>
1 parent 76eeb9b commit 15f519e

File tree

6 files changed

+145
-107
lines changed

6 files changed

+145
-107
lines changed

‎fs/ceph/debugfs.c‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ static int mdsc_show(struct seq_file *s, void *p)
5555
struct ceph_mds_client *mdsc = fsc->mdsc;
5656
struct ceph_mds_request *req;
5757
struct rb_node *rp;
58-
int pathlen = 0;
59-
u64 pathbase;
6058
char *path;
6159

6260
mutex_lock(&mdsc->mutex);
@@ -81,8 +79,8 @@ static int mdsc_show(struct seq_file *s, void *p)
8179
if (req->r_inode) {
8280
seq_printf(s, " #%llx", ceph_ino(req->r_inode));
8381
} else if (req->r_dentry) {
84-
path = ceph_mdsc_build_path(mdsc, req->r_dentry, &pathlen,
85-
&pathbase, 0);
82+
struct ceph_path_info path_info;
83+
path = ceph_mdsc_build_path(mdsc, req->r_dentry, &path_info, 0);
8684
if (IS_ERR(path))
8785
path = NULL;
8886
spin_lock(&req->r_dentry->d_lock);
@@ -91,7 +89,7 @@ static int mdsc_show(struct seq_file *s, void *p)
9189
req->r_dentry,
9290
path ? path : "");
9391
spin_unlock(&req->r_dentry->d_lock);
94-
ceph_mdsc_free_path(path, pathlen);
92+
ceph_mdsc_free_path_info(&path_info);
9593
} else if (req->r_path1) {
9694
seq_printf(s, " #%llx/%s", req->r_ino1.ino,
9795
req->r_path1);
@@ -100,8 +98,8 @@ static int mdsc_show(struct seq_file *s, void *p)
10098
}
10199

102100
if (req->r_old_dentry) {
103-
path = ceph_mdsc_build_path(mdsc, req->r_old_dentry, &pathlen,
104-
&pathbase, 0);
101+
struct ceph_path_info path_info;
102+
path = ceph_mdsc_build_path(mdsc, req->r_old_dentry, &path_info, 0);
105103
if (IS_ERR(path))
106104
path = NULL;
107105
spin_lock(&req->r_old_dentry->d_lock);
@@ -111,7 +109,7 @@ static int mdsc_show(struct seq_file *s, void *p)
111109
req->r_old_dentry,
112110
path ? path : "");
113111
spin_unlock(&req->r_old_dentry->d_lock);
114-
ceph_mdsc_free_path(path, pathlen);
112+
ceph_mdsc_free_path_info(&path_info);
115113
} else if (req->r_path2 && req->r_op != CEPH_MDS_OP_SYMLINK) {
116114
if (req->r_ino2.ino)
117115
seq_printf(s, " #%llx/%s", req->r_ino2.ino,

‎fs/ceph/dir.c‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,10 +1271,8 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
12711271

12721272
/* If op failed, mark everyone involved for errors */
12731273
if (result) {
1274-
int pathlen = 0;
1275-
u64 base = 0;
1276-
char *path = ceph_mdsc_build_path(mdsc, dentry, &pathlen,
1277-
&base, 0);
1274+
struct ceph_path_info path_info = {0};
1275+
char *path = ceph_mdsc_build_path(mdsc, dentry, &path_info, 0);
12781276

12791277
/* mark error on parent + clear complete */
12801278
mapping_set_error(req->r_parent->i_mapping, result);
@@ -1288,8 +1286,8 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
12881286
mapping_set_error(req->r_old_inode->i_mapping, result);
12891287

12901288
pr_warn_client(cl, "failure path=(%llx)%s result=%d!\n",
1291-
base, IS_ERR(path) ? "<<bad>>" : path, result);
1292-
ceph_mdsc_free_path(path, pathlen);
1289+
path_info.vino.ino, IS_ERR(path) ? "<<bad>>" : path, result);
1290+
ceph_mdsc_free_path_info(&path_info);
12931291
}
12941292
out:
12951293
iput(req->r_old_inode);
@@ -1347,8 +1345,6 @@ static int ceph_unlink(struct inode *dir, struct dentry *dentry)
13471345
int err = -EROFS;
13481346
int op;
13491347
char *path;
1350-
int pathlen;
1351-
u64 pathbase;
13521348

13531349
if (ceph_snap(dir) == CEPH_SNAPDIR) {
13541350
/* rmdir .snap/foo is RMSNAP */
@@ -1367,14 +1363,15 @@ static int ceph_unlink(struct inode *dir, struct dentry *dentry)
13671363
if (!dn) {
13681364
try_async = false;
13691365
} else {
1370-
path = ceph_mdsc_build_path(mdsc, dn, &pathlen, &pathbase, 0);
1366+
struct ceph_path_info path_info;
1367+
path = ceph_mdsc_build_path(mdsc, dn, &path_info, 0);
13711368
if (IS_ERR(path)) {
13721369
try_async = false;
13731370
err = 0;
13741371
} else {
13751372
err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
13761373
}
1377-
ceph_mdsc_free_path(path, pathlen);
1374+
ceph_mdsc_free_path_info(&path_info);
13781375
dput(dn);
13791376

13801377
/* For none EACCES cases will let the MDS do the mds auth check */

‎fs/ceph/file.c‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,6 @@ int ceph_open(struct inode *inode, struct file *file)
368368
int flags, fmode, wanted;
369369
struct dentry *dentry;
370370
char *path;
371-
int pathlen;
372-
u64 pathbase;
373371
bool do_sync = false;
374372
int mask = MAY_READ;
375373

@@ -399,14 +397,15 @@ int ceph_open(struct inode *inode, struct file *file)
399397
if (!dentry) {
400398
do_sync = true;
401399
} else {
402-
path = ceph_mdsc_build_path(mdsc, dentry, &pathlen, &pathbase, 0);
400+
struct ceph_path_info path_info;
401+
path = ceph_mdsc_build_path(mdsc, dentry, &path_info, 0);
403402
if (IS_ERR(path)) {
404403
do_sync = true;
405404
err = 0;
406405
} else {
407406
err = ceph_mds_check_access(mdsc, path, mask);
408407
}
409-
ceph_mdsc_free_path(path, pathlen);
408+
ceph_mdsc_free_path_info(&path_info);
410409
dput(dentry);
411410

412411
/* For none EACCES cases will let the MDS do the mds auth check */
@@ -614,15 +613,13 @@ static void ceph_async_create_cb(struct ceph_mds_client *mdsc,
614613
mapping_set_error(req->r_parent->i_mapping, result);
615614

616615
if (result) {
617-
int pathlen = 0;
618-
u64 base = 0;
619-
char *path = ceph_mdsc_build_path(mdsc, req->r_dentry, &pathlen,
620-
&base, 0);
616+
struct ceph_path_info path_info = {0};
617+
char *path = ceph_mdsc_build_path(mdsc, req->r_dentry, &path_info, 0);
621618

622619
pr_warn_client(cl,
623620
"async create failure path=(%llx)%s result=%d!\n",
624-
base, IS_ERR(path) ? "<<bad>>" : path, result);
625-
ceph_mdsc_free_path(path, pathlen);
621+
path_info.vino.ino, IS_ERR(path) ? "<<bad>>" : path, result);
622+
ceph_mdsc_free_path_info(&path_info);
626623

627624
ceph_dir_clear_complete(req->r_parent);
628625
if (!d_unhashed(dentry))
@@ -791,8 +788,6 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
791788
int mask;
792789
int err;
793790
char *path;
794-
int pathlen;
795-
u64 pathbase;
796791

797792
doutc(cl, "%p %llx.%llx dentry %p '%pd' %s flags %d mode 0%o\n",
798793
dir, ceph_vinop(dir), dentry, dentry,
@@ -814,7 +809,8 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
814809
if (!dn) {
815810
try_async = false;
816811
} else {
817-
path = ceph_mdsc_build_path(mdsc, dn, &pathlen, &pathbase, 0);
812+
struct ceph_path_info path_info;
813+
path = ceph_mdsc_build_path(mdsc, dn, &path_info, 0);
818814
if (IS_ERR(path)) {
819815
try_async = false;
820816
err = 0;
@@ -826,7 +822,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
826822
mask |= MAY_WRITE;
827823
err = ceph_mds_check_access(mdsc, path, mask);
828824
}
829-
ceph_mdsc_free_path(path, pathlen);
825+
ceph_mdsc_free_path_info(&path_info);
830826
dput(dn);
831827

832828
/* For none EACCES cases will let the MDS do the mds auth check */

‎fs/ceph/inode.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,22 +2487,21 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode,
24872487
int truncate_retry = 20; /* The RMW will take around 50ms */
24882488
struct dentry *dentry;
24892489
char *path;
2490-
int pathlen;
2491-
u64 pathbase;
24922490
bool do_sync = false;
24932491

24942492
dentry = d_find_alias(inode);
24952493
if (!dentry) {
24962494
do_sync = true;
24972495
} else {
2498-
path = ceph_mdsc_build_path(mdsc, dentry, &pathlen, &pathbase, 0);
2496+
struct ceph_path_info path_info;
2497+
path = ceph_mdsc_build_path(mdsc, dentry, &path_info, 0);
24992498
if (IS_ERR(path)) {
25002499
do_sync = true;
25012500
err = 0;
25022501
} else {
25032502
err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
25042503
}
2505-
ceph_mdsc_free_path(path, pathlen);
2504+
ceph_mdsc_free_path_info(&path_info);
25062505
dput(dentry);
25072506

25082507
/* For none EACCES cases will let the MDS do the mds auth check */

0 commit comments

Comments
 (0)