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

Michael Bommarito <[email protected]>
Newsgroups gmane.linux.nfs,gmane.linux.kernel
Message-ID <[email protected]>
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: [email protected]
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <[email protected]>
---
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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org/).
    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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.