Re: [PATCH RFC] nfs4: add a get_acl stub handler

[Christian Brauner]

On Thu, Sep 07, 2023 at 01:32:36PM -0400, Jeff Layton wrote:
> In older kernels, attempting to fetch that system.posix_acl_access on
> NFSv4 would return -EOPNOTSUPP, but in more recent kernels that returns
> -ENODATA.
> 
> Most filesystems that don't support POSIX ACLs leave the SB_POSIXACL
> flag clear, which cues the VFS to return -EOPNOTSUPP in this situation.
> We can't do that with NFSv4 since that flag also cues the VFS to avoid
> applying the umask early.
> 
> Fix this by adding a stub get_acl handler for NFSv4 that always returns
> -EOPNOTSUPP.
> 
> Reported-by: Ondrej Valousek <ondrej.valousek.xm@xxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> I suspect that this problem popped in due to some VFS layer changes. I
> haven't identified the patch that broke it, but I think this is probably
> the least invasive way to fix it.
> 
> Another alternative would be to return -EOPNOTSUPP on filesystems that
> set SB_POSIXACL but that don't set get_acl or get_inode_acl.
> 
> Thoughts?

Yes: I hate POSIX ACLs. ;)

Before the VFS rework to only rely on i_op->*acl* methods POSIX ACLs
were set using sb->s_xattr handlers. So when a filesystem raised
SB_POSIXACL but didn't set sb->s_xattr handlers for POSIX ACLs we would:

__vfs_getxattr()
-> xattr_resolve_name()
  // no match so return EOPNOTSUPP

No we have

vfs_get_acl()
-> __get_acl()
   -> i_op->get_acl
   // no get_acl inode method return ENODATA

So as a bugfix to backport I think you should do exactly what you do
here because I'm not sure if some fs relies on ENODATA to be returned if
no get_acl inode method is set. There's a lot of quirkiness everywhere.
But we should look through all callers and if nothing relies on EINVAL
just start returning EOPNOTSUPP if no get_acl i_op is set.

Looks good to me,
Acked-by: Christian Brauner <brauner@xxxxxxxxxx>