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

[Chuck Lever]

On Thu, May 28, 2026, at 7:11 PM, Chuck Lever wrote:
> On Thu, May 28, 2026, at 6:11 PM, Rick Macklem wrote:
>> 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.
>
> The NFSACL protocol limits the number of ACEs in an ACL to NFS_ACL_MAX_ENTRIES
> (1024). It’s a limit that is defined in the protocol itself.
>
> The NFSv4 protocol sets no similar limit. In particular, NFS_ACL_MAX_ENTRIES
> is not a constant defined or used by the NFSv4.x family of protocols IIRC.
>
> Using NFS_ACL_MAX_ENTRIES to cap the number of ACEs in NFSv4 ACLs is a
> convenience, but it adds technical debt (slight though it may be).
>
> So… We can define an implementation limit for NFSv4 ACL support in NFSD.
> But it shouldn’t be called NFS_ACL_MAX_ENTRIES, IMHO. For clarity of
> documentation, pick a number (could be 1024) and state in a comment that
> it is an NFSD implementation constraint, not a protocol limit. Name the
> constant something like NFSD4_MAX_yada to make it clear it is an
> implementation limit, not a protocol limit.

A different take on this might be that we want to ensure that ACLs set
via the NFSv4 POSIX ACL extension can always be retrieved via NFSACL.
In that case, capping the ACE count at the same number makes sense.
As long as the reason for this is clearly mentioned.


-- 
Chuck Lever