Commit 3f965021 authored by Chuck Lever's avatar Chuck Lever
Browse files

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: default avatarDan Aloni <dan.aloni@vastdata.com>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
Reviewed-by: default avatarBruce Fields <bfields@fieldses.org>
parent 6260d9a5

fs/nfsd/nfs3proc.c

+0 −6
Original line number Diff line number Diff line
@@ -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;

}

fs/nfsd/vfs.c

+36 −17
Original line number Diff line number Diff line
@@ -1114,42 +1114,61 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, 
}



#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);

fs/nfsd/vfs.h

+2 −2
Original line number Diff line number Diff line
@@ -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,