On Fri, Jan 29, 2021 at 01:37:10AM +0000, Trond Myklebust wrote:
> On Fri, 2021-01-29 at 01:34 +0200, guy keren wrote:
> > On 1/29/21 12:36 AM, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
> >
> > The NFSv4 protocol doesn't have any notion of reomoving an attribute,
> > so
> > removexattr(path,"system.nfs4_acl") doesn't make sense.
> >
> > There's no documented return value. Arguably it could be EOPNOTSUPP
> > but
> > I'm a little worried an application might take that to mean that we
> > don't support ACLs or xattrs. How about EINVAL?
> >
> > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> > ---
> > fs/nfs/nfs4proc.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 2f4679a62712..d50dea5f5723 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5895,6 +5895,9 @@ static int __nfs4_proc_set_acl(struct inode
> > *inode, const void *buf, size_t bufl
> > unsigned int npages = DIV_ROUND_UP(buflen, PAGE_SIZE);
> > int ret, i;
> >
> > + /* You can't remove system.nfs4_acl: */
> > + if (buflen == 0)
> > + return -EINVAL;
> > if (!nfs4_server_supports_acls(server))
> > return -EOPNOTSUPP;
> > if (npages > ARRAY_SIZE(pages))
> >
> > question: what happens if someone is attempting to create an empty
> > ACL on a file? as far as i know, this is legal.
> > won't you arrive into this position with a buflen of 0? it should be
> > similar to 'chmod 0 <file>'.
> >
>
> Agreed. If the server doesn't support removing the ACL then it should
> be up to it to enforce that condition. I see nothing in the NFS
> protocol that says it is up to the NFS client to act as the enforcer
> here.
Agreed.
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.
--b.
