Due to commit 8cfb9015280d ("NFS: Always provide aligned buffers to the RPC read layers") on the client, a read of 0xfff is aligned up to server rsize of 0x1000.
As a result, in a test where the server has a file of size 0x7fffffffffffffff, and the client tries to read from the offset 0x7ffffffffffff000, the read causes loff_t overflow in the server and it returns an NFS code of EINVAL to the client. The client as a result indefinitely retries the request.
This fixes the issue at server side by trimming reads past NFS_OFFSET_MAX. It also adds a missing check for out of bound offset in NFSv3, copying a similar check from NFSv4.x.
Cc: stable@vger.kernel.org Signed-off-by: Dan Aloni dan.aloni@vastdata.com --- fs/nfsd/nfs4proc.c | 3 +++ fs/nfsd/vfs.c | 6 ++++++ 2 files changed, 9 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 486c5dba4b65..816bdf212559 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -785,6 +785,9 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (read->rd_offset >= OFFSET_MAX) return nfserr_inval;
+ if (unlikely(read->rd_offset + read->rd_length > OFFSET_MAX)) + read->rd_length = OFFSET_MAX - read->rd_offset; + trace_nfsd_read_start(rqstp, &cstate->current_fh, read->rd_offset, read->rd_length);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 738d564ca4ce..ad4df374433e 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1045,6 +1045,12 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file; __be32 err;
+ if (unlikely(offset >= NFS_OFFSET_MAX)) + return nfserr_inval; + + if (unlikely(offset + *count > NFS_OFFSET_MAX)) + *count = NFS_OFFSET_MAX - offset; + trace_nfsd_read_start(rqstp, fhp, offset, *count); err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf); if (err)
On Sun, 2022-01-23 at 11:50 +0200, Dan Aloni wrote:
Due to commit 8cfb9015280d ("NFS: Always provide aligned buffers to the RPC read layers") on the client, a read of 0xfff is aligned up to server rsize of 0x1000.
As a result, in a test where the server has a file of size 0x7fffffffffffffff, and the client tries to read from the offset 0x7ffffffffffff000, the read causes loff_t overflow in the server and it returns an NFS code of EINVAL to the client. The client as a result indefinitely retries the request.
This fixes the issue at server side by trimming reads past NFS_OFFSET_MAX. It also adds a missing check for out of bound offset in NFSv3, copying a similar check from NFSv4.x.
Cc: stable@vger.kernel.org Signed-off-by: Dan Aloni dan.aloni@vastdata.com
fs/nfsd/nfs4proc.c | 3 +++ fs/nfsd/vfs.c | 6 ++++++ 2 files changed, 9 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 486c5dba4b65..816bdf212559 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -785,6 +785,9 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (read->rd_offset >= OFFSET_MAX) return nfserr_inval; + if (unlikely(read->rd_offset + read->rd_length > OFFSET_MAX)) + read->rd_length = OFFSET_MAX - read->rd_offset;
trace_nfsd_read_start(rqstp, &cstate->current_fh, read->rd_offset, read->rd_length); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 738d564ca4ce..ad4df374433e 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1045,6 +1045,12 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file; __be32 err; + if (unlikely(offset >= NFS_OFFSET_MAX)) + return nfserr_inval;
POSIX does allow you to lseek to the maximum filesize offset (sb-
s_maxbytes), however any subsequent read will return 0 bytes (i.e.
eof), whereas a subsequent write will return EFBIG.
+ if (unlikely(offset + *count > NFS_OFFSET_MAX)) + *count = NFS_OFFSET_MAX - offset;
Can we please fix this to use the actual per-filesystem file size limit, which is set as sb->s_maxbytes, instead of using NFS_OFFSET_MAX?
Ditto for 'read' above.
trace_nfsd_read_start(rqstp, fhp, offset, *count); err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf); if (err)
On Jan 23, 2022, at 10:29 AM, Trond Myklebust trondmy@hammerspace.com wrote:
On Sun, 2022-01-23 at 11:50 +0200, Dan Aloni wrote:
Due to commit 8cfb9015280d ("NFS: Always provide aligned buffers to the RPC read layers") on the client, a read of 0xfff is aligned up to server rsize of 0x1000.
As a result, in a test where the server has a file of size 0x7fffffffffffffff, and the client tries to read from the offset 0x7ffffffffffff000, the read causes loff_t overflow in the server and it returns an NFS code of EINVAL to the client. The client as a result indefinitely retries the request.
This fixes the issue at server side by trimming reads past NFS_OFFSET_MAX. It also adds a missing check for out of bound offset in NFSv3, copying a similar check from NFSv4.x.
Cc: stable@vger.kernel.org Signed-off-by: Dan Aloni dan.aloni@vastdata.com
fs/nfsd/nfs4proc.c | 3 +++ fs/nfsd/vfs.c | 6 ++++++ 2 files changed, 9 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 486c5dba4b65..816bdf212559 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -785,6 +785,9 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (read->rd_offset >= OFFSET_MAX) return nfserr_inval;
if (unlikely(read->rd_offset + read->rd_length > OFFSET_MAX))read->rd_length = OFFSET_MAX - read->rd_offset;trace_nfsd_read_start(rqstp, &cstate->current_fh, read->rd_offset, read->rd_length);diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 738d564ca4ce..ad4df374433e 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1045,6 +1045,12 @@ __be32 nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file; __be32 err;
if (unlikely(offset >= NFS_OFFSET_MAX))return nfserr_inval;POSIX does allow you to lseek to the maximum filesize offset (sb-
s_maxbytes), however any subsequent read will return 0 bytes (i.e.
eof), whereas a subsequent write will return EFBIG.
I'm simply trying to clarify your request.
You've stated that the Linux NFS client does not handle INVAL responses, even though both RFC 1813 and 8881 permit it. So are you suggesting (here) that the Linux NFS server should not return INVAL on READs beyond the filesystem's supported maximum file size but instead return a successful 0-byte result with EOF set?
if (unlikely(offset + *count > NFS_OFFSET_MAX))*count = NFS_OFFSET_MAX - offset;Can we please fix this to use the actual per-filesystem file size limit, which is set as sb->s_maxbytes, instead of using NFS_OFFSET_MAX?
Ditto for 'read' above.
That sounds reasonable. But I wonder if there are some other places that need the same treatment.
trace_nfsd_read_start(rqstp, fhp, offset, *count); err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf); if (err)
-- Chuck Lever
linux-stable-mirror@lists.linaro.org
-
Chuck Lever III -
Dan Aloni -
Trond Myklebust