NFSD: COMMIT operations must not return NFS?ERR_INVAL

Since, well, forever, the Linux NFS server's nfsd_commit() function
has returned nfserr_inval when the passed-in byte range arguments
were non-sensical.

However, according to RFC 1813 section 3.3.21, NFSv3 COMMIT requests
are permitted to return only the following non-zero status codes:

      NFS3ERR_IO
      NFS3ERR_STALE
      NFS3ERR_BADHANDLE
      NFS3ERR_SERVERFAULT

NFS3ERR_INVAL is not included in that list. Likewise, NFS4ERR_INVAL
is not listed in the COMMIT row of Table 6 in RFC 8881.

RFC 7530 does permit COMMIT to return NFS4ERR_INVAL, but does not
specify when it can or should be used.

Instead of dropping or failing a COMMIT request in a byte range that
is not supported, turn it into a valid request by treating one or
both arguments as zero. Offset zero means start-of-file, count zero
means until-end-of-file, so we only ever extend the commit range.
NFS servers are always allowed to commit more and sooner than
requested.

The range check is no longer bounded by NFS_OFFSET_MAX, but rather
by the value that is returned in the maxfilesize field of the NFSv3
FSINFO procedure or the NFSv4 maxfilesize file attribute.

Note that this change results in a new pynfs failure:

CMT4     st_commit.testCommitOverflow                             : RUNNING
CMT4     st_commit.testCommitOverflow                             : FAILURE
           COMMIT with offset + count overflow should return
           NFS4ERR_INVAL, instead got NFS4_OK

IMO the test is not correct as written: RFC 8881 does not allow the
COMMIT operation to return NFS4ERR_INVAL.

Reported-by: Dan Aloni <dan.aloni@vastdata.com>
Cc: stable@vger.kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Bruce Fields <bfields@fieldses.org>

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index aca38ed1526e238b7695d737c02c43078eda449f..52ad1972cc33cbfc3073bc5cd3e37cbeb7aea803 100644 (file)
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -663,15 +663,9 @@ nfsd3_proc_commit(struct svc_rqst *rqstp)
                                argp->count,
                                (unsigned long long) argp->offset);
 
-       if (argp->offset > NFS_OFFSET_MAX) {
-               resp->status = nfserr_inval;
-               goto out;
-       }
-
        fh_copy(&resp->fh, &argp->fh);
        resp->status = nfsd_commit(rqstp, &resp->fh, argp->offset,
                                   argp->count, resp->verf);
-out:
        return rpc_success;
 }
 

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 0cccceb105e73393d0448d7da58d5477a910e2d3..91600e71be19183f82158663540caa2b55d02db2 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1114,42 +1114,61 @@ out:
 }
 
 #ifdef CONFIG_NFSD_V3
-/*
- * Commit all pending writes to stable storage.
+/**
+ * nfsd_commit - Commit pending writes to stable storage
+ * @rqstp: RPC request being processed
+ * @fhp: NFS filehandle
+ * @offset: raw offset from beginning of file
+ * @count: raw count of bytes to sync
+ * @verf: filled in with the server's current write verifier
  *
- * Note: we only guarantee that data that lies within the range specified
- * by the 'offset' and 'count' parameters will be synced.
+ * Note: we guarantee that data that lies within the range specified
+ * by the 'offset' and 'count' parameters will be synced. The server
+ * is permitted to sync data that lies outside this range at the
+ * same time.
  *
  * Unfortunately we cannot lock the file to make sure we return full WCC
  * data to the client, as locking happens lower down in the filesystem.
+ *
+ * Return values:
+ *   An nfsstat value in network byte order.
  */
 __be32
-nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp,
-               loff_t offset, unsigned long count, __be32 *verf)
+nfsd_commit(struct svc_rqst *rqstp, struct svc_fh *fhp, u64 offset,
+           u32 count, __be32 *verf)
 {
+       u64                     maxbytes;
+       loff_t                  start, end;
        struct nfsd_net         *nn;
        struct nfsd_file        *nf;
-       loff_t                  end = LLONG_MAX;
-       __be32                  err = nfserr_inval;
-
-       if (offset < 0)
-               goto out;
-       if (count != 0) {
-               end = offset + (loff_t)count - 1;
-               if (end < offset)
-                       goto out;
-       }
+       __be32                  err;
 
        err = nfsd_file_acquire(rqstp, fhp,
                        NFSD_MAY_WRITE|NFSD_MAY_NOT_BREAK_LEASE, &nf);
        if (err)
                goto out;
+
+       /*
+        * Convert the client-provided (offset, count) range to a
+        * (start, end) range. If the client-provided range falls
+        * outside the maximum file size of the underlying FS,
+        * clamp the sync range appropriately.
+        */
+       start = 0;
+       end = LLONG_MAX;
+       maxbytes = (u64)fhp->fh_dentry->d_sb->s_maxbytes;
+       if (offset < maxbytes) {
+               start = offset;
+               if (count && (offset + count - 1 < maxbytes))
+                       end = offset + count - 1;
+       }
+
        nn = net_generic(nf->nf_net, nfsd_net_id);
        if (EX_ISSYNC(fhp->fh_export)) {
                errseq_t since = READ_ONCE(nf->nf_file->f_wb_err);
                int err2;
 
-               err2 = vfs_fsync_range(nf->nf_file, offset, end, 0);
+               err2 = vfs_fsync_range(nf->nf_file, start, end, 0);
                switch (err2) {
                case 0:
                        nfsd_copy_write_verifier(verf, nn);

diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 9f56dcb22ff72adeb34fa71fe842e2815086b7c1..2c43d10e3cab419642cd0b28de4c3e55ff1d7cc2 100644 (file)
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -74,8 +74,8 @@ __be32                do_nfsd_create(struct svc_rqst *, struct svc_fh *,
                                char *name, int len, struct iattr *attrs,
                                struct svc_fh *res, int createmode,
                                u32 *verifier, bool *truncp, bool *created);
-__be32         nfsd_commit(struct svc_rqst *, struct svc_fh *,
-                               loff_t, unsigned long, __be32 *verf);
+__be32         nfsd_commit(struct svc_rqst *rqst, struct svc_fh *fhp,
+                               u64 offset, u32 count, __be32 *verf);
 #endif /* CONFIG_NFSD_V3 */
 #ifdef CONFIG_NFSD_V4
 __be32         nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,