Skip to content

Commit db378e6

Browse files
Alex Markuzegregkh
Alex Markuze
authored and
gregkh
committed
ceph: fix race condition validating r_parent before applying state
commit 15f519e upstream. 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: stable@vger.kernel.org Signed-off-by: Alex Markuze <amarkuze@redhat.com> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 35dbbc3 commit db378e6

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
@@ -1263,10 +1263,8 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
12631263

12641264
/* If op failed, mark everyone involved for errors */
12651265
if (result) {
1266-
int pathlen = 0;
1267-
u64 base = 0;
1268-
char *path = ceph_mdsc_build_path(mdsc, dentry, &pathlen,
1269-
&base, 0);
1266+
struct ceph_path_info path_info = {0};
1267+
char *path = ceph_mdsc_build_path(mdsc, dentry, &path_info, 0);
12701268

12711269
/* mark error on parent + clear complete */
12721270
mapping_set_error(req->r_parent->i_mapping, result);
@@ -1280,8 +1278,8 @@ static void ceph_async_unlink_cb(struct ceph_mds_client *mdsc,
12801278
mapping_set_error(req->r_old_inode->i_mapping, result);
12811279

12821280
pr_warn_client(cl, "failure path=(%llx)%s result=%d!\n",
1283-
base, IS_ERR(path) ? "<<bad>>" : path, result);
1284-
ceph_mdsc_free_path(path, pathlen);
1281+
path_info.vino.ino, IS_ERR(path) ? "<<bad>>" : path, result);
1282+
ceph_mdsc_free_path_info(&path_info);
12851283
}
12861284
out:
12871285
iput(req->r_old_inode);
@@ -1339,8 +1337,6 @@ static int ceph_unlink(struct inode *dir, struct dentry *dentry)
13391337
int err = -EROFS;
13401338
int op;
13411339
char *path;
1342-
int pathlen;
1343-
u64 pathbase;
13441340

13451341
if (ceph_snap(dir) == CEPH_SNAPDIR) {
13461342
/* rmdir .snap/foo is RMSNAP */
@@ -1359,14 +1355,15 @@ static int ceph_unlink(struct inode *dir, struct dentry *dentry)
13591355
if (!dn) {
13601356
try_async = false;
13611357
} else {
1362-
path = ceph_mdsc_build_path(mdsc, dn, &pathlen, &pathbase, 0);
1358+
struct ceph_path_info path_info;
1359+
path = ceph_mdsc_build_path(mdsc, dn, &path_info, 0);
13631360
if (IS_ERR(path)) {
13641361
try_async = false;
13651362
err = 0;
13661363
} else {
13671364
err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
13681365
}
1369-
ceph_mdsc_free_path(path, pathlen);
1366+
ceph_mdsc_free_path_info(&path_info);
13701367
dput(dn);
13711368

13721369
/* 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
@@ -2483,22 +2483,21 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode,
24832483
int truncate_retry = 20; /* The RMW will take around 50ms */
24842484
struct dentry *dentry;
24852485
char *path;
2486-
int pathlen;
2487-
u64 pathbase;
24882486
bool do_sync = false;
24892487

24902488
dentry = d_find_alias(inode);
24912489
if (!dentry) {
24922490
do_sync = true;
24932491
} else {
2494-
path = ceph_mdsc_build_path(mdsc, dentry, &pathlen, &pathbase, 0);
2492+
struct ceph_path_info path_info;
2493+
path = ceph_mdsc_build_path(mdsc, dentry, &path_info, 0);
24952494
if (IS_ERR(path)) {
24962495
do_sync = true;
24972496
err = 0;
24982497
} else {
24992498
err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
25002499
}
2501-
ceph_mdsc_free_path(path, pathlen);
2500+
ceph_mdsc_free_path_info(&path_info);
25022501
dput(dentry);
25032502

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

0 commit comments

Comments
 (0)