[PATCH v2 0/3] NFS: Modernize decoding uint32 arrays

[PATCH v2 0/3] NFS: Modernize decoding uint32 arrays

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

We've had several reports over the last few months people have tried to
add bounds checking to the various places that need to decode a uint32
array. None of them update the callsites to use
xdr_stream_decode_uint32_array(), which has all the proper bounds
checking already implemented. These patches make that change.

Changed in v2:
 - Properly handle -EMSGSIZE in decode_bitmap()

Thoughts?
Anna

Anna Schumaker (3):
  NFS: Have decode_pnfs_layout_types() use XDR's decode_uint32_array()
  NFS: Modernize the callback decode_bitmap() function
  NFS: Modernize decode_getdevinfo() bitmap decoding

 fs/nfs/callback_xdr.c | 26 +++++++-------------
 fs/nfs/nfs4xdr.c      | 55 +++++++++----------------------------------
 2 files changed, 20 insertions(+), 61 deletions(-)

-- 
2.56.0

[PATCH v2 1/3] NFS: Have decode_pnfs_layout_types() use XDR's decode_uint32_array()

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

Rather than hardcoding in an alternative that doesn't properly check for
32-bit overflow during decoding.

Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
---
 fs/nfs/nfs4xdr.c | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 740f3bc4a19d..4d12b97f423e 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4924,32 +4924,17 @@ 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;
+	ssize_t nlayouttypes;
 
-	p = xdr_inline_decode(xdr, 4);
-	if (unlikely(!p))
-		return -EIO;
-	fsinfo->nlayouttypes = be32_to_cpup(p);
+	nlayouttypes = xdr_stream_decode_uint32_array(xdr,
+						      fsinfo->layouttype,
+						      NFS_MAX_LAYOUT_TYPES);
+	if (nlayouttypes == -EMSGSIZE)
+		nlayouttypes = NFS_MAX_LAYOUT_TYPES;
+	if (nlayouttypes < 0)
+		return nlayouttypes;
 
-	/* 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;
-
-	/* 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);
-		fsinfo->nlayouttypes = NFS_MAX_LAYOUT_TYPES;
-	}
-
-	for(i = 0; i < fsinfo->nlayouttypes; ++i)
-		fsinfo->layouttype[i] = be32_to_cpup(p++);
+	fsinfo->nlayouttypes = nlayouttypes;
 	return 0;
 }
 
-- 
2.56.0

[PATCH v2 2/3] NFS: Modernize the callback decode_bitmap() function

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

The sunrpc function xdr_stream_decode_uint32_array() is already designed
to decode arrays of integers with all necessary bounds checking. Update
the decode_bitmap() function to call this and map the return value into
something the callback handler expects.

Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
---
 fs/nfs/callback_xdr.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 5a1f05469268..25c18e505c60 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -100,24 +100,16 @@ static __be32 decode_fh(struct xdr_stream *xdr, struct nfs_fh *fh)
 	return 0;
 }
 
-static __be32 decode_bitmap(struct xdr_stream *xdr, uint32_t *bitmap)
+static __be32 decode_bitmap(struct xdr_stream *xdr, __u32 *bitmap,
+			    size_t bitmap_size)
 {
-	__be32 *p;
-	unsigned int attrlen;
+	ssize_t status;
 
-	p = xdr_inline_decode(xdr, 4);
-	if (unlikely(p == NULL))
+	status = xdr_stream_decode_uint32_array(xdr, bitmap, bitmap_size);
+	if (unlikely(status == -EMSGSIZE))
+		return 0;
+	if (unlikely(status < 0))
 		return htonl(NFS4ERR_RESOURCE);
-	attrlen = ntohl(*p);
-	p = xdr_inline_decode(xdr, attrlen << 2);
-	if (unlikely(p == NULL))
-		return htonl(NFS4ERR_RESOURCE);
-	if (likely(attrlen > 0))
-		bitmap[0] = ntohl(*p++);
-	if (attrlen > 1)
-		bitmap[1] = ntohl(*p++);
-	if (attrlen > 2)
-		bitmap[2] = ntohl(*p);
 	return 0;
 }
 
@@ -182,7 +174,7 @@ static __be32 decode_getattr_args(struct svc_rqst *rqstp,
 	status = decode_fh(xdr, &args->fh);
 	if (unlikely(status != 0))
 		return status;
-	return decode_bitmap(xdr, args->bitmap);
+	return decode_bitmap(xdr, args->bitmap, ARRAY_SIZE(args->bitmap));
 }
 
 static __be32 decode_recall_args(struct svc_rqst *rqstp,
@@ -458,7 +450,7 @@ static __be32 decode_recallany_args(struct svc_rqst *rqstp,
 	if (unlikely(p == NULL))
 		return htonl(NFS4ERR_BADXDR);
 	args->craa_objs_to_keep = ntohl(*p++);
-	status = decode_bitmap(xdr, bitmap);
+	status = decode_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap));
 	if (unlikely(status))
 		return status;
 	args->craa_type_mask = bitmap[0];
-- 
2.56.0

[PATCH v2 3/3] NFS: Modernize decode_getdevinfo() bitmap decoding

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

Use the decode_bitmap4() function for decoding notification bitmaps.
This function calls xdr_stream_decode_uint32_array() to do proper bounds
and overflow checking.

Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
---
 fs/nfs/nfs4xdr.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 4d12b97f423e..c0971af20154 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -6064,7 +6064,7 @@ static int decode_getdeviceinfo(struct xdr_stream *xdr,
 {
 	struct pnfs_device *pdev = res->pdev;
 	__be32 *p;
-	uint32_t len, type;
+	uint32_t type;
 	int status;
 
 	status = decode_op_hdr(xdr, OP_GETDEVICEINFO);
@@ -6099,26 +6099,8 @@ static int decode_getdeviceinfo(struct xdr_stream *xdr,
 		return -EIO;
 
 	/* Parse notification bitmap, verifying that it is zero. */
-	p = xdr_inline_decode(xdr, 4);
-	if (unlikely(!p))
+	if (decode_bitmap4(xdr, &res->notification, 1) < 0)
 		return -EIO;
-	len = be32_to_cpup(p);
-	if (len) {
-		uint32_t i;
-
-		p = xdr_inline_decode(xdr, 4 * len);
-		if (unlikely(!p))
-			return -EIO;
-
-		res->notification = be32_to_cpup(p++);
-		for (i = 1; i < len; i++) {
-			if (be32_to_cpup(p++)) {
-				dprintk("%s: unsupported notification\n",
-					__func__);
-				return -EIO;
-			}
-		}
-	}
 	return 0;
 }
 
-- 
2.56.0

Re: [PATCH v2 0/3] NFS: Modernize decoding uint32 arrays

[Benjamin Coddington]

On 20 Oct 2026, at 12:37, Anna Schumaker wrote:

> From: Anna Schumaker <anna.schumaker@hammerspace.com>
>
> We've had several reports over the last few months people have tried to
> add bounds checking to the various places that need to decode a uint32
> array. None of them update the callsites to use
> xdr_stream_decode_uint32_array(), which has all the proper bounds
> checking already implemented. These patches make that change.
>
> Changed in v2:
>  - Properly handle -EMSGSIZE in decode_bitmap()
>
> Thoughts?

Looks good -

Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>

Ben