ceph: vet the target and parent inodes before updating dentry lease
authorJeff Layton <jlayton@redhat.com>
Fri, 27 Jan 2017 14:13:57 +0000 (09:13 -0500)
committerIlya Dryomov <idryomov@gmail.com>
Mon, 20 Feb 2017 11:16:09 +0000 (12:16 +0100)
In a later patch, we're going to need to allow ceph_fill_trace to
update the dentry's lease when the parent is not locked. This is
potentially racy though -- by the time we get around to processing the
trace, the parent may have already changed.

Change update_dentry_lease to take a ceph_vino pointer and use that to
ensure that the dentry's parent still matches it before updating the
lease.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Yan, Zheng <zyan@redhat.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
fs/ceph/inode.c

index 542e3c6b0f1b5ec1da5b96b4bb8273604059b508..f5111df561e4c7ae8fb7e15079982772845e2028 100644 (file)
@@ -1016,7 +1016,9 @@ out:
 static void update_dentry_lease(struct dentry *dentry,
                                struct ceph_mds_reply_lease *lease,
                                struct ceph_mds_session *session,
-                               unsigned long from_time)
+                               unsigned long from_time,
+                               struct ceph_vino *tgt_vino,
+                               struct ceph_vino *dir_vino)
 {
        struct ceph_dentry_info *di = ceph_dentry(dentry);
        long unsigned duration = le32_to_cpu(lease->duration_ms);
@@ -1024,13 +1026,27 @@ static void update_dentry_lease(struct dentry *dentry,
        long unsigned half_ttl = from_time + (duration * HZ / 2) / 1000;
        struct inode *dir;
 
+       /*
+        * Make sure dentry's inode matches tgt_vino. NULL tgt_vino means that
+        * we expect a negative dentry.
+        */
+       if (!tgt_vino && d_really_is_positive(dentry))
+               return;
+
+       if (tgt_vino && (d_really_is_negative(dentry) ||
+                       !ceph_ino_compare(d_inode(dentry), tgt_vino)))
+               return;
+
        spin_lock(&dentry->d_lock);
        dout("update_dentry_lease %p duration %lu ms ttl %lu\n",
             dentry, duration, ttl);
 
-       /* make lease_rdcache_gen match directory */
        dir = d_inode(dentry->d_parent);
 
+       /* make sure parent matches dir_vino */
+       if (!ceph_ino_compare(dir, dir_vino))
+               goto out_unlock;
+
        /* only track leases on regular dentries */
        if (ceph_snap(dir) != CEPH_NOSNAP)
                goto out_unlock;
@@ -1113,7 +1129,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
        struct ceph_mds_session *session = req->r_session;
        struct ceph_mds_reply_info_parsed *rinfo = &req->r_reply_info;
        struct inode *in = NULL;
-       struct ceph_vino vino;
+       struct ceph_vino tvino, dvino;
        struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
        int err = 0;
 
@@ -1154,8 +1170,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
                        dname.name = rinfo->dname;
                        dname.len = rinfo->dname_len;
                        dname.hash = full_name_hash(parent, dname.name, dname.len);
-                       vino.ino = le64_to_cpu(rinfo->targeti.in->ino);
-                       vino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
+                       tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
+                       tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
 retry_lookup:
                        dn = d_lookup(parent, &dname);
                        dout("d_lookup on parent=%p name=%.*s got %p\n",
@@ -1172,8 +1188,8 @@ retry_lookup:
                                }
                                err = 0;
                        } else if (d_really_is_positive(dn) &&
-                                  (ceph_ino(d_inode(dn)) != vino.ino ||
-                                   ceph_snap(d_inode(dn)) != vino.snap)) {
+                                  (ceph_ino(d_inode(dn)) != tvino.ino ||
+                                   ceph_snap(d_inode(dn)) != tvino.snap)) {
                                dout(" dn %p points to wrong inode %p\n",
                                     dn, d_inode(dn));
                                d_delete(dn);
@@ -1187,10 +1203,10 @@ retry_lookup:
        }
 
        if (rinfo->head->is_target) {
-               vino.ino = le64_to_cpu(rinfo->targeti.in->ino);
-               vino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
+               tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
+               tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
 
-               in = ceph_get_inode(sb, vino);
+               in = ceph_get_inode(sb, tvino);
                if (IS_ERR(in)) {
                        err = PTR_ERR(in);
                        goto done;
@@ -1231,10 +1247,12 @@ retry_lookup:
                BUG_ON(!dn);
                BUG_ON(!dir);
                BUG_ON(d_inode(dn->d_parent) != dir);
-               BUG_ON(ceph_ino(dir) !=
-                      le64_to_cpu(rinfo->diri.in->ino));
-               BUG_ON(ceph_snap(dir) !=
-                      le64_to_cpu(rinfo->diri.in->snapid));
+
+               dvino.ino = le64_to_cpu(rinfo->diri.in->ino);
+               dvino.snap = le64_to_cpu(rinfo->diri.in->snapid);
+
+               BUG_ON(ceph_ino(dir) != dvino.ino);
+               BUG_ON(ceph_snap(dir) != dvino.snap);
 
                /* do we have a lease on the whole dir? */
                have_dir_cap =
@@ -1291,7 +1309,8 @@ retry_lookup:
                                        d_add(dn, NULL);
                                update_dentry_lease(dn, rinfo->dlease,
                                                    session,
-                                                   req->r_request_started);
+                                                   req->r_request_started,
+                                                   NULL, &dvino);
                        }
                        goto done;
                }
@@ -1314,9 +1333,13 @@ retry_lookup:
                        have_lease = false;
                }
 
-               if (have_lease)
+               if (have_lease) {
+                       tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
+                       tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
                        update_dentry_lease(dn, rinfo->dlease, session,
-                                           req->r_request_started);
+                                           req->r_request_started,
+                                           &tvino, &dvino);
+               }
                dout(" final dn %p\n", dn);
        } else if ((req->r_op == CEPH_MDS_OP_LOOKUPSNAP ||
                    req->r_op == CEPH_MDS_OP_MKSNAP) &&
@@ -1490,14 +1513,14 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
        /* FIXME: release caps/leases if error occurs */
        for (i = 0; i < rinfo->dir_nr; i++) {
                struct ceph_mds_reply_dir_entry *rde = rinfo->dir_entries + i;
-               struct ceph_vino vino;
+               struct ceph_vino tvino, dvino;
 
                dname.name = rde->name;
                dname.len = rde->name_len;
                dname.hash = full_name_hash(parent, dname.name, dname.len);
 
-               vino.ino = le64_to_cpu(rde->inode.in->ino);
-               vino.snap = le64_to_cpu(rde->inode.in->snapid);
+               tvino.ino = le64_to_cpu(rde->inode.in->ino);
+               tvino.snap = le64_to_cpu(rde->inode.in->snapid);
 
                if (rinfo->hash_order) {
                        u32 hash = ceph_str_hash(ci->i_dir_layout.dl_dir_hash,
@@ -1526,8 +1549,8 @@ retry_lookup:
                                goto out;
                        }
                } else if (d_really_is_positive(dn) &&
-                          (ceph_ino(d_inode(dn)) != vino.ino ||
-                           ceph_snap(d_inode(dn)) != vino.snap)) {
+                          (ceph_ino(d_inode(dn)) != tvino.ino ||
+                           ceph_snap(d_inode(dn)) != tvino.snap)) {
                        dout(" dn %p points to wrong inode %p\n",
                             dn, d_inode(dn));
                        d_delete(dn);
@@ -1539,7 +1562,7 @@ retry_lookup:
                if (d_really_is_positive(dn)) {
                        in = d_inode(dn);
                } else {
-                       in = ceph_get_inode(parent->d_sb, vino);
+                       in = ceph_get_inode(parent->d_sb, tvino);
                        if (IS_ERR(in)) {
                                dout("new_inode badness\n");
                                d_drop(dn);
@@ -1584,8 +1607,9 @@ retry_lookup:
 
                ceph_dentry(dn)->offset = rde->offset;
 
+               dvino = ceph_vino(d_inode(parent));
                update_dentry_lease(dn, rde->lease, req->r_session,
-                                   req->r_request_started);
+                                   req->r_request_started, &tvino, &dvino);
 
                if (err == 0 && skipped == 0 && cache_ctl.index >= 0) {
                        ret = fill_readdir_cache(d_inode(parent), dn,