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

On Fri, 2026-05-29 at 09:34 +0200, Cedric Blancher wrote:
> What about (finally) getting rid of the bubble sort?
> 
> Ced
> 

It's certainly not clear to me that it's necessary (is it?). I'm not
opposed to removing it, but I'd rather not make a behavioral change
like this in the context of this patchset.

Maybe you could propose a patch to remove it?


> On Fri, 29 May 2026 at 00:02, Jeff Layton <[email protected]> wrote:
> > 
> > From: Chris Mason <[email protected]>
> > 
> > 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.
> > 
> > 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 <[email protected]>
> > ---
> >  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
> > 
> > 
> 

-- 
Jeff Layton <[email protected]>