Re: [PATCH 09/10] nfsd: cap decoded POSIX ACL count to bound sort cost

[Rick Macklem]

On Thu, May 28, 2026 at 2:56 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If you are unsure, forward the message to ITHelp@xxxxxxxxxxx for review.
>
>
> From: Chris Mason <clm@xxxxxxxx>
>
> nfsd4_decode_posixacl() reads a u32 entry count off the wire and passes
> it straight to posix_acl_alloc() and sort_pacl_range(). The latter is
> an O(n^2) bubble sort, so a client-chosen count drives unbounded CPU in
> the server's compound processing path.
>
>     nfsd4_decode_posixacl()
>       xdr_stream_decode_u32(&count)       /* uncapped u32 */
>       posix_acl_alloc(count, GFP_KERNEL)
>       sort_pacl_range(*acl, 0, count - 1) /* O(n^2) bubble sort */
>
> The encoder side in the same file already rejects ACLs whose a_count
> exceeds NFS_ACL_MAX_ENTRIES, but the decoder introduced in commit
> 5fc51dfc2eb1 ("NFSD: Add support for XDR decoding POSIX draft ACLs")
> omitted the symmetric check.
My recollection is that Chuck didn't like this limit. He argued that it was
specific to the NFSv3 ACL protocol and that the limit on the size of a NFSv4
RPC message was sufficient.  I, personally, think that 1024 is a reasonable
limit for # of ACEs, but Chuck can jump in here if he doesn't agree.
(Note that, if user/groups are configured as "#s in the string", a lot of ACEs
fits in a 4Mbyte RPC message and the server file system will also set a limit
although, as you note, that happens after the sort.)

The good news w.r.t. the sort is that they are normally already sorted,
so the bubble sort is normally just a single pass, I think?

rick

>
> Fix by rejecting a wire count greater than NFS_ACL_MAX_ENTRIES with
> nfserr_resource, before any allocation, so the sort is bounded by
> NFS_ACL_MAX_ENTRIES^2 comparisons.
>
> Fixes: 5fc51dfc2eb1 ("NFSD: Add support for XDR decoding POSIX draft ACLs")
> Assisted-by: kres:claude-opus-4-7
> Signed-off-by: Chris Mason <clm@xxxxxxxx>
> ---
>  fs/nfsd/nfs4xdr.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index c6c50c376b23..5469c6c207ba 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -448,6 +448,8 @@ nfsd4_decode_posixacl(struct nfsd4_compoundargs *argp, struct posix_acl **acl)
>
>         if (xdr_stream_decode_u32(argp->xdr, &count) < 0)
>                 return nfserr_bad_xdr;
> +       if (count > NFS_ACL_MAX_ENTRIES)
> +               return nfserr_resource;
>
>         *acl = posix_acl_alloc(count, GFP_KERNEL);
>         if (*acl == NULL)
>
> --
> 2.54.0
>