> On Dec 17, 2021, at 5:23 PM, Bruce Fields <bfields@xxxxxxxxxx> wrote:
>
> On Fri, Dec 17, 2021 at 5:07 PM <trondmy@xxxxxxxxxx> wrote:
>>
>> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>>
>> According to RFC1813: "If count is 0, the WRITE will succeed
>> and return a count of 0, barring errors due to permissions checking."
>
> Yes, I'm surprised we're not already doing this right.
>
> I wonder how far back this bug goes.
>
> The svc.c code is from 8154ef2776aa "NFSD: Clean up legacy NFS WRITE
> argument XDR decoders", but the behavior might predate that code. The
> nfsd_vfs_write() logic, I'm not sure I understand.
The new check in nfsd_vfs_write() might circumvent the VFS
layer's security checks, so I agree, that is a little suspect.
> We have a pynfs test for the v4 case (WRT4), but I guess we must have
> nothing testing the v3 case.
I guess cthon04 doesn't check this case.
But it seems to me WRT4 should already tickle any problems
with nfsd_vfs_write(), shouldn't it?
> --b.
>
>>
>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>> ---
>> fs/nfsd/vfs.c | 3 +++
>> net/sunrpc/svc.c | 2 +-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>> index 4d57befdac6e..38fdfcbb079e 100644
>> --- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -969,6 +969,9 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
>>
>> trace_nfsd_write_opened(rqstp, fhp, offset, *cnt);
>>
>> + if (!*cnt)
>> + return nfs_ok;
>> +
>> if (sb->s_export_op)
>> exp_op_flags = sb->s_export_op->flags;
>>
>> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
>> index a3bbe5ce4570..d1ccf37a4588 100644
>> --- a/net/sunrpc/svc.c
>> +++ b/net/sunrpc/svc.c
>> @@ -1692,7 +1692,7 @@ unsigned int svc_fill_write_vector(struct svc_rqst *rqstp, struct page **pages,
>> * entirely in rq_arg.pages. In this case, @first is empty.
>> */
>> i = 0;
>> - if (first->iov_len) {
>> + if (first->iov_len || !total) {
>> vec[i].iov_base = first->iov_base;
>> vec[i].iov_len = min_t(size_t, total, first->iov_len);
>> total -= vec[i].iov_len;
>> --
>> 2.33.1
>>
>
--
Chuck Lever
