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

[Trond Myklebust <trondmy@hammerspace.com>]

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@redhat.com>
> 
> 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@redhat.com>
> ---
>  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.

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