fs/9p: Prevent parallel rename when doing fid_lookup

During fid lookup we need to make sure that the dentry->d_parent doesn't
change so that we can safely walk the parent dentries. To ensure that
we need to prevent cross directory rename during fid_lookup. Add a
per superblock rename_sem rw_semaphore to prevent parallel fid lookup and
rename.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
Signed-off-by: Eric Van Hensbergen <ericvh@gmail.com>

diff --git a/fs/9p/fid.c b/fs/9p/fid.c
index 5d6cfcb..3585636 100644 (file)
--- a/fs/9p/fid.c
+++ b/fs/9p/fid.c
@@ -97,6 +97,34 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, u32 uid, int any)
        return ret;
 }
 
+/*
+ * We need to hold v9ses->rename_sem as long as we hold references
+ * to returned path array. Array element contain pointers to
+ * dentry names.
+ */
+static int build_path_from_dentry(struct v9fs_session_info *v9ses,
+                                 struct dentry *dentry, char ***names)
+{
+       int n = 0, i;
+       char **wnames;
+       struct dentry *ds;
+
+       for (ds = dentry; !IS_ROOT(ds); ds = ds->d_parent)
+               n++;
+
+       wnames = kmalloc(sizeof(char *) * n, GFP_KERNEL);
+       if (!wnames)
+               goto err_out;
+
+       for (ds = dentry, i = (n-1); i >= 0; i--, ds = ds->d_parent)
+               wnames[i] = (char  *)ds->d_name.name;
+
+       *names = wnames;
+       return n;
+err_out:
+       return -ENOMEM;
+}
+
 /**
  * v9fs_fid_lookup - lookup for a fid, try to walk if not found
  * @dentry: dentry to look for fid in
@@ -112,7 +140,7 @@ struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
        int i, n, l, clone, any, access;
        u32 uid;
        struct p9_fid *fid, *old_fid = NULL;
-       struct dentry *d, *ds;
+       struct dentry *ds;
        struct v9fs_session_info *v9ses;
        char **wnames, *uname;
 
@@ -139,50 +167,62 @@ struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
        fid = v9fs_fid_find(dentry, uid, any);
        if (fid)
                return fid;
-
+       /*
+        * we don't have a matching fid. To do a TWALK we need
+        * parent fid. We need to prevent rename when we want to
+        * look at the parent.
+        */
+       down_read(&v9ses->rename_sem);
        ds = dentry->d_parent;
        fid = v9fs_fid_find(ds, uid, any);
-       if (!fid) { /* walk from the root */
-               n = 0;
-               for (ds = dentry; !IS_ROOT(ds); ds = ds->d_parent)
-                       n++;
-
-               fid = v9fs_fid_find(ds, uid, any);
-               if (!fid) { /* the user is not attached to the fs yet */
-                       if (access == V9FS_ACCESS_SINGLE)
-                               return ERR_PTR(-EPERM);
-
-                       if (v9fs_proto_dotu(v9ses) ||
-                               v9fs_proto_dotl(v9ses))
-                               uname = NULL;
-                       else
-                               uname = v9ses->uname;
+       if (fid) {
+               /* Found the parent fid do a lookup with that */
+               fid = p9_client_walk(fid, 1, (char **)&dentry->d_name.name, 1);
+               goto fid_out;
+       }
+       up_read(&v9ses->rename_sem);
 
-                       fid = p9_client_attach(v9ses->clnt, NULL, uname, uid,
-                               v9ses->aname);
+       /* start from the root and try to do a lookup */
+       fid = v9fs_fid_find(dentry->d_sb->s_root, uid, any);
+       if (!fid) {
+               /* the user is not attached to the fs yet */
+               if (access == V9FS_ACCESS_SINGLE)
+                       return ERR_PTR(-EPERM);
 
-                       if (IS_ERR(fid))
-                               return fid;
+               if (v9fs_proto_dotu(v9ses) || v9fs_proto_dotl(v9ses))
+                               uname = NULL;
+               else
+                       uname = v9ses->uname;
 
-                       v9fs_fid_add(ds, fid);
-               }
-       } else /* walk from the parent */
-               n = 1;
+               fid = p9_client_attach(v9ses->clnt, NULL, uname, uid,
+                                      v9ses->aname);
+               if (IS_ERR(fid))
+                       return fid;
 
-       if (ds == dentry)
+               v9fs_fid_add(dentry->d_sb->s_root, fid);
+       }
+       /* If we are root ourself just return that */
+       if (dentry->d_sb->s_root == dentry)
                return fid;
-
-       wnames = kmalloc(sizeof(char *) * n, GFP_KERNEL);
-       if (!wnames)
-               return ERR_PTR(-ENOMEM);
-
-       for (d = dentry, i = (n-1); i >= 0; i--, d = d->d_parent)
-               wnames[i] = (char *) d->d_name.name;
-
+       /*
+        * Do a multipath walk with attached root.
+        * When walking parent we need to make sure we
+        * don't have a parallel rename happening
+        */
+       down_read(&v9ses->rename_sem);
+       n  = build_path_from_dentry(v9ses, dentry, &wnames);
+       if (n < 0) {
+               fid = ERR_PTR(n);
+               goto err_out;
+       }
        clone = 1;
        i = 0;
        while (i < n) {
                l = min(n - i, P9_MAXWELEM);
+               /*
+                * We need to hold rename lock when doing a multipath
+                * walk to ensure none of the patch component change
+                */
                fid = p9_client_walk(fid, l, &wnames[i], clone);
                if (IS_ERR(fid)) {
                        if (old_fid) {
@@ -194,15 +234,17 @@ struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
                                p9_client_clunk(old_fid);
                        }
                        kfree(wnames);
-                       return fid;
+                       goto err_out;
                }
                old_fid = fid;
                i += l;
                clone = 0;
        }
-
        kfree(wnames);
+fid_out:
        v9fs_fid_add(dentry, fid);
+err_out:
+       up_read(&v9ses->rename_sem);
        return fid;
 }
 

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 3c49201..38dc0e0 100644 (file)
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -237,6 +237,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
                __putname(v9ses->uname);
                return ERR_PTR(-ENOMEM);
        }
+       init_rwsem(&v9ses->rename_sem);
 
        rc = bdi_setup_and_register(&v9ses->bdi, "9p", BDI_CAP_MAP_COPY);
        if (rc) {

diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index bec4d0b..4c963c9 100644 (file)
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -104,6 +104,7 @@ struct v9fs_session_info {
        struct p9_client *clnt; /* 9p client */
        struct list_head slist; /* list of sessions registered with v9fs */
        struct backing_dev_info bdi;
+       struct rw_semaphore rename_sem;
 };
 
 struct p9_fid *v9fs_session_init(struct v9fs_session_info *, const char *,

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 39352ef..75c261f 100644 (file)
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -948,6 +948,7 @@ static struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
 
        sb = dir->i_sb;
        v9ses = v9fs_inode2v9ses(dir);
+       /* We can walk d_parent because we hold the dir->i_mutex */
        dfid = v9fs_fid_lookup(dentry->d_parent);
        if (IS_ERR(dfid))
                return ERR_CAST(dfid);
@@ -1055,27 +1056,33 @@ v9fs_vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
                goto clunk_olddir;
        }
 
+       down_write(&v9ses->rename_sem);
        if (v9fs_proto_dotl(v9ses)) {
                retval = p9_client_rename(oldfid, newdirfid,
                                        (char *) new_dentry->d_name.name);
                if (retval != -ENOSYS)
                        goto clunk_newdir;
        }
+       if (old_dentry->d_parent != new_dentry->d_parent) {
+               /*
+                * 9P .u can only handle file rename in the same directory
+                */
 
-       /* 9P can only handle file rename in the same directory */
-       if (memcmp(&olddirfid->qid, &newdirfid->qid, sizeof(newdirfid->qid))) {
                P9_DPRINTK(P9_DEBUG_ERROR,
                                "old dir and new dir are different\n");
                retval = -EXDEV;
                goto clunk_newdir;
        }
-
        v9fs_blank_wstat(&wstat);
        wstat.muid = v9ses->uname;
        wstat.name = (char *) new_dentry->d_name.name;
        retval = p9_client_wstat(oldfid, &wstat);
 
 clunk_newdir:
+       if (!retval)
+               /* successful rename */
+               d_move(old_dentry, new_dentry);
+       up_write(&v9ses->rename_sem);
        p9_client_clunk(newdirfid);
 
 clunk_olddir:

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 0a2e203..4b9ede0 100644 (file)
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -287,4 +287,5 @@ struct file_system_type v9fs_fs_type = {
        .get_sb = v9fs_get_sb,
        .kill_sb = v9fs_kill_super,
        .owner = THIS_MODULE,
+       .fs_flags = FS_RENAME_DOES_D_MOVE,
 };