Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <8470f03b-0abd-497c-a4e4-3ea6ea042da4@gmail.com>
Date:   Thu, 28 Sep 2023 09:47:28 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1] NFSD: Fix zero NFSv4 READ results when RQ_SPLICE_OK is
 not set
Content-Language: en-US, lt-LT
To:     Chuck Lever <cel@kernel.org>, linux-nfs@vger.kernel.org
Cc:     Chuck Lever <chuck.lever@oracle.com>
References: <169583500802.5201.6400721981172612933.stgit@bazille.1015granger.net>
From:   =?UTF-8?Q?Mantas_Mikul=C4=97nas?= <grawity@gmail.com>
In-Reply-To: <169583500802.5201.6400721981172612933.stgit@bazille.1015granger.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 27/09/2023 20.16, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> nfsd4_encode_readv() uses xdr->buf->page_len as a starting point for
> the nfsd_iter_read() sink buffer -- page_len is going to be offset
> by the parts of the COMPOUND that have already been encoded into
> xdr->buf->pages.
>
> However, that value must be captured /before/
> xdr_reserve_space_vec() advances page_len by the expected size of
> the read payload. Otherwise, the whole front part of the first
> page of the payload in the reply will be uninitialized.
>
> Mantas hit this because sec=krb5i forces RQ_SPLICE_OK off, which
> invokes the readv part of the nfsd4_encode_read() path. Also,
> older Linux NFS clients appear to send shorter READ requests
> for files smaller than a page, whereas newer clients just send
> page-sized requests and let the server send as many bytes as
> are in the file.
>
> Reported-by: Mantas Mikulėnas <grawity@gmail.com>
> Closes: https://lore.kernel.org/linux-nfs/f1d0b234-e650-0f6e-0f5d-126b3d51d1eb@gmail.com/
> Fixes: 703d75215555 ("NFSD: Hoist rq_vec preparation into nfsd_read() [step two]")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>   fs/nfsd/nfs4xdr.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 2e40c74d2f72..92c7dde148a4 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -4113,6 +4113,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
>   				 struct file *file, unsigned long maxcount)
>   {
>   	struct xdr_stream *xdr = resp->xdr;
> +	unsigned int base = xdr->buf->page_len & ~PAGE_MASK;
>   	unsigned int starting_len = xdr->buf->len;
>   	__be32 zero = xdr_zero;
>   	__be32 nfserr;
> @@ -4121,8 +4122,7 @@ static __be32 nfsd4_encode_readv(struct nfsd4_compoundres *resp,
>   		return nfserr_resource;
>   
>   	nfserr = nfsd_iter_read(resp->rqstp, read->rd_fhp, file,
> -				read->rd_offset, &maxcount,
> -				xdr->buf->page_len & ~PAGE_MASK,
> +				read->rd_offset, &maxcount, base,
>   				&read->rd_eof);
>   	read->rd_length = maxcount;
>   	if (nfserr)
>
Thanks, this seems to work for me.