Re: [PATCH] NFSD: copy SETXATTR data from all XDR buffer segments

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-09-15 at 13:00 -0300, Aldo Ariel Panzardo wrote:
> nfsd4_vbuf_from_vector() manually copies the head followed by whole
> pages starting at offset zero. This assumes that all remaining data
> starts at the beginning of the page array and that no data is stored
> in the tail.
> 
> However, nfsd4_decode_setxattr() passes an xdr_buf returned by
> xdr_stream_subsegment(). Such a sub-buffer can have a nonzero
> page_base and can span its page and tail segments. Ignoring that
> layout can copy the wrong bytes into the value passed to
> vfs_setxattr().
> 
> Use read_bytes_from_xdr_buf(), which observes the complete xdr_buf
> layout.
> 
> Fixes: c1346a1216ab ("NFSD: Replace the internals of the READ_BUF() macro")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  fs/nfsd/nfs4xdr.c | 19 +++----------------
>  1 file changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index e17488a911..0f9f926cbd 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2288,10 +2288,8 @@ static __be32
>  nfsd4_vbuf_from_vector(struct nfsd4_compoundargs *argp, struct xdr_buf *xdr,
>  		       char **bufp, size_t buflen)
>  {
> -	struct page **pages = xdr->pages;
>  	struct kvec *head = xdr->head;
> -	char *tmp, *dp;
> -	u32 len;
> +	char *tmp;
>  
>  	if (buflen <= head->iov_len) {
>  		/*
> @@ -2306,19 +2304,8 @@ nfsd4_vbuf_from_vector(struct nfsd4_compoundargs *argp, struct xdr_buf *xdr,
>  	if (tmp == NULL)
>  		return nfserr_jukebox;
>  
> -	dp = tmp;
> -	memcpy(dp, head->iov_base, head->iov_len);
> -	buflen -= head->iov_len;
> -	dp += head->iov_len;
> -
> -	while (buflen > 0) {
> -		len = min_t(u32, buflen, PAGE_SIZE);
> -		memcpy(dp, page_address(*pages), len);
> -
> -		buflen -= len;
> -		dp += len;
> -		pages++;
> -	}
> +	if (read_bytes_from_xdr_buf(xdr, 0, tmp, buflen))
> +		return nfserr_bad_xdr;
>  
>  	*bufp = tmp;
>  	return 0;

Reviewed-by: Jeff Layton <jlayton@kernel.org>