Re: [PATCH] nfs: we don't support removing system.nfs4_acl

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2021-01-28 at 21:50 -0500, bfields@fieldses.org wrote:
> On Thu, Jan 28, 2021 at 09:35:27PM -0500, bfields@fieldses.org wrote:
> > Note that this patch doesn't prevent an application from setting a
> > zero-length ACL.  The xattr format is XDR with the first four bytes
> > representing the number of ACEs, so you'd set a zero-length ACL by
> > passing down a 4-byte all-zero buffer as the new value of the
> > system.nfs4_acl xattr.
> > 
> > A zero-length NULL buffer is what's used to implement removexattr:
> > 
> > int
> > __vfs_removexattr(struct dentry *dentry, const char *name)
> > {
> >         ...
> >         return handler->set(handler, dentry, inode, name, NULL, 0,
> > XATTR_REPLACE);
> > }
> > 
> > That's the case this patch covers.
> 
> So, I should have said in the changelog, apologies--the behavior
> without
> this patch is that when it gets a removexattr, the client sends a
> SETATTR with a bitmap claiming there's an ACL attribute, but a
> zero-length attribute value list, and the server (correctly) returns
> BADXDR.
> 

I don't see anything in the spec that prohibits a zero length array
size for nfs41_aces<> or states that should return NFS4ERR_BADXDR. Why
shouldn't we allow that?

Windows, for instance has explicit support for such an ACL:
https://docs.microsoft.com/en-us/windows/win32/secauthz/null-dacls-and-empty-dacls



-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com