nfsd: handle failure to collect pre/post-op attrs more sanely

Collecting pre_op_attrs can fail, in which case it's probably best to
fail the whole operation.

Change fh_fill_pre_attrs and fh_fill_both_attrs to return __be32, and
have the callers check the return code and abort the operation if it's
not nfs_ok.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index fc8d5b7..268ef57 100644 (file)
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -307,7 +307,9 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (!IS_POSIXACL(inode))
                iap->ia_mode &= ~current_umask();
 
-       fh_fill_pre_attrs(fhp);
+       status = fh_fill_pre_attrs(fhp);
+       if (status != nfs_ok)
+               goto out;
        host_err = vfs_create(&nop_mnt_idmap, inode, child, iap->ia_mode, true);
        if (host_err < 0) {
                status = nfserrno(host_err);

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5ae6708..cdf58d1 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -297,12 +297,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        }
 
        if (d_really_is_positive(child)) {
-               status = nfs_ok;
-
                /* NFSv4 protocol requires change attributes even though
                 * no change happened.
                 */
-               fh_fill_both_attrs(fhp);
+               status = fh_fill_both_attrs(fhp);
+               if (status != nfs_ok)
+                       goto out;
 
                switch (open->op_createmode) {
                case NFS4_CREATE_UNCHECKED:
@@ -345,7 +345,9 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (!IS_POSIXACL(inode))
                iap->ia_mode &= ~current_umask();
 
-       fh_fill_pre_attrs(fhp);
+       status = fh_fill_pre_attrs(fhp);
+       if (status != nfs_ok)
+               goto out;
        status = nfsd4_vfs_create(fhp, child, open);
        if (status != nfs_ok)
                goto out;
@@ -424,11 +426,11 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
        } else {
                status = nfsd_lookup(rqstp, current_fh,
                                     open->op_fname, open->op_fnamelen, *resfh);
-               if (!status)
+               if (status == nfs_ok)
                        /* NFSv4 protocol requires change attributes even though
                         * no change happened.
                         */
-                       fh_fill_both_attrs(current_fh);
+                       status = fh_fill_both_attrs(current_fh);
        }
        if (status)
                goto out;

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index c291389..355bf0d 100644 (file)
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -614,7 +614,7 @@ out_negative:
  * @fhp: file handle to be updated
  *
  */
-void fh_fill_pre_attrs(struct svc_fh *fhp)
+__be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
 {
        bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
        struct inode *inode;
@@ -622,12 +622,12 @@ void fh_fill_pre_attrs(struct svc_fh *fhp)
        __be32 err;
 
        if (fhp->fh_no_wcc || fhp->fh_pre_saved)
-               return;
+               return nfs_ok;
 
        inode = d_inode(fhp->fh_dentry);
        err = fh_getattr(fhp, &stat);
        if (err)
-               return;
+               return err;
 
        if (v4)
                fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
@@ -636,6 +636,7 @@ void fh_fill_pre_attrs(struct svc_fh *fhp)
        fhp->fh_pre_ctime = stat.ctime;
        fhp->fh_pre_size  = stat.size;
        fhp->fh_pre_saved = true;
+       return nfs_ok;
 }
 
 /**
@@ -643,26 +644,27 @@ void fh_fill_pre_attrs(struct svc_fh *fhp)
  * @fhp: file handle to be updated
  *
  */
-void fh_fill_post_attrs(struct svc_fh *fhp)
+__be32 fh_fill_post_attrs(struct svc_fh *fhp)
 {
        bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
        struct inode *inode = d_inode(fhp->fh_dentry);
        __be32 err;
 
        if (fhp->fh_no_wcc)
-               return;
+               return nfs_ok;
 
        if (fhp->fh_post_saved)
                printk("nfsd: inode locked twice during operation.\n");
 
        err = fh_getattr(fhp, &fhp->fh_post_attr);
        if (err)
-               return;
+               return err;
 
        fhp->fh_post_saved = true;
        if (v4)
                fhp->fh_post_change =
                        nfsd4_change_attribute(&fhp->fh_post_attr, inode);
+       return nfs_ok;
 }
 
 /**
@@ -672,16 +674,20 @@ void fh_fill_post_attrs(struct svc_fh *fhp)
  * This is used when the directory wasn't changed, but wcc attributes
  * are needed anyway.
  */
-void fh_fill_both_attrs(struct svc_fh *fhp)
+__be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp)
 {
-       fh_fill_post_attrs(fhp);
-       if (!fhp->fh_post_saved)
-               return;
+       __be32 err;
+
+       err = fh_fill_post_attrs(fhp);
+       if (err)
+               return err;
+
        fhp->fh_pre_change = fhp->fh_post_change;
        fhp->fh_pre_mtime = fhp->fh_post_attr.mtime;
        fhp->fh_pre_ctime = fhp->fh_post_attr.ctime;
        fhp->fh_pre_size = fhp->fh_post_attr.size;
        fhp->fh_pre_saved = true;
+       return nfs_ok;
 }
 
 /*

diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 4e0ecf0..40426f8 100644 (file)
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -294,7 +294,7 @@ static inline void fh_clear_pre_post_attrs(struct svc_fh *fhp)
 }
 
 u64 nfsd4_change_attribute(struct kstat *stat, struct inode *inode);
-extern void fh_fill_pre_attrs(struct svc_fh *fhp);
-extern void fh_fill_post_attrs(struct svc_fh *fhp);
-extern void fh_fill_both_attrs(struct svc_fh *fhp);
+__be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp);
+__be32 fh_fill_post_attrs(struct svc_fh *fhp);
+__be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp);
 #endif /* _LINUX_NFSD_NFSFH_H */

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 2c9074a..c463ef5 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1540,7 +1540,9 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
        dput(dchild);
        if (err)
                goto out_unlock;
-       fh_fill_pre_attrs(fhp);
+       err = fh_fill_pre_attrs(fhp);
+       if (err != nfs_ok)
+               goto out_unlock;
        err = nfsd_create_locked(rqstp, fhp, attrs, type, rdev, resfhp);
        fh_fill_post_attrs(fhp);
 out_unlock:
@@ -1635,13 +1637,16 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
                inode_unlock(dentry->d_inode);
                goto out_drop_write;
        }
-       fh_fill_pre_attrs(fhp);
+       err = fh_fill_pre_attrs(fhp);
+       if (err != nfs_ok)
+               goto out_unlock;
        host_err = vfs_symlink(&nop_mnt_idmap, d_inode(dentry), dnew, path);
        err = nfserrno(host_err);
        cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
        if (!err)
                nfsd_create_setattr(rqstp, fhp, resfhp, attrs);
        fh_fill_post_attrs(fhp);
+out_unlock:
        inode_unlock(dentry->d_inode);
        if (!err)
                err = nfserrno(commit_metadata(fhp));
@@ -1703,7 +1708,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
        err = nfserr_noent;
        if (d_really_is_negative(dold))
                goto out_dput;
-       fh_fill_pre_attrs(ffhp);
+       err = fh_fill_pre_attrs(ffhp);
+       if (err != nfs_ok)
+               goto out_dput;
        host_err = vfs_link(dold, &nop_mnt_idmap, dirp, dnew, NULL);
        fh_fill_post_attrs(ffhp);
        inode_unlock(dirp);
@@ -1789,8 +1796,12 @@ retry:
        }
 
        trap = lock_rename(tdentry, fdentry);
-       fh_fill_pre_attrs(ffhp);
-       fh_fill_pre_attrs(tfhp);
+       err = fh_fill_pre_attrs(ffhp);
+       if (err != nfs_ok)
+               goto out_unlock;
+       err = fh_fill_pre_attrs(tfhp);
+       if (err != nfs_ok)
+               goto out_unlock;
 
        odentry = lookup_one_len(fname, fdentry, flen);
        host_err = PTR_ERR(odentry);
@@ -1857,6 +1868,7 @@ retry:
                fh_fill_post_attrs(ffhp);
                fh_fill_post_attrs(tfhp);
        }
+out_unlock:
        unlock_rename(tdentry, fdentry);
        fh_drop_write(ffhp);
 
@@ -1916,12 +1928,14 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
                goto out_unlock;
        }
        rinode = d_inode(rdentry);
-       ihold(rinode);
+       err = fh_fill_pre_attrs(fhp);
+       if (err != nfs_ok)
+               goto out_unlock;
 
+       ihold(rinode);
        if (!type)
                type = d_inode(rdentry)->i_mode & S_IFMT;
 
-       fh_fill_pre_attrs(fhp);
        if (type != S_IFDIR) {
                int retries;
 
@@ -2341,16 +2355,18 @@ nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name)
                return nfserrno(ret);
 
        inode_lock(fhp->fh_dentry->d_inode);
-       fh_fill_pre_attrs(fhp);
-
+       err = fh_fill_pre_attrs(fhp);
+       if (err != nfs_ok)
+               goto out_unlock;
        ret = __vfs_removexattr_locked(&nop_mnt_idmap, fhp->fh_dentry,
                                       name, NULL);
-
+       err = nfsd_xattr_errno(ret);
        fh_fill_post_attrs(fhp);
+out_unlock:
        inode_unlock(fhp->fh_dentry->d_inode);
        fh_drop_write(fhp);
 
-       return nfsd_xattr_errno(ret);
+       return err;
 }
 
 __be32
@@ -2368,15 +2384,17 @@ nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
        if (ret)
                return nfserrno(ret);
        inode_lock(fhp->fh_dentry->d_inode);
-       fh_fill_pre_attrs(fhp);
-
-       ret = __vfs_setxattr_locked(&nop_mnt_idmap, fhp->fh_dentry, name, buf,
-                                   len, flags, NULL);
+       err = fh_fill_pre_attrs(fhp);
+       if (err != nfs_ok)
+               goto out_unlock;
+       ret = __vfs_setxattr_locked(&nop_mnt_idmap, fhp->fh_dentry,
+                                   name, buf, len, flags, NULL);
        fh_fill_post_attrs(fhp);
+       err = nfsd_xattr_errno(ret);
+out_unlock:
        inode_unlock(fhp->fh_dentry->d_inode);
        fh_drop_write(fhp);
-
-       return nfsd_xattr_errno(ret);
+       return err;
 }
 #endif