nfs: don't open in ->d_revalidate

NFSv4 can't do reliable opens in d_revalidate, since it cannot know whether a
mount needs to be followed or not.  It does check d_mountpoint() on the dentry,
which can result in a weird error if the VFS found that the mount does not in
fact need to be followed, e.g.:

  # mount --bind /mnt/nfs /mnt/nfs-clone
  # echo something > /mnt/nfs/tmp/bar
  # echo x > /tmp/file
  # mount --bind /tmp/file /mnt/nfs-clone/tmp/bar
  # cat  /mnt/nfs/tmp/bar
  cat: /mnt/nfs/tmp/bar: Not a directory

Which should, by any sane filesystem, result in "something" being printed.

So instead do the open in f_op->open() and in the unlikely case that the cached
dentry turned out to be invalid, drop the dentry and return EOPENSTALE to let
the VFS retry.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

2 files changed, 78 insertions, 55 deletions

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 0989a2099688..f430057ff3b3 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1354,10 +1354,10 @@ out:
 }
 
 #ifdef CONFIG_NFS_V4
-static int nfs_open_revalidate(struct dentry *, struct nameidata *);
+static int nfs4_lookup_revalidate(struct dentry *, struct nameidata *);
 
 const struct dentry_operations nfs4_dentry_operations = {
-        .d_revalidate   = nfs_open_revalidate,
+        .d_revalidate   = nfs4_lookup_revalidate,
         .d_delete       = nfs_dentry_delete,
         .d_iput         = nfs_dentry_iput,
         .d_automount    = nfs_d_automount,
@@ -1519,13 +1519,11 @@ no_open:
         return nfs_lookup(dir, dentry, nd);
 }
 
-static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
+static int nfs4_lookup_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
         struct dentry *parent = NULL;
         struct inode *inode;
         struct inode *dir;
-        struct nfs_open_context *ctx;
-        struct iattr attr;
         int openflags, ret = 0;
 
         if (nd->flags & LOOKUP_RCU)
@@ -1554,57 +1552,13 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
         /* We cannot do exclusive creation on a positive dentry */
         if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
                 goto no_open_dput;
-        /* We can't create new files here */
-        openflags &= ~(O_CREAT|O_EXCL);
-
-        ctx = create_nfs_open_context(dentry, openflags);
-        ret = PTR_ERR(ctx);
-        if (IS_ERR(ctx))
-                goto out;
 
-        attr.ia_valid = ATTR_OPEN;
-        if (openflags & O_TRUNC) {
-                attr.ia_valid |= ATTR_SIZE;
-                attr.ia_size = 0;
-                nfs_wb_all(inode);
-        }
-
-        /*
-         * Note: we're not holding inode->i_mutex and so may be racing with
-         * operations that change the directory. We therefore save the
-         * change attribute *before* we do the RPC call.
-         */
-        inode = NFS_PROTO(dir)->open_context(dir, ctx, openflags, &attr);
-        if (IS_ERR(inode)) {
-                ret = PTR_ERR(inode);
-                switch (ret) {
-                case -EPERM:
-                case -EACCES:
-                case -EDQUOT:
-                case -ENOSPC:
-                case -EROFS:
-                        goto out_put_ctx;
-                default:
-                        goto out_drop;
-                }
-        }
-        iput(inode);
-        if (inode != dentry->d_inode)
-                goto out_drop;
+        /* Let f_op->open() actually open (and revalidate) the file */
+        ret = 1;
 
-        nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
-        ret = nfs_intent_set_file(nd, ctx);
-        if (ret >= 0)
-                ret = 1;
 out:
         dput(parent);
         return ret;
-out_drop:
-        d_drop(dentry);
-        ret = 0;
-out_put_ctx:
-        put_nfs_open_context(ctx);
-        goto out;
 
 no_open_dput:
         dput(parent);
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 56311ca5f9f8..a6708e6b438d 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -879,12 +879,81 @@ const struct file_operations nfs_file_operations = {
 static int
 nfs4_file_open(struct inode *inode, struct file *filp)
 {
+        struct nfs_open_context *ctx;
+        struct dentry *dentry = filp->f_path.dentry;
+        struct dentry *parent = NULL;
+        struct inode *dir;
+        unsigned openflags = filp->f_flags;
+        struct iattr attr;
+        int err;
+
+        BUG_ON(inode != dentry->d_inode);
         /*
-         * NFSv4 opens are handled in d_lookup and d_revalidate. If we get to
-         * this point, then something is very wrong
+         * If no cached dentry exists or if it's negative, NFSv4 handled the
+         * opens in ->lookup() or ->create().
+         *
+         * We only get this far for a cached positive dentry.  We skipped
+         * revalidation, so handle it here by dropping the dentry and returning
+         * -EOPENSTALE.  The VFS will retry the lookup/create/open.
          */
-        dprintk("NFS: %s called! inode=%p filp=%p\n", __func__, inode, filp);
-        return -ENOTDIR;
+
+        dprintk("NFS: open file(%s/%s)\n",
+                dentry->d_parent->d_name.name,
+                dentry->d_name.name);
+
+        if ((openflags & O_ACCMODE) == 3)
+                openflags--;
+
+        /* We can't create new files here */
+        openflags &= ~(O_CREAT|O_EXCL);
+
+        parent = dget_parent(dentry);
+        dir = parent->d_inode;
+
+        ctx = alloc_nfs_open_context(filp->f_path.dentry, filp->f_mode);
+        err = PTR_ERR(ctx);
+        if (IS_ERR(ctx))
+                goto out;
+
+        attr.ia_valid = ATTR_OPEN;
+        if (openflags & O_TRUNC) {
+                attr.ia_valid |= ATTR_SIZE;
+                attr.ia_size = 0;
+                nfs_wb_all(inode);
+        }
+
+        inode = NFS_PROTO(dir)->open_context(dir, ctx, openflags, &attr);
+        if (IS_ERR(inode)) {
+                err = PTR_ERR(inode);
+                switch (err) {
+                case -EPERM:
+                case -EACCES:
+                case -EDQUOT:
+                case -ENOSPC:
+                case -EROFS:
+                        goto out_put_ctx;
+                default:
+                        goto out_drop;
+                }
+        }
+        iput(inode);
+        if (inode != dentry->d_inode)
+                goto out_drop;
+
+        nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+        nfs_file_set_open_context(filp, ctx);
+        err = 0;
+
+out_put_ctx:
+        put_nfs_open_context(ctx);
+out:
+        dput(parent);
+        return err;
+
+out_drop:
+        d_drop(dentry);
+        err = -EOPENSTALE;
+        goto out_put_ctx;
 }
 
 const struct file_operations nfs4_file_operations = {