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

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

[Aldo Ariel Panzardo]

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;
-- 
2.43.0

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

[Jeff Layton]

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>

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

[Chuck Lever]

On Tue, 15 Sep 2026 13:00:22 -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().
> 
> [...]

Applied to nfsd-testing, thanks!

[1/1] NFSD: copy SETXATTR data from all XDR buffer segments
      commit: e551e7dde7b82be0d3e6b848c9c5fef728d020dd

--
Chuck Lever