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

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: [email protected]
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <[email protected]>
> ---
> 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
[email protected], trond.myklebust-F/[email protected]