| Message ID | 20220929153041.500115-5-brauner@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | acl: add vfs posix acl api | expand |
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? (BTW in both cases the mnt_userns for the underlying fs is available and used to translate the acl.) If a filesystem cannot implement a get_acl() without a dentry, then what will happen to caller's that don't have a dentry? Thanks, Miklos
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. > 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.
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
On Fri, Sep 30, 2022 at 11:43:07AM +0200, Miklos Szeredi wrote: > 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. Oh sorry, I misunderstood your questions then. The reason why I didn't consolidate them was simply the different in permission checking. So currently in current mainline overlayfs does acl = get_acl() in it's get acl method and does vfs_getxattr() in ovl_posix_acl_xattr_get(). The difference is that vfs_getxattr() goes through regular lsm hooks checking whereas get_acl() does not. So I thought that using get_acl() was done to not call lsm hooks in there. If that's not the case then I can consolidate both into one implementation.
On Fri, 30 Sept 2022 at 12:06, Christian Brauner <brauner@kernel.org> wrote: > > On Fri, Sep 30, 2022 at 11:43:07AM +0200, Miklos Szeredi wrote: > > 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. > > Oh sorry, I misunderstood your questions then. The reason why I didn't > consolidate them was simply the different in permission checking. > So currently in current mainline overlayfs does acl = get_acl() in it's > get acl method and does vfs_getxattr() in ovl_posix_acl_xattr_get(). > > The difference is that vfs_getxattr() goes through regular lsm hooks > checking whereas get_acl() does not. So I thought that using get_acl() > was done to not call lsm hooks in there. If that's not the case then I > can consolidate both into one implementation. So there are two paths to getting an acl: 1) permission checking and 2) retrieving the value via getxattr(2). This is a similar situation as reading a symlink vs. following it. When following a symlink overlayfs always reads the link on the underlying fs just as if it was a readlink(2) call, calling security_inode_readlink() instead of security_inode_follow_link(). This is logical: we are reading the link from the underlying storage, and following it on overlayfs. Applying the same logic to acl: we do need to call the security_inode_getxattr() on the underlying fs, even if just want to check permissions on overlay. This is currently not done, which is an inconsistency. Maybe adding the check to ovl_get_acl() is the right way to go, but I'm a little afraid of a performance regression. Will look into that. So this patchset nicely reveals how acl retrieval could be done two ways, and how overlayfs employed both for different purposes. But what would be even nicer if there was just one way to retrieve the acl and overlayfs and cifs be moved over to that. Thanks, Miklos
On Fri, Sep 30, 2022 at 02:24:33PM +0200, Miklos Szeredi wrote: > On Fri, 30 Sept 2022 at 12:06, Christian Brauner <brauner@kernel.org> wrote: > > > > On Fri, Sep 30, 2022 at 11:43:07AM +0200, Miklos Szeredi wrote: > > > 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. > > > > Oh sorry, I misunderstood your questions then. The reason why I didn't > > consolidate them was simply the different in permission checking. > > So currently in current mainline overlayfs does acl = get_acl() in it's > > get acl method and does vfs_getxattr() in ovl_posix_acl_xattr_get(). > > > > The difference is that vfs_getxattr() goes through regular lsm hooks > > checking whereas get_acl() does not. So I thought that using get_acl() > > was done to not call lsm hooks in there. If that's not the case then I > > can consolidate both into one implementation. > > So there are two paths to getting an acl: 1) permission checking and > 2) retrieving the value via getxattr(2). > > This is a similar situation as reading a symlink vs. following it. > When following a symlink overlayfs always reads the link on the > underlying fs just as if it was a readlink(2) call, calling > security_inode_readlink() instead of security_inode_follow_link(). > This is logical: we are reading the link from the underlying storage, > and following it on overlayfs. > > Applying the same logic to acl: we do need to call the > security_inode_getxattr() on the underlying fs, even if just want to > check permissions on overlay. This is currently not done, which is an > inconsistency. > > Maybe adding the check to ovl_get_acl() is the right way to go, but > I'm a little afraid of a performance regression. Will look into that. Ok, sounds good. I can probably consolidate the two versions but retain the difference in permission checking or would you prefer I leave them distinct for now? > > So this patchset nicely reveals how acl retrieval could be done two > ways, and how overlayfs employed both for different purposes. But > what would be even nicer if there was just one way to retrieve the acl > and overlayfs and cifs be moved over to that. I think this is a good long term goal to have. We're certainly not done with improving things after this work. Sometimes it just takes a little time to phase out legacy baggage as we all are well aware.
On Fri, 30 Sept 2022 at 14:49, Christian Brauner <brauner@kernel.org> wrote: > > On Fri, Sep 30, 2022 at 02:24:33PM +0200, Miklos Szeredi wrote: > > Maybe adding the check to ovl_get_acl() is the right way to go, but > > I'm a little afraid of a performance regression. Will look into that. > > Ok, sounds good. I can probably consolidate the two versions but retain > the difference in permission checking or would you prefer I leave them > distinct for now? No, please consolidate them. That's a good first step. > > So this patchset nicely reveals how acl retrieval could be done two > > ways, and how overlayfs employed both for different purposes. But > > what would be even nicer if there was just one way to retrieve the acl > > and overlayfs and cifs be moved over to that. > > I think this is a good long term goal to have. We're certainly not done > with improving things after this work. Sometimes it just takes a little > time to phase out legacy baggage as we all are well aware. But then which is legacy? The old .get_acl() or the new .get_acl()? My impression is that it's the new one. But in that case the big renaming patch doesn't make any sense. Thanks, Miklos
On Fri, Sep 30, 2022 at 03:01:22PM +0200, Miklos Szeredi wrote: > On Fri, 30 Sept 2022 at 14:49, Christian Brauner <brauner@kernel.org> wrote: > > > > On Fri, Sep 30, 2022 at 02:24:33PM +0200, Miklos Szeredi wrote: > > > > Maybe adding the check to ovl_get_acl() is the right way to go, but > > > I'm a little afraid of a performance regression. Will look into that. > > > > Ok, sounds good. I can probably consolidate the two versions but retain > > the difference in permission checking or would you prefer I leave them > > distinct for now? > > No, please consolidate them. That's a good first step. Ok, will do. > > > > So this patchset nicely reveals how acl retrieval could be done two > > > ways, and how overlayfs employed both for different purposes. But > > > what would be even nicer if there was just one way to retrieve the acl > > > and overlayfs and cifs be moved over to that. > > > > I think this is a good long term goal to have. We're certainly not done > > with improving things after this work. Sometimes it just takes a little > > time to phase out legacy baggage as we all are well aware. > > But then which is legacy? The old .get_acl() or the new .get_acl()? > My impression is that it's the new one. But in that case the big > renaming patch doesn't make any sense. Since "legacy" would mean "the older one" it wasn't the best term here. It'll come down to which one we will be able to remove. I'm exploring both options. If you want to make the removal of the renaming a hard requirement for being ok with this series I can reverse the renaming. (Note that Christoph had requested consistent naming for get and set acl.)
On Fri, Sep 30, 2022 at 5:06 AM Miklos Szeredi <miklos@szeredi.hu> wrote: > > 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. ... > > > 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. Can you give an example of this? How can you read an ACL without an inode or open file struct? ACL wouldn't fit in a dentry right? By the way there is an option that we can use on open to return the "maximal access" that that user/group has for the file (a 32 bit mask showing whether the effective user has read, write, append, read attributes, write acl etc. permissions). Would this be helpful for you to have us do when you revalidate dentries? > 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? It doesn't do much good to check if user1 on client1 can access the file on server if any user on client2 can access the file - unless the server is checking ACLs, so the server checks are the more important ones. The permission checking on the client doesn't really matter in many scenarios (the security checks that matter are usually only on the server). The ACLs are stored on the server and evaluated by the server so duplicating ACL evaluation on BOTH client and server only helps in cases where the server doesn't know who the local Linux user is (e.g. single user mounts - where all local users use the same authenticated session). It is common e.g. to mount with "noperm" mount option - in which case the client checks are turned off (since the server ones are the checks that matter the most). Note that the SMB3 protocol (and also NFSv4.1/4.2) support a richer ACL model on the server that is more secure (or at least more granular) in some scenarios than the simpler POSIX ACL model. Are there examples of how this would work for the richacl examples (e.g. NFSv4.1/4.2 or cifs.ko or NTFS or ...)?
On Tue, Oct 04, 2022 at 02:53:41PM -0500, Steve French wrote: > On Fri, Sep 30, 2022 at 5:06 AM Miklos Szeredi <miklos@szeredi.hu> wrote: > > > > 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. > ... > > > > 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. > > Can you give an example of this? How can you read an ACL without an > inode or open file struct? ACL wouldn't fit in a dentry right? By We're just talking about thet fact that {g,s}etxattr(system.posix_acl_{access,default}) work on cifs but getting acls based on inode operations isn't supported. Consequently you can't use the acls for permission checking in the vfs for cifs. If as you say below that's intentional because the client doesn't perform access checks then that's probably fine.
On Wed, 5 Oct 2022 at 09:15, Christian Brauner <brauner@kernel.org> wrote: > We're just talking about thet fact that > {g,s}etxattr(system.posix_acl_{access,default}) work on cifs but > getting acls based on inode operations isn't supported. Consequently you > can't use the acls for permission checking in the vfs for cifs. If as > you say below that's intentional because the client doesn't perform > access checks then that's probably fine. Now I just need to wrap my head around how this interacts with all the uid/gid transformations. Do these (userns, mnt_userns) even make sense for the case of remotely checked permissions? Thanks, Miklos
On Thu, Oct 06, 2022 at 08:31:47AM +0200, Miklos Szeredi wrote: > On Wed, 5 Oct 2022 at 09:15, Christian Brauner <brauner@kernel.org> wrote: > > > We're just talking about thet fact that > > {g,s}etxattr(system.posix_acl_{access,default}) work on cifs but > > getting acls based on inode operations isn't supported. Consequently you > > can't use the acls for permission checking in the vfs for cifs. If as > > you say below that's intentional because the client doesn't perform > > access checks then that's probably fine. > > Now I just need to wrap my head around how this interacts with all the > uid/gid transformations. Currently it doesn't because cifs doesn't support idmapped mounts. > > Do these (userns, mnt_userns) even make sense for the case of remotely > checked permissions? Namespaces are local concepts. They are relevant for permission checking and are e.g., used to generate a {g,u}id that may be sent to a server. A concrete example would be a network filesystems that would change the ownership of a file and the client calls it's ->setattr() inode operation. The fs_userns and/or mnt_userns is used to generate a raw {g,u}id value to be sent to the server (So all netns call from_kuid() ultimately to send a raw {g,u}id over the wire. The server can then do whatever additional permission checking it wants based on that {g,u}id. For acls it's the same. We use the namespaces to generate the raw values and send them to the server that stores them. Either in the acl uapi format or if the netfs has a custom format for acls or translate them into it's own acl format. The server can then use them for permission checking however it wants. But if the server allows the client to retrieve them during permission checking in the vfs then we need to translate that raw format from the server into the proper local format again at which point the namespaces are relevant again. I hope that helped.
On Thu, 6 Oct 2022 at 09:41, Christian Brauner <brauner@kernel.org> wrote: > > On Thu, Oct 06, 2022 at 08:31:47AM +0200, Miklos Szeredi wrote: > > On Wed, 5 Oct 2022 at 09:15, Christian Brauner <brauner@kernel.org> wrote: > > > > > We're just talking about thet fact that > > > {g,s}etxattr(system.posix_acl_{access,default}) work on cifs but > > > getting acls based on inode operations isn't supported. Consequently you > > > can't use the acls for permission checking in the vfs for cifs. If as > > > you say below that's intentional because the client doesn't perform > > > access checks then that's probably fine. > > > > Now I just need to wrap my head around how this interacts with all the > > uid/gid transformations. > > Currently it doesn't because cifs doesn't support idmapped mounts. > > > > > Do these (userns, mnt_userns) even make sense for the case of remotely > > checked permissions? > > Namespaces are local concepts. They are relevant for permission checking > and are e.g., used to generate a {g,u}id that may be sent to a server. A > concrete example would be a network filesystems that would change the > ownership of a file and the client calls it's ->setattr() inode > operation. The fs_userns and/or mnt_userns is used to generate a raw > {g,u}id value to be sent to the server (So all netns call from_kuid() > ultimately to send a raw {g,u}id over the wire. The server can then do > whatever additional permission checking it wants based on that {g,u}id. > > For acls it's the same. We use the namespaces to generate the raw values > and send them to the server that stores them. Either in the acl uapi > format or if the netfs has a custom format for acls or translate them > into it's own acl format. The server can then use them for permission > checking however it wants. But if the server allows the client to > retrieve them during permission checking in the vfs then we need to > translate that raw format from the server into the proper local format > again at which point the namespaces are relevant again. > I hope that helped. Yes, I got it now. Thanks, Miklos
diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst index 1cd2930e54ee..4a0fcaeec58c 100644 --- a/Documentation/filesystems/locking.rst +++ b/Documentation/filesystems/locking.rst @@ -83,6 +83,7 @@ prototypes:: int (*fileattr_set)(struct user_namespace *mnt_userns, struct dentry *dentry, struct fileattr *fa); int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa); + struct posix_acl * (*get_acl)(struct user_namespace *, struct dentry *, int); locking rules: all may block @@ -104,6 +105,7 @@ get_link: no setattr: exclusive permission: no (may not block if called in rcu-walk mode) get_inode_acl: no +get_acl: no getattr: no listxattr: no fiemap: no diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst index 4fc6f1e23012..344f5f421c64 100644 --- a/Documentation/filesystems/vfs.rst +++ b/Documentation/filesystems/vfs.rst @@ -440,6 +440,7 @@ As of kernel 2.6.22, the following members are defined: int (*atomic_open)(struct inode *, struct dentry *, struct file *, unsigned open_flag, umode_t create_mode); int (*tmpfile) (struct user_namespace *, struct inode *, struct dentry *, umode_t); + struct posix_acl * (*get_acl)(struct user_namespace *, struct dentry *, int); int (*set_acl)(struct user_namespace *, struct inode *, struct posix_acl *, int); int (*fileattr_set)(struct user_namespace *mnt_userns, struct dentry *dentry, struct fileattr *fa); diff --git a/include/linux/fs.h b/include/linux/fs.h index 11cddd040578..badff81b9dde 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2168,6 +2168,8 @@ struct inode_operations { umode_t create_mode); int (*tmpfile) (struct user_namespace *, struct inode *, struct dentry *, umode_t); + struct posix_acl *(*get_acl)(struct user_namespace *, struct dentry *, + int); int (*set_acl)(struct user_namespace *, struct dentry *, struct posix_acl *, int); int (*fileattr_set)(struct user_namespace *mnt_userns,
The current way of setting and getting posix acls through the generic xattr interface is error prone and type unsafe. The vfs needs to interpret and fixup posix acls before storing or reporting it to userspace. Various hacks exist to make this work. The code is hard to understand and difficult to maintain in it's current form. Instead of making this work by hacking posix acls through xattr handlers we are building a dedicated posix acl api around the get and set inode operations. This removes a lot of hackiness and makes the codepaths easier to maintain. A lot of background can be found in [1]. Since some filesystem rely on the dentry being available to them when setting posix acls (e.g., 9p and cifs) they cannot rely on the old get acl inode operation to retrieve posix acl and need to implement their own custom handlers because of that. In a previous patch we renamed the old get acl inode operation to ->get_inode_acl(). We decided to rename it and implement a new one since ->get_inode_acl() is called generic_permission() and inode_permission() both of which can be called during an filesystem's ->permission() handler. So simply passing a dentry argument to ->get_acl() would have amounted to also having to pass a dentry argument to ->permission(). We avoided that change. 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. Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org [1] Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> --- Notes: /* v2 */ unchanged /* v3 */ unchanged /* v4 */ unchanged Documentation/filesystems/locking.rst | 2 ++ Documentation/filesystems/vfs.rst | 1 + include/linux/fs.h | 2 ++ 3 files changed, 5 insertions(+)