[PATCH] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types

[PATCH] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types

[Michael Bommarito]

decode_pnfs_layout_types() reads a server-controlled u32 count and then
reserves 4 * count bytes:

	fsinfo->nlayouttypes = be32_to_cpup(p);
	...
	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);

The multiplication is u32, so any count >= 0x40000000 wraps to a small
value and xdr_inline_decode() reserves too few bytes. The NFS_MAX_LAYOUT
cap is applied only afterwards, so the subsequent reads run past the
short reservation. array_size() is not a safe guard here either:
xdr_inline_decode() runs its nbytes argument through
XDR_QUADLEN(((l) + 3) >> 2), which wraps SIZE_MAX to a zero-word
reservation instead of failing.

Reject counts that cannot fit in the u32 multiplication before the
reservation, and use sizeof(__be32) so the size arithmetic is explicit.

A malicious NFSv4.1+ server returning a crafted FATTR4_FS_LAYOUT_TYPES
attribute triggers this on the client during FSINFO decode.

This is the decode_pnfs_layout_types companion to the same XDR-wrap class
fixed in decode_getdeviceinfo (NFSv4.1/pnfs: bound notification bitmap
length in decode_getdeviceinfo).

Fixes: ca440c383a58 ("pnfs: add a new mechanism to select a layout driver according to an ordered list")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Reproduced on a UML KASAN build with a KUnit case that feeds
decode_pnfs_layout_types() an xdr buffer whose nlayouttypes is 0x40000001:
nlayouttypes * 4 wraps, xdr_inline_decode() reserves a short buffer, and
the decode reads past xdr->end (KASAN slab-out-of-bounds read). With this
patch the count is rejected (-EIO) before the reservation. Benign control:
a small nlayouttypes decodes correctly on both stock and patched. The
KUnit test can ride as a separate patch (matching the decode_getdeviceinfo
series). Before/after logs available on request.

 fs/nfs/nfs4xdr.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c23c2eee1b5c4..8b7994f61d303 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4903,8 +4903,16 @@ static int decode_pnfs_layout_types(struct xdr_stream *xdr,
 	if (fsinfo->nlayouttypes == 0)
 		return 0;
 
+	/* Reject counts that would overflow the u32 multiplication below.
+	 * array_size() is not sufficient here: xdr_inline_decode() passes
+	 * nbytes through XDR_QUADLEN(((l)+3)>>2), which wraps SIZE_MAX to
+	 * a zero-word reservation rather than failing.
+	 */
+	if (fsinfo->nlayouttypes > U32_MAX / sizeof(__be32))
+		return -EIO;
+
 	/* Decode and set first layout type, move xdr->p past unused types */
-	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);
+	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * sizeof(__be32));
 	if (unlikely(!p))
 		return -EIO;
 

base-commit: ef0c9f75a19532d7675384708fc8621e10850104
-- 
2.53.0

Re: [PATCH] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types

[Trond Myklebust]

On Mon, 2026-06-22 at 08:48 -0400, Michael Bommarito wrote:
> decode_pnfs_layout_types() reads a server-controlled u32 count and
> then
> reserves 4 * count bytes:
> 
> 	fsinfo->nlayouttypes = be32_to_cpup(p);
> 	...
> 	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);
> 
> The multiplication is u32, so any count >= 0x40000000 wraps to a
> small
> value and xdr_inline_decode() reserves too few bytes. The
> NFS_MAX_LAYOUT
> cap is applied only afterwards, so the subsequent reads run past the
> short reservation. array_size() is not a safe guard here either:
> xdr_inline_decode() runs its nbytes argument through
> XDR_QUADLEN(((l) + 3) >> 2), which wraps SIZE_MAX to a zero-word
> reservation instead of failing.
> 
> Reject counts that cannot fit in the u32 multiplication before the
> reservation, and use sizeof(__be32) so the size arithmetic is
> explicit.
> 
> A malicious NFSv4.1+ server returning a crafted
> FATTR4_FS_LAYOUT_TYPES
> attribute triggers this on the client during FSINFO decode.
> 
> This is the decode_pnfs_layout_types companion to the same XDR-wrap
> class
> fixed in decode_getdeviceinfo (NFSv4.1/pnfs: bound notification
> bitmap
> length in decode_getdeviceinfo).
> 
> Fixes: ca440c383a58 ("pnfs: add a new mechanism to select a layout
> driver according to an ordered list")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> Reproduced on a UML KASAN build with a KUnit case that feeds
> decode_pnfs_layout_types() an xdr buffer whose nlayouttypes is
> 0x40000001:
> nlayouttypes * 4 wraps, xdr_inline_decode() reserves a short buffer,
> and
> the decode reads past xdr->end (KASAN slab-out-of-bounds read). With
> this
> patch the count is rejected (-EIO) before the reservation. Benign
> control:
> a small nlayouttypes decodes correctly on both stock and patched. The
> KUnit test can ride as a separate patch (matching the
> decode_getdeviceinfo
> series). Before/after logs available on request.
> 
>  fs/nfs/nfs4xdr.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index c23c2eee1b5c4..8b7994f61d303 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4903,8 +4903,16 @@ static int decode_pnfs_layout_types(struct
> xdr_stream *xdr,
>  	if (fsinfo->nlayouttypes == 0)
>  		return 0;
>  
> +	/* Reject counts that would overflow the u32 multiplication
> below.
> +	 * array_size() is not sufficient here: xdr_inline_decode()
> passes
> +	 * nbytes through XDR_QUADLEN(((l)+3)>>2), which wraps
> SIZE_MAX to
> +	 * a zero-word reservation rather than failing.
> +	 */
> +	if (fsinfo->nlayouttypes > U32_MAX / sizeof(__be32))
> +		return -EIO;
> +
>  	/* Decode and set first layout type, move xdr->p past unused
> types */
> -	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);
> +	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes *
> sizeof(__be32));
>  	if (unlikely(!p))
>  		return -EIO;
>  
> 
> base-commit: ef0c9f75a19532d7675384708fc8621e10850104

This is just open coding xdr_stream_decode_uint32_array(), and doing so
incorrectly.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

[PATCH v2] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types

[Michael Bommarito]

decode_pnfs_layout_types() reads a server-controlled u32 count and then
hand-decodes that many layout types, reserving 4 * count bytes:

	fsinfo->nlayouttypes = be32_to_cpup(p);
	...
	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);

The multiplication is u32, so a count >= 0x40000000 wraps and
xdr_inline_decode() reserves too few bytes; the NFS_MAX_LAYOUT_TYPES cap
is applied only afterwards, so the subsequent reads run past the short
reservation.

This open-codes xdr_stream_decode_uint32_array(), which reads the count,
rejects a length that would overflow the size calculation, decodes the
array, zero-fills the remainder, and bounds the result against the
destination size. Use it. A server advertising more than
NFS_MAX_LAYOUT_TYPES stays non-fatal as before by treating -EMSGSIZE as
a cap.

A malicious NFSv4.1+ server returning a crafted FATTR4_FS_LAYOUT_TYPES
attribute triggers the original overflow on the client during FSINFO
decode.

Fixes: ca440c383a58 ("pnfs: add a new mechanism to select a layout driver according to an ordered list")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: use xdr_stream_decode_uint32_array() instead of an open-coded count
    read and manual bound, per Trond Myklebust's review of v1
    (https://lore.kernel.org/all/20260622124836.1696330-1-michael.bommarito@gmail.com/).
    v1 added a hand-rolled U32_MAX guard; the helper already does the
    overflow-safe count read, array decode, and destination bound.

Reproduced on a UML KASAN build: a crafted FATTR4_FS_LAYOUT_TYPES with
nlayouttypes=0x40000001 made the v1/original u32 multiply wrap and the
decode read past xdr->end (KASAN slab-out-of-bounds read). With this
patch xdr_stream_decode_uint32_array() rejects the length (-EBADMSG ->
-EIO) before any over-read; a valid small count still decodes, and a
count > NFS_MAX_LAYOUT_TYPES is capped as before. Before/after logs
available on request.

 fs/nfs/nfs4xdr.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c23c2eee1b5c4..4c1beef30522a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4891,32 +4891,20 @@ static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr,
 static int decode_pnfs_layout_types(struct xdr_stream *xdr,
 				    struct nfs_fsinfo *fsinfo)
 {
-	__be32 *p;
-	uint32_t i;
-
-	p = xdr_inline_decode(xdr, 4);
-	if (unlikely(!p))
-		return -EIO;
-	fsinfo->nlayouttypes = be32_to_cpup(p);
-
-	/* pNFS is not supported by the underlying file system */
-	if (fsinfo->nlayouttypes == 0)
-		return 0;
-
-	/* Decode and set first layout type, move xdr->p past unused types */
-	p = xdr_inline_decode(xdr, fsinfo->nlayouttypes * 4);
-	if (unlikely(!p))
-		return -EIO;
+	ssize_t ret;
 
-	/* If we get too many, then just cap it at the max */
-	if (fsinfo->nlayouttypes > NFS_MAX_LAYOUT_TYPES) {
-		printk(KERN_INFO "NFS: %s: Warning: Too many (%u) pNFS layout types\n",
-			__func__, fsinfo->nlayouttypes);
+	ret = xdr_stream_decode_uint32_array(xdr, fsinfo->layouttype,
+					     NFS_MAX_LAYOUT_TYPES);
+	if (ret == -EMSGSIZE) {
+		/* Server listed more types than we support; keep the first
+		 * NFS_MAX_LAYOUT_TYPES, as before.
+		 */
 		fsinfo->nlayouttypes = NFS_MAX_LAYOUT_TYPES;
+		return 0;
 	}
-
-	for(i = 0; i < fsinfo->nlayouttypes; ++i)
-		fsinfo->layouttype[i] = be32_to_cpup(p++);
+	if (ret < 0)
+		return -EIO;
+	fsinfo->nlayouttypes = ret;
 	return 0;
 }
 

base-commit: ef0c9f75a19532d7675384708fc8621e10850104
-- 
2.53.0