nfsd: Fix NFSv3 atomicity bugs in nfsd_setattr()

The main point of the guarded SETATTR is to prevent races with other
WRITE and SETATTR calls. That requires that the check of the guard time
against the inode ctime be done after taking the inode lock.

Furthermore, we need to take into account the 32-bit nature of
timestamps in NFSv3, and the possibility that files may change at a
faster rate than once a second.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: NeilBrown <neilb@suse.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index b78eceebd945e30f4c38da1c12f9ff171ae1bca0..dfcc957e460d64b9936340f27badc2ad55b59405 100644 (file)
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -71,13 +71,15 @@ nfsd3_proc_setattr(struct svc_rqst *rqstp)
        struct nfsd_attrs attrs = {
                .na_iattr       = &argp->attrs,
        };
+       const struct timespec64 *guardtime = NULL;
 
        dprintk("nfsd: SETATTR(3)  %s\n",
                                SVCFH_fmt(&argp->fh));
 
        fh_copy(&resp->fh, &argp->fh);
-       resp->status = nfsd_setattr(rqstp, &resp->fh, &attrs,
-                                   argp->check_guard, argp->guardtime);
+       if (argp->check_guard)
+               guardtime = &argp->guardtime;
+       resp->status = nfsd_setattr(rqstp, &resp->fh, &attrs, guardtime);
        return rpc_success;
 }
 

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index f32128955ec8d1dd0e2130e04155aada2f6cb6c1..a7a07470c1f846408e1b31d97bc7a55fa471bb5a 100644 (file)
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -295,17 +295,14 @@ svcxdr_decode_sattr3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 static bool
 svcxdr_decode_sattrguard3(struct xdr_stream *xdr, struct nfsd3_sattrargs *args)
 {
-       __be32 *p;
        u32 check;
 
        if (xdr_stream_decode_bool(xdr, &check) < 0)
                return false;
        if (check) {
-               p = xdr_inline_decode(xdr, XDR_UNIT * 2);
-               if (!p)
+               if (!svcxdr_decode_nfstime3(xdr, &args->guardtime))
                        return false;
                args->check_guard = 1;
-               args->guardtime = be32_to_cpup(p);
        } else
                args->check_guard = 0;
 

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2f524f45350824eacf99cacf0f2fdb8410148bee..2927b1263f086d8f0e64e8858d290c6a81822d36 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1171,8 +1171,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                goto out;
        save_no_wcc = cstate->current_fh.fh_no_wcc;
        cstate->current_fh.fh_no_wcc = true;
-       status = nfsd_setattr(rqstp, &cstate->current_fh, &attrs,
-                               0, (time64_t)0);
+       status = nfsd_setattr(rqstp, &cstate->current_fh, &attrs, NULL);
        cstate->current_fh.fh_no_wcc = save_no_wcc;
        if (!status)
                status = nfserrno(attrs.na_labelerr);

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7476726634f5318a24fb9b8d9989d604cb2286d3..aee12adf059811506a8a4c8b662b6b2b5c30a37f 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5460,7 +5460,7 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
                return 0;
        if (!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
                return nfserr_inval;
-       return nfsd_setattr(rqstp, fh, &attrs, 0, (time64_t)0);
+       return nfsd_setattr(rqstp, fh, &attrs, NULL);
 }
 
 static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index a7315928a760794ecb96507933629fec734651bb..36370b957b63378f023005b73d0af27363d564e0 100644 (file)
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -103,7 +103,7 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
                }
        }
 
-       resp->status = nfsd_setattr(rqstp, fhp, &attrs, 0, (time64_t)0);
+       resp->status = nfsd_setattr(rqstp, fhp, &attrs, NULL);
        if (resp->status != nfs_ok)
                goto out;
 
@@ -390,8 +390,8 @@ nfsd_proc_create(struct svc_rqst *rqstp)
                 */
                attr->ia_valid &= ATTR_SIZE;
                if (attr->ia_valid)
-                       resp->status = nfsd_setattr(rqstp, newfhp, &attrs, 0,
-                                                   (time64_t)0);
+                       resp->status = nfsd_setattr(rqstp, newfhp, &attrs,
+                                                   NULL);
        }
 
 out_unlock:

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 76e89329b9f11ddc09d46630986a122a090ed201..a3a4400e75be1046f901a08ac1ca4258e46e3f87 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -476,7 +476,6 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap)
  * @rqstp: controlling RPC transaction
  * @fhp: filehandle of target
  * @attr: attributes to set
- * @check_guard: set to 1 if guardtime is a valid timestamp
  * @guardtime: do not act if ctime.tv_sec does not match this timestamp
  *
  * This call may adjust the contents of @attr (in particular, this
@@ -488,8 +487,7 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap)
  */
 __be32
 nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
-            struct nfsd_attrs *attr,
-            int check_guard, time64_t guardtime)
+            struct nfsd_attrs *attr, const struct timespec64 *guardtime)
 {
        struct dentry   *dentry;
        struct inode    *inode;
@@ -538,9 +536,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
        nfsd_sanitize_attrs(inode, iap);
 
-       if (check_guard && guardtime != inode_get_ctime_sec(inode))
-               return nfserr_notsync;
-
        /*
         * The size case is special, it changes the file in addition to the
         * attributes, and file systems don't expect it to be mixed with
@@ -558,6 +553,16 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
        err = fh_fill_pre_attrs(fhp);
        if (err)
                goto out_unlock;
+
+       if (guardtime) {
+               struct timespec64 ctime = inode_get_ctime(inode);
+               if ((u32)guardtime->tv_sec != (u32)ctime.tv_sec ||
+                   guardtime->tv_nsec != ctime.tv_nsec) {
+                       err = nfserr_notsync;
+                       goto out_fill_attrs;
+               }
+       }
+
        for (retries = 1;;) {
                struct iattr attrs;
 
@@ -585,6 +590,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
                attr->na_aclerr = set_posix_acl(&nop_mnt_idmap,
                                                dentry, ACL_TYPE_DEFAULT,
                                                attr->na_dpacl);
+out_fill_attrs:
        fh_fill_post_attrs(fhp);
 out_unlock:
        inode_unlock(inode);
@@ -1411,7 +1417,7 @@ nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
         * if the attributes have not changed.
         */
        if (iap->ia_valid)
-               status = nfsd_setattr(rqstp, resfhp, attrs, 0, (time64_t)0);
+               status = nfsd_setattr(rqstp, resfhp, attrs, NULL);
        else
                status = nfserrno(commit_metadata(resfhp));
 

diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 1efa4e8dfb03495482caedb23ae69fedaea67398..c60fdb6200fded34c06fa46ce414991ac51a265e 100644 (file)
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -69,7 +69,7 @@ __be32                 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
                                const char *, unsigned int,
                                struct svc_export **, struct dentry **);
 __be32         nfsd_setattr(struct svc_rqst *, struct svc_fh *,
-                               struct nfsd_attrs *, int, time64_t);
+                            struct nfsd_attrs *, const struct timespec64 *);
 int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32         nfsd4_vfs_fallocate(struct svc_rqst *, struct svc_fh *,

diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 03fe4e21306cba0a95eb91adaefeb377e08bd977..522067b7fd755930a1c2e42b826d9132ac2993be 100644 (file)
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -14,7 +14,7 @@ struct nfsd3_sattrargs {
        struct svc_fh           fh;
        struct iattr            attrs;
        int                     check_guard;
-       time64_t                guardtime;
+       struct timespec64       guardtime;
 };
 
 struct nfsd3_diropargs {