| Message ID | 20220926140827.142806-18-brauner@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acl: add vfs posix acl api | expand |
On Mon, Sep 26, 2022 at 11:24 AM Christian Brauner <brauner@kernel.org> wrote: > > In previous patches we implemented get and set inode operations for all > non-stacking filesystems that support posix acls but didn't yet > implement get and/or set acl inode operations. This specifically > affected cifs and 9p. > > Now we can build a posix acl api based solely on get and set inode > operations. We add a new vfs_remove_acl() api that can be used to set > posix acls. This finally removes all type unsafety and type conversion > issues explained in detail in [1] that we aim to get rid of. > > After we finished building the vfs api we can switch stacking > filesystems to rely on the new posix api and then finally switch the > xattr system calls themselves to rely on the posix acl api. > > 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 > > fs/posix_acl.c | 65 +++++++++++++++++++++++++++++++++++++++ > include/linux/posix_acl.h | 8 +++++ > 2 files changed, 73 insertions(+) ... > diff --git a/fs/posix_acl.c b/fs/posix_acl.c > index 18873be583a9..40038851bfe1 100644 > --- a/fs/posix_acl.c > +++ b/fs/posix_acl.c > @@ -1484,3 +1484,68 @@ struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns, > return acl; > } > EXPORT_SYMBOL(vfs_get_acl); > + > +/** > + * vfs_remove_acl - remove posix acls > + * @mnt_userns: user namespace of the mount > + * @dentry: the dentry based on which to retrieve the posix acls > + * @acl_name: the name of the posix acl > + * > + * This function removes posix acls. > + * > + * Return: On success 0, on error negative errno. > + */ > +int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry, > + const char *acl_name) > +{ > + int acl_type; > + int error; > + struct inode *inode = d_inode(dentry); > + struct inode *delegated_inode = NULL; > + > + acl_type = posix_acl_type(acl_name); > + if (acl_type < 0) > + return -EINVAL; > + > +retry_deleg: > + inode_lock(inode); > + > + /* > + * We only care about restrictions the inode struct itself places upon > + * us otherwise POSIX ACLs aren't subject to any VFS restrictions. > + */ > + error = xattr_permission(mnt_userns, inode, acl_name, MAY_WRITE); > + if (error) > + goto out_inode_unlock; > + > + error = security_inode_removexattr(mnt_userns, dentry, acl_name); > + if (error) > + goto out_inode_unlock; Similar to my comments in patch 16/30 for vfs_get_acl(), I would suggest a dedicated ACL remove hook here. Yes, it's still a little bit silly, but if we are going to make one dedicated hook, we might as well do them all. > + error = try_break_deleg(inode, &delegated_inode); > + if (error) > + goto out_inode_unlock; > + > + if (inode->i_opflags & IOP_XATTR) > + error = set_posix_acl(mnt_userns, dentry, acl_type, NULL); > + else if (unlikely(is_bad_inode(inode))) > + error = -EIO; > + else > + error = -EOPNOTSUPP; > + if (!error) { > + fsnotify_xattr(dentry); > + evm_inode_post_removexattr(dentry, acl_name); > + } > + > +out_inode_unlock: > + inode_unlock(inode); > + > + if (delegated_inode) { > + error = break_deleg_wait(&delegated_inode); > + if (!error) > + goto retry_deleg; > + } > + > + return error; > +} > +EXPORT_SYMBOL(vfs_remove_acl); -- paul-moore.com
On Tue, Sep 27, 2022 at 06:55:37PM -0400, Paul Moore wrote: > On Mon, Sep 26, 2022 at 11:24 AM Christian Brauner <brauner@kernel.org> wrote: > > > > In previous patches we implemented get and set inode operations for all > > non-stacking filesystems that support posix acls but didn't yet > > implement get and/or set acl inode operations. This specifically > > affected cifs and 9p. > > > > Now we can build a posix acl api based solely on get and set inode > > operations. We add a new vfs_remove_acl() api that can be used to set > > posix acls. This finally removes all type unsafety and type conversion > > issues explained in detail in [1] that we aim to get rid of. > > > > After we finished building the vfs api we can switch stacking > > filesystems to rely on the new posix api and then finally switch the > > xattr system calls themselves to rely on the posix acl api. > > > > 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 > > > > fs/posix_acl.c | 65 +++++++++++++++++++++++++++++++++++++++ > > include/linux/posix_acl.h | 8 +++++ > > 2 files changed, 73 insertions(+) > > ... > > > diff --git a/fs/posix_acl.c b/fs/posix_acl.c > > index 18873be583a9..40038851bfe1 100644 > > --- a/fs/posix_acl.c > > +++ b/fs/posix_acl.c > > @@ -1484,3 +1484,68 @@ struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns, > > return acl; > > } > > EXPORT_SYMBOL(vfs_get_acl); > > + > > +/** > > + * vfs_remove_acl - remove posix acls > > + * @mnt_userns: user namespace of the mount > > + * @dentry: the dentry based on which to retrieve the posix acls > > + * @acl_name: the name of the posix acl > > + * > > + * This function removes posix acls. > > + * > > + * Return: On success 0, on error negative errno. > > + */ > > +int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry, > > + const char *acl_name) > > +{ > > + int acl_type; > > + int error; > > + struct inode *inode = d_inode(dentry); > > + struct inode *delegated_inode = NULL; > > + > > + acl_type = posix_acl_type(acl_name); > > + if (acl_type < 0) > > + return -EINVAL; > > + > > +retry_deleg: > > + inode_lock(inode); > > + > > + /* > > + * We only care about restrictions the inode struct itself places upon > > + * us otherwise POSIX ACLs aren't subject to any VFS restrictions. > > + */ > > + error = xattr_permission(mnt_userns, inode, acl_name, MAY_WRITE); > > + if (error) > > + goto out_inode_unlock; > > + > > + error = security_inode_removexattr(mnt_userns, dentry, acl_name); > > + if (error) > > + goto out_inode_unlock; > > Similar to my comments in patch 16/30 for vfs_get_acl(), I would > suggest a dedicated ACL remove hook here. Yes, it's still a little > bit silly, but if we are going to make one dedicated hook, we might as > well do them all. Sure, I don't mind doing that. I'll add the infrastructure and then the individual LSMs can add their own hooks.
diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 18873be583a9..40038851bfe1 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -1484,3 +1484,68 @@ struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns, return acl; } EXPORT_SYMBOL(vfs_get_acl); + +/** + * vfs_remove_acl - remove posix acls + * @mnt_userns: user namespace of the mount + * @dentry: the dentry based on which to retrieve the posix acls + * @acl_name: the name of the posix acl + * + * This function removes posix acls. + * + * Return: On success 0, on error negative errno. + */ +int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry, + const char *acl_name) +{ + int acl_type; + int error; + struct inode *inode = d_inode(dentry); + struct inode *delegated_inode = NULL; + + acl_type = posix_acl_type(acl_name); + if (acl_type < 0) + return -EINVAL; + +retry_deleg: + inode_lock(inode); + + /* + * We only care about restrictions the inode struct itself places upon + * us otherwise POSIX ACLs aren't subject to any VFS restrictions. + */ + error = xattr_permission(mnt_userns, inode, acl_name, MAY_WRITE); + if (error) + goto out_inode_unlock; + + error = security_inode_removexattr(mnt_userns, dentry, acl_name); + if (error) + goto out_inode_unlock; + + error = try_break_deleg(inode, &delegated_inode); + if (error) + goto out_inode_unlock; + + if (inode->i_opflags & IOP_XATTR) + error = set_posix_acl(mnt_userns, dentry, acl_type, NULL); + else if (unlikely(is_bad_inode(inode))) + error = -EIO; + else + error = -EOPNOTSUPP; + if (!error) { + fsnotify_xattr(dentry); + evm_inode_post_removexattr(dentry, acl_name); + } + +out_inode_unlock: + inode_unlock(inode); + + if (delegated_inode) { + error = break_deleg_wait(&delegated_inode); + if (!error) + goto retry_deleg; + } + + return error; +} +EXPORT_SYMBOL(vfs_remove_acl); diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 06e65b1c6e28..c5dd541babc0 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -104,6 +104,8 @@ int vfs_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry, const char *acl_name, struct posix_acl *kacl); struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns, struct dentry *dentry, const char *acl_name); +int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry, + const char *acl_name); #else static inline int posix_acl_chmod(struct user_namespace *mnt_userns, struct dentry *dentry, umode_t mode) @@ -145,6 +147,12 @@ static inline struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns, { return NULL; } + +static inline int vfs_remove_acl(struct user_namespace *mnt_userns, + struct dentry *dentry, const char *acl_name) +{ + return 0; +} #endif /* CONFIG_FS_POSIX_ACL */ struct posix_acl *get_acl(struct inode *inode, int type);
In previous patches we implemented get and set inode operations for all non-stacking filesystems that support posix acls but didn't yet implement get and/or set acl inode operations. This specifically affected cifs and 9p. Now we can build a posix acl api based solely on get and set inode operations. We add a new vfs_remove_acl() api that can be used to set posix acls. This finally removes all type unsafety and type conversion issues explained in detail in [1] that we aim to get rid of. After we finished building the vfs api we can switch stacking filesystems to rely on the new posix api and then finally switch the xattr system calls themselves to rely on the posix acl api. 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 fs/posix_acl.c | 65 +++++++++++++++++++++++++++++++++++++++ include/linux/posix_acl.h | 8 +++++ 2 files changed, 73 insertions(+)