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

[PATCH 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.

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 | 24 ++++++-------------
 fs/nfs/nfs4xdr.c      | 55 +++++++++----------------------------------
 2 files changed, 18 insertions(+), 61 deletions(-)

-- 
2.56.0

[PATCH 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 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 | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 5a1f05469268..b6ecfb73d850 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -100,24 +100,14 @@ 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 < 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 +172,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 +448,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 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 2/3] NFS: Modernize the callback decode_bitmap() function

[Benjamin Coddington]

On 12 Oct 2026, at 8:33, Anna Schumaker wrote:

> 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 | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index 5a1f05469268..b6ecfb73d850 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -100,24 +100,14 @@ 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 < 0))
>  		return htonl(NFS4ERR_RESOURCE);

There's a slight behavior change here..

xdr_stream_decode_uint32_array() returns -EMSGSIZE when the wire count
is larger than bitmap_size. That makes a well-formed bitmap4 of four or
more words fail CB_GETATTR and CB_RECALL_ANY with NFS4ERR_RESOURCE. The
old code accepted such a bitmap and ignored the words past bitmap[2].
knfsd never sends more than three words, but bitmap4 is variable length
and trailing zero words are legal. The client side already treats
-EMSGSIZE as success in nfs4xdr.c's decode_bitmap4()..

Minor.. any concern about that?

Ben

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

[Benjamin Coddington]

On 12 Oct 2026, at 8:33, 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.
>
> Thoughts?

I like it - even if a little disappointed that you didn't state "malicious
server" anywhere.

Ben

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

[Anna Schumaker]

On Mon, Oct 12, 2026, at 9:13 AM, Benjamin Coddington wrote:
> On 12 Oct 2026, at 8:33, Anna Schumaker wrote:
>
>> 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 | 24 +++++++-----------------
>>  1 file changed, 7 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
>> index 5a1f05469268..b6ecfb73d850 100644
>> --- a/fs/nfs/callback_xdr.c
>> +++ b/fs/nfs/callback_xdr.c
>> @@ -100,24 +100,14 @@ 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 < 0))
>>  		return htonl(NFS4ERR_RESOURCE);
>
> There's a slight behavior change here..
>
> xdr_stream_decode_uint32_array() returns -EMSGSIZE when the wire count
> is larger than bitmap_size. That makes a well-formed bitmap4 of four or
> more words fail CB_GETATTR and CB_RECALL_ANY with NFS4ERR_RESOURCE. The
> old code accepted such a bitmap and ignored the words past bitmap[2].
> knfsd never sends more than three words, but bitmap4 is variable length
> and trailing zero words are legal. The client side already treats
> -EMSGSIZE as success in nfs4xdr.c's decode_bitmap4()..
>
> Minor.. any concern about that?

I should probably try to preserve the old behavior as much as I can. I'll
fix that in a v2.

Thanks for the review!
Anna

>
> Ben