Re: [PATCH v4 04/30] fs: add new get acl method

[Miklos Szeredi <miklos@szeredi.hu>]

On Fri, 30 Sept 2022 at 11:09, Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Sep 30, 2022 at 10:53:05AM +0200, Miklos Szeredi wrote:
> > On Thu, 29 Sept 2022 at 17:31, Christian Brauner <brauner@kernel.org> wrote:
> >
> > > This adds a new ->get_acl() inode operations which takes a dentry
> > > argument which filesystems such as 9p, cifs, and overlayfs can implement
> > > to get posix acls.
> >
> > This is confusing.   For example overlayfs ends up with two functions
> > that are similar, but not quite the same:
> >
> >  ovl_get_acl -> ovl_get_acl_path -> vfs_get_acl -> __get_acl(mnt_userns, ...)
> >
> >  ovl_get_inode_acl -> get_inode_acl -> __get_acl(&init_user_ns, ...)
> >
> > So what's the difference and why do we need both?  If one can retrive
> > the acl without dentry, then why do we need the one with the dentry?
>
> The ->get_inode_acl() method is called during generic_permission() and
> inode_permission() both of which are called from various filesystems in
> their ->permission inode operations. There's no dentry available during
> the permission inode operation and there are filesystems like 9p and
> cifs that need a dentry.

This doesn't answer the question about why we need two for overlayfs
and what's the difference between them.

>
> > If a filesystem cannot implement a get_acl() without a dentry, then
> > what will happen to caller's that don't have a dentry?
>
> This happens today for cifs where posix acls can be created and read but
> they cannot be used for permission checking where no inode is available.
> New filesystems shouldn't have this issue.

That's weird, how does it make sense to set acl on a filesystem that
cannot use it for permission checking?   Maybe the permission checking
is done by the server?

Steve?

Thanks,
Miklos