Linux NFS development
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: Trond Myklebust <trondmy@kernel.org>, Anna Schumaker <anna@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types
Date: Fri, 26 Jun 2026 07:18:52 -0400	[thread overview]
Message-ID: <20260626111853.801493-1-michael.bommarito@gmail.com> (raw)
In-Reply-To: <20260622124836.1696330-1-michael.bommarito@gmail.com>

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


      parent reply	other threads:[~2026-06-26 11:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-22 12:48 [PATCH] NFSv4.1/pnfs: bound layout-type count in decode_pnfs_layout_types Michael Bommarito
2026-06-22 14:20 ` Trond Myklebust
2026-06-26 11:18 ` Michael Bommarito [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260626111853.801493-1-michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=anna@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox