Skip to content

Commit bec324f

Browse files
Alex Markuzeidryomov
authored andcommitted
ceph: fix race condition where r_parent becomes stale before sending message
When the parent directory's i_rwsem is not locked, req->r_parent may become stale due to concurrent operations (e.g. rename) between dentry lookup and message creation. Validate that r_parent matches the encoded parent inode and update to the correct inode if a mismatch is detected. [ idryomov: folded a follow-up fix from Alex to drop extra reference from ceph_get_reply_dir() in ceph_fill_trace(): ceph_get_reply_dir() may return a different, referenced inode when r_parent is stale and the parent directory lock is not held. ceph_fill_trace() used that inode but failed to drop the reference when it differed from req->r_parent, leaking an inode reference. Keep the directory inode in a local variable and iput() it at function end if it does not match req->r_parent. ] 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>
1 parent 15f519e commit bec324f

File tree

1 file changed

+69
-12
lines changed

1 file changed

+69
-12
lines changed

fs/ceph/inode.c

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,52 @@ static int ceph_set_ino_cb(struct inode *inode, void *data)
5555
return 0;
5656
}
5757

58+
/*
59+
* Check if the parent inode matches the vino from directory reply info
60+
*/
61+
static inline bool ceph_vino_matches_parent(struct inode *parent,
62+
struct ceph_vino vino)
63+
{
64+
return ceph_ino(parent) == vino.ino && ceph_snap(parent) == vino.snap;
65+
}
66+
67+
/*
68+
* Validate that the directory inode referenced by @req->r_parent matches the
69+
* inode number and snapshot id contained in the reply's directory record. If
70+
* they do not match – which can theoretically happen if the parent dentry was
71+
* moved between the time the request was issued and the reply arrived – fall
72+
* back to looking up the correct inode in the inode cache.
73+
*
74+
* A reference is *always* returned. Callers that receive a different inode
75+
* than the original @parent are responsible for dropping the extra reference
76+
* once the reply has been processed.
77+
*/
78+
static struct inode *ceph_get_reply_dir(struct super_block *sb,
79+
struct inode *parent,
80+
struct ceph_mds_reply_info_parsed *rinfo)
81+
{
82+
struct ceph_vino vino;
83+
84+
if (unlikely(!rinfo->diri.in))
85+
return parent; /* nothing to compare against */
86+
87+
/* If we didn't have a cached parent inode to begin with, just bail out. */
88+
if (!parent)
89+
return NULL;
90+
91+
vino.ino = le64_to_cpu(rinfo->diri.in->ino);
92+
vino.snap = le64_to_cpu(rinfo->diri.in->snapid);
93+
94+
if (likely(ceph_vino_matches_parent(parent, vino)))
95+
return parent; /* matches – use the original reference */
96+
97+
/* Mismatch – this should be rare. Emit a WARN and obtain the correct inode. */
98+
WARN_ONCE(1, "ceph: reply dir mismatch (parent valid %llx.%llx reply %llx.%llx)\n",
99+
ceph_ino(parent), ceph_snap(parent), vino.ino, vino.snap);
100+
101+
return ceph_get_inode(sb, vino, NULL);
102+
}
103+
58104
/**
59105
* ceph_new_inode - allocate a new inode in advance of an expected create
60106
* @dir: parent directory for new inode
@@ -1523,6 +1569,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15231569
struct ceph_vino tvino, dvino;
15241570
struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);
15251571
struct ceph_client *cl = fsc->client;
1572+
struct inode *parent_dir = NULL;
15261573
int err = 0;
15271574

15281575
doutc(cl, "%p is_dentry %d is_target %d\n", req,
@@ -1536,10 +1583,17 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15361583
}
15371584

15381585
if (rinfo->head->is_dentry) {
1539-
struct inode *dir = req->r_parent;
1540-
1541-
if (dir) {
1542-
err = ceph_fill_inode(dir, NULL, &rinfo->diri,
1586+
/*
1587+
* r_parent may be stale, in cases when R_PARENT_LOCKED is not set,
1588+
* so we need to get the correct inode
1589+
*/
1590+
parent_dir = ceph_get_reply_dir(sb, req->r_parent, rinfo);
1591+
if (unlikely(IS_ERR(parent_dir))) {
1592+
err = PTR_ERR(parent_dir);
1593+
goto done;
1594+
}
1595+
if (parent_dir) {
1596+
err = ceph_fill_inode(parent_dir, NULL, &rinfo->diri,
15431597
rinfo->dirfrag, session, -1,
15441598
&req->r_caps_reservation);
15451599
if (err < 0)
@@ -1548,14 +1602,14 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15481602
WARN_ON_ONCE(1);
15491603
}
15501604

1551-
if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME &&
1605+
if (parent_dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME &&
15521606
test_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags) &&
15531607
!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
15541608
bool is_nokey = false;
15551609
struct qstr dname;
15561610
struct dentry *dn, *parent;
15571611
struct fscrypt_str oname = FSTR_INIT(NULL, 0);
1558-
struct ceph_fname fname = { .dir = dir,
1612+
struct ceph_fname fname = { .dir = parent_dir,
15591613
.name = rinfo->dname,
15601614
.ctext = rinfo->altname,
15611615
.name_len = rinfo->dname_len,
@@ -1564,10 +1618,10 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15641618
BUG_ON(!rinfo->head->is_target);
15651619
BUG_ON(req->r_dentry);
15661620

1567-
parent = d_find_any_alias(dir);
1621+
parent = d_find_any_alias(parent_dir);
15681622
BUG_ON(!parent);
15691623

1570-
err = ceph_fname_alloc_buffer(dir, &oname);
1624+
err = ceph_fname_alloc_buffer(parent_dir, &oname);
15711625
if (err < 0) {
15721626
dput(parent);
15731627
goto done;
@@ -1576,7 +1630,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15761630
err = ceph_fname_to_usr(&fname, NULL, &oname, &is_nokey);
15771631
if (err < 0) {
15781632
dput(parent);
1579-
ceph_fname_free_buffer(dir, &oname);
1633+
ceph_fname_free_buffer(parent_dir, &oname);
15801634
goto done;
15811635
}
15821636
dname.name = oname.name;
@@ -1595,7 +1649,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
15951649
dname.len, dname.name, dn);
15961650
if (!dn) {
15971651
dput(parent);
1598-
ceph_fname_free_buffer(dir, &oname);
1652+
ceph_fname_free_buffer(parent_dir, &oname);
15991653
err = -ENOMEM;
16001654
goto done;
16011655
}
@@ -1610,12 +1664,12 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
16101664
ceph_snap(d_inode(dn)) != tvino.snap)) {
16111665
doutc(cl, " dn %p points to wrong inode %p\n",
16121666
dn, d_inode(dn));
1613-
ceph_dir_clear_ordered(dir);
1667+
ceph_dir_clear_ordered(parent_dir);
16141668
d_delete(dn);
16151669
dput(dn);
16161670
goto retry_lookup;
16171671
}
1618-
ceph_fname_free_buffer(dir, &oname);
1672+
ceph_fname_free_buffer(parent_dir, &oname);
16191673

16201674
req->r_dentry = dn;
16211675
dput(parent);
@@ -1794,6 +1848,9 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
17941848
&dvino, ptvino);
17951849
}
17961850
done:
1851+
/* Drop extra ref from ceph_get_reply_dir() if it returned a new inode */
1852+
if (unlikely(!IS_ERR_OR_NULL(parent_dir) && parent_dir != req->r_parent))
1853+
iput(parent_dir);
17971854
doutc(cl, "done err=%d\n", err);
17981855
return err;
17991856
}

0 commit comments

Comments
 (0)