| Message ID | 20230125-fs-acl-remove-generic-xattr-handlers-v1-6-6cf155b492b6@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acl: remove remaining posix acl handlers | expand |
On Wed 25-01-23 12:28:51, Christian Brauner wrote: > Last cycle we introduced a new posix acl api. Filesystems now only need > to implement the inode operations for posix acls. The generic xattr > handlers aren't used anymore by the vfs and will be completely removed. > Keeping the handler around is confusing and gives the false impression > that the xattr infrastructure of the vfs is used to interact with posix > acls when it really isn't anymore. > > For this to work we simply rework the ->listxattr() inode operation to > not rely on the generix posix acl handlers anymore. > > Cc: Jan Kara <jack@suse.com> > Cc: <linux-ext4@vger.kernel.org> > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> Looks good to me. Feel free to add: Acked-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext2/xattr.c | 60 +++++++++++++++++++++++++++++++++------------------------ > 1 file changed, 35 insertions(+), 25 deletions(-) > > diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c > index 641abfa4b718..86ba6a33349e 100644 > --- a/fs/ext2/xattr.c > +++ b/fs/ext2/xattr.c > @@ -98,25 +98,9 @@ static struct buffer_head *ext2_xattr_cache_find(struct inode *, > static void ext2_xattr_rehash(struct ext2_xattr_header *, > struct ext2_xattr_entry *); > > -static const struct xattr_handler *ext2_xattr_handler_map[] = { > - [EXT2_XATTR_INDEX_USER] = &ext2_xattr_user_handler, > -#ifdef CONFIG_EXT2_FS_POSIX_ACL > - [EXT2_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler, > - [EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler, > -#endif > - [EXT2_XATTR_INDEX_TRUSTED] = &ext2_xattr_trusted_handler, > -#ifdef CONFIG_EXT2_FS_SECURITY > - [EXT2_XATTR_INDEX_SECURITY] = &ext2_xattr_security_handler, > -#endif > -}; > - > const struct xattr_handler *ext2_xattr_handlers[] = { > &ext2_xattr_user_handler, > &ext2_xattr_trusted_handler, > -#ifdef CONFIG_EXT2_FS_POSIX_ACL > - &posix_acl_access_xattr_handler, > - &posix_acl_default_xattr_handler, > -#endif > #ifdef CONFIG_EXT2_FS_SECURITY > &ext2_xattr_security_handler, > #endif > @@ -125,14 +109,41 @@ const struct xattr_handler *ext2_xattr_handlers[] = { > > #define EA_BLOCK_CACHE(inode) (EXT2_SB(inode->i_sb)->s_ea_block_cache) > > -static inline const struct xattr_handler * > -ext2_xattr_handler(int name_index) > +static const char *ext2_xattr_prefix(int xattr_index, struct dentry *dentry) > { > + const char *name = NULL; > const struct xattr_handler *handler = NULL; > > - if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map)) > - handler = ext2_xattr_handler_map[name_index]; > - return handler; > + switch (xattr_index) { > + case EXT2_XATTR_INDEX_USER: > + handler = &ext2_xattr_user_handler; > + break; > + case EXT2_XATTR_INDEX_TRUSTED: > + handler = &ext2_xattr_trusted_handler; > + break; > +#ifdef CONFIG_EXT2_FS_SECURITY > + case EXT2_XATTR_INDEX_SECURITY: > + handler = &ext2_xattr_security_handler; > + break; > +#endif > +#ifdef CONFIG_EXT2_FS_POSIX_ACL > + case EXT2_XATTR_INDEX_POSIX_ACL_ACCESS: > + if (posix_acl_dentry_list(dentry)) > + name = XATTR_NAME_POSIX_ACL_ACCESS; > + break; > + case EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT: > + if (posix_acl_dentry_list(dentry)) > + name = XATTR_NAME_POSIX_ACL_DEFAULT; > + break; > +#endif > + default: > + return NULL; > + } > + > + if (xattr_dentry_list(handler, dentry)) > + name = xattr_prefix(handler); > + > + return name; > } > > static bool > @@ -333,11 +344,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size) > /* list the attribute names */ > for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry); > entry = EXT2_XATTR_NEXT(entry)) { > - const struct xattr_handler *handler = > - ext2_xattr_handler(entry->e_name_index); > + const char *prefix; > > - if (handler && (!handler->list || handler->list(dentry))) { > - const char *prefix = handler->prefix ?: handler->name; > + prefix = ext2_xattr_prefix(entry->e_name_index, dentry); > + if (prefix) { > size_t prefix_len = strlen(prefix); > size_t size = prefix_len + entry->e_name_len + 1; > > > -- > 2.34.1 >
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c index 641abfa4b718..86ba6a33349e 100644 --- a/fs/ext2/xattr.c +++ b/fs/ext2/xattr.c @@ -98,25 +98,9 @@ static struct buffer_head *ext2_xattr_cache_find(struct inode *, static void ext2_xattr_rehash(struct ext2_xattr_header *, struct ext2_xattr_entry *); -static const struct xattr_handler *ext2_xattr_handler_map[] = { - [EXT2_XATTR_INDEX_USER] = &ext2_xattr_user_handler, -#ifdef CONFIG_EXT2_FS_POSIX_ACL - [EXT2_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler, - [EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler, -#endif - [EXT2_XATTR_INDEX_TRUSTED] = &ext2_xattr_trusted_handler, -#ifdef CONFIG_EXT2_FS_SECURITY - [EXT2_XATTR_INDEX_SECURITY] = &ext2_xattr_security_handler, -#endif -}; - const struct xattr_handler *ext2_xattr_handlers[] = { &ext2_xattr_user_handler, &ext2_xattr_trusted_handler, -#ifdef CONFIG_EXT2_FS_POSIX_ACL - &posix_acl_access_xattr_handler, - &posix_acl_default_xattr_handler, -#endif #ifdef CONFIG_EXT2_FS_SECURITY &ext2_xattr_security_handler, #endif @@ -125,14 +109,41 @@ const struct xattr_handler *ext2_xattr_handlers[] = { #define EA_BLOCK_CACHE(inode) (EXT2_SB(inode->i_sb)->s_ea_block_cache) -static inline const struct xattr_handler * -ext2_xattr_handler(int name_index) +static const char *ext2_xattr_prefix(int xattr_index, struct dentry *dentry) { + const char *name = NULL; const struct xattr_handler *handler = NULL; - if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map)) - handler = ext2_xattr_handler_map[name_index]; - return handler; + switch (xattr_index) { + case EXT2_XATTR_INDEX_USER: + handler = &ext2_xattr_user_handler; + break; + case EXT2_XATTR_INDEX_TRUSTED: + handler = &ext2_xattr_trusted_handler; + break; +#ifdef CONFIG_EXT2_FS_SECURITY + case EXT2_XATTR_INDEX_SECURITY: + handler = &ext2_xattr_security_handler; + break; +#endif +#ifdef CONFIG_EXT2_FS_POSIX_ACL + case EXT2_XATTR_INDEX_POSIX_ACL_ACCESS: + if (posix_acl_dentry_list(dentry)) + name = XATTR_NAME_POSIX_ACL_ACCESS; + break; + case EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT: + if (posix_acl_dentry_list(dentry)) + name = XATTR_NAME_POSIX_ACL_DEFAULT; + break; +#endif + default: + return NULL; + } + + if (xattr_dentry_list(handler, dentry)) + name = xattr_prefix(handler); + + return name; } static bool @@ -333,11 +344,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size) /* list the attribute names */ for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry); entry = EXT2_XATTR_NEXT(entry)) { - const struct xattr_handler *handler = - ext2_xattr_handler(entry->e_name_index); + const char *prefix; - if (handler && (!handler->list || handler->list(dentry))) { - const char *prefix = handler->prefix ?: handler->name; + prefix = ext2_xattr_prefix(entry->e_name_index, dentry); + if (prefix) { size_t prefix_len = strlen(prefix); size_t size = prefix_len + entry->e_name_len + 1;
Last cycle we introduced a new posix acl api. Filesystems now only need to implement the inode operations for posix acls. The generic xattr handlers aren't used anymore by the vfs and will be completely removed. Keeping the handler around is confusing and gives the false impression that the xattr infrastructure of the vfs is used to interact with posix acls when it really isn't anymore. For this to work we simply rework the ->listxattr() inode operation to not rely on the generix posix acl handlers anymore. Cc: Jan Kara <jack@suse.com> Cc: <linux-ext4@vger.kernel.org> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> --- fs/ext2/xattr.c | 60 +++++++++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 25 deletions(-)