* [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
@ 2023-02-01 13:14 Christian Brauner
2023-02-01 13:14 ` [PATCH v3 01/10] xattr: simplify listxattr helpers Christian Brauner
` (12 more replies)
0 siblings, 13 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd,
reiserfs-devel, linux-unionfs
Hey everyone,
After we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handlers around in the
filesystems xattr handlers for two reasons:
(1) Because a few filesystems rely on the ->list() method of the generic
POSIX ACL xattr handlers in their ->listxattr() inode operation.
(2) POSIX ACLs are only available if IOP_XATTR is raised. The IOP_XATTR
flag is raised in inode_init_always() based on whether the
sb->s_xattr pointer is non-NULL. IOW, the registered xattr handlers
of the filesystem are used to raise IOP_XATTR.
If we were to remove the generic POSIX ACL xattr handlers from all
filesystems we would risk regressing filesystems that only implement
POSIX ACL support and no other xattrs (nfs3 comes to mind).
This series makes it possible to remove the generic POSIX ACL xattr
handlers from the sb->s_xattr list of all filesystems. This is a crucial
step as the generic POSIX ACL xattr handlers aren't used for POSIX ACLs
anymore and POSIX ACLs don't depend on the xattr infrastructure in a
meaningful way anymore.
Adressing problem (1) will require more work long-term. It would be best
to get rid of the ->list() method of xattr handlers completely if we
can.
For erofs, ext{2,4}, f2fs, jffs2, ocfs2, and reiserfs we keep the dummy
handler around so they can continue to use array-based xattr handler
indexing. The series does simplify the ->listxattr() implementation of
all these filesystems.
This series decouples POSIX ACLs from IOP_XATTR as they don't depend on
xattr handlers anymore. With this we can finally remove the dummy xattr
handlers from all filesystems xattr handlers.
All filesystems with reasonable integration into xfstests have been
tested with:
./check -g acl,attr,cap,idmapped,io_uring,perms,unlink
All tests pass without regression on xfstests for-next branch.
Since erofs doesn't have integration into xfstests yet afaict I have
tested it with the testuite available in erofs-utils. They also all pass
without any regressions.
Thanks!
Christian
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Decouple POSIX ACLs from IOP_XATTR.
- Allow vfs_listxattr() to function without checking for IOP_XATTR
making it possible to list POSIX ACLs for filesystems that only
implement POSIX ACLs and no other xattrs.
- Give reiserfs a set of dedicated inode operation for private inodes
that have turned of xattrs completely.
- Link to v2: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v2-0-214cfb88bb56@kernel.org
Changes in v2:
- Please see changelogs of the individual patches.
- Christoph & Christian:
Remove SB_I_XATTR and instead introduce IOP_NOACL so filesystems can
opt out of POSIX ACLs for specific inodes. Decouple POSIX ACLs from
IOP_XATTR.
- Keep generic posix acl xattr handlers so filesystems that use array
based indexing on xattr handlers can continue to do so.
- Minor fixes.
- Link to v1: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-0-6cf155b492b6@kernel.org
---
Christian Brauner (10):
xattr: simplify listxattr helpers
xattr: add listxattr helper
xattr: remove unused argument
fs: drop unused posix acl handlers
fs: simplify ->listxattr() implementation
reiserfs: rework ->listxattr() implementation
fs: rename generic posix acl handlers
reiserfs: rework priv inode handling
ovl: check for ->listxattr() support
acl: don't depend on IOP_XATTR
fs/9p/xattr.c | 4 --
fs/btrfs/xattr.c | 4 --
fs/ceph/xattr.c | 4 --
fs/cifs/xattr.c | 4 --
fs/ecryptfs/inode.c | 4 --
fs/erofs/xattr.c | 12 +---
fs/erofs/xattr.h | 20 ++++---
fs/ext2/xattr.c | 25 ++++----
fs/ext4/xattr.c | 25 ++++----
fs/f2fs/xattr.c | 24 ++++----
fs/gfs2/xattr.c | 2 -
fs/jffs2/xattr.c | 29 +++++-----
fs/jfs/xattr.c | 4 --
fs/nfs/nfs3_fs.h | 1 -
fs/nfs/nfs3acl.c | 6 --
fs/nfs/nfs3super.c | 3 -
fs/nfsd/nfs4xdr.c | 3 +-
fs/ntfs3/xattr.c | 4 --
fs/ocfs2/xattr.c | 14 ++---
fs/orangefs/xattr.c | 2 -
fs/overlayfs/copy_up.c | 3 +-
fs/overlayfs/super.c | 8 ---
fs/posix_acl.c | 61 +++++++++++++++-----
fs/reiserfs/file.c | 7 +++
fs/reiserfs/inode.c | 6 +-
fs/reiserfs/namei.c | 50 ++++++++++++++--
fs/reiserfs/reiserfs.h | 2 +
fs/reiserfs/xattr.c | 55 +++++++++---------
fs/xattr.c | 124 ++++++++++++++++++++--------------------
fs/xfs/xfs_xattr.c | 4 --
include/linux/posix_acl.h | 7 +++
include/linux/posix_acl_xattr.h | 5 +-
include/linux/xattr.h | 19 +++++-
mm/shmem.c | 4 --
34 files changed, 292 insertions(+), 257 deletions(-)
---
base-commit: ab072681eabe1ce0a9a32d4baa1a27a2d046bc4a
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 01/10] xattr: simplify listxattr helpers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 02/10] xattr: add listxattr helper Christian Brauner
` (11 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
The generic_listxattr() and simple_xattr_list() helpers list xattrs and
contain duplicated code. Add two helpers that both generic_listxattr()
and simple_xattr_list() can use.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
- Leave newline after variable declaration in xattr_list_one().
- Move posix_acl_listxattr() into fs/posix_acl.c.
---
fs/posix_acl.c | 25 +++++++++++++
fs/xattr.c | 89 ++++++++++++++++++-----------------------------
include/linux/posix_acl.h | 7 ++++
include/linux/xattr.h | 1 +
4 files changed, 66 insertions(+), 56 deletions(-)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index d7bc81fc0840..c0886dc8e714 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -958,6 +958,31 @@ set_posix_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
}
EXPORT_SYMBOL(set_posix_acl);
+int posix_acl_listxattr(struct inode *inode, char **buffer,
+ ssize_t *remaining_size)
+{
+ int err;
+
+ if (!IS_POSIXACL(inode))
+ return 0;
+
+ if (inode->i_acl) {
+ err = xattr_list_one(buffer, remaining_size,
+ XATTR_NAME_POSIX_ACL_ACCESS);
+ if (err)
+ return err;
+ }
+
+ if (inode->i_default_acl) {
+ err = xattr_list_one(buffer, remaining_size,
+ XATTR_NAME_POSIX_ACL_DEFAULT);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static bool
posix_acl_xattr_list(struct dentry *dentry)
{
diff --git a/fs/xattr.c b/fs/xattr.c
index adab9a70b536..20a562d431fe 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,6 +949,21 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
return error;
}
+int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
+{
+ size_t len;
+
+ len = strlen(name) + 1;
+ if (*buffer) {
+ if (*remaining_size < len)
+ return -ERANGE;
+ memcpy(*buffer, name, len);
+ *buffer += len;
+ }
+ *remaining_size -= len;
+ return 0;
+}
+
/*
* Combine the results of the list() operation from every xattr_handler in the
* list.
@@ -957,33 +972,22 @@ ssize_t
generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
{
const struct xattr_handler *handler, **handlers = dentry->d_sb->s_xattr;
- unsigned int size = 0;
-
- if (!buffer) {
- for_each_xattr_handler(handlers, handler) {
- if (!handler->name ||
- (handler->list && !handler->list(dentry)))
- continue;
- size += strlen(handler->name) + 1;
- }
- } else {
- char *buf = buffer;
- size_t len;
-
- for_each_xattr_handler(handlers, handler) {
- if (!handler->name ||
- (handler->list && !handler->list(dentry)))
- continue;
- len = strlen(handler->name);
- if (len + 1 > buffer_size)
- return -ERANGE;
- memcpy(buf, handler->name, len + 1);
- buf += len + 1;
- buffer_size -= len + 1;
- }
- size = buf - buffer;
+ ssize_t remaining_size = buffer_size;
+ int err = 0;
+
+ err = posix_acl_listxattr(d_inode(dentry), &buffer, &remaining_size);
+ if (err)
+ return err;
+
+ for_each_xattr_handler(handlers, handler) {
+ if (!handler->name || (handler->list && !handler->list(dentry)))
+ continue;
+ err = xattr_list_one(&buffer, &remaining_size, handler->name);
+ if (err)
+ return err;
}
- return size;
+
+ return err ? err : buffer_size - remaining_size;
}
EXPORT_SYMBOL(generic_listxattr);
@@ -1245,20 +1249,6 @@ static bool xattr_is_trusted(const char *name)
return !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN);
}
-static int xattr_list_one(char **buffer, ssize_t *remaining_size,
- const char *name)
-{
- size_t len = strlen(name) + 1;
- if (*buffer) {
- if (*remaining_size < len)
- return -ERANGE;
- memcpy(*buffer, name, len);
- *buffer += len;
- }
- *remaining_size -= len;
- return 0;
-}
-
/**
* simple_xattr_list - list all xattr objects
* @inode: inode from which to get the xattrs
@@ -1287,22 +1277,9 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
ssize_t remaining_size = size;
int err = 0;
-#ifdef CONFIG_FS_POSIX_ACL
- if (IS_POSIXACL(inode)) {
- if (inode->i_acl) {
- err = xattr_list_one(&buffer, &remaining_size,
- XATTR_NAME_POSIX_ACL_ACCESS);
- if (err)
- return err;
- }
- if (inode->i_default_acl) {
- err = xattr_list_one(&buffer, &remaining_size,
- XATTR_NAME_POSIX_ACL_DEFAULT);
- if (err)
- return err;
- }
- }
-#endif
+ err = posix_acl_listxattr(inode, &buffer, &remaining_size);
+ if (err)
+ return err;
read_lock(&xattrs->lock);
for (rbp = rb_first(&xattrs->rb_root); rbp; rbp = rb_next(rbp)) {
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index ee608d22ecb9..e7995c68f641 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -106,6 +106,8 @@ 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);
+int posix_acl_listxattr(struct inode *inode, char **buffer,
+ ssize_t *remaining_size);
#else
static inline int posix_acl_chmod(struct user_namespace *mnt_userns,
struct dentry *dentry, umode_t mode)
@@ -153,6 +155,11 @@ static inline int vfs_remove_acl(struct user_namespace *mnt_userns,
{
return -EOPNOTSUPP;
}
+static inline int posix_acl_listxattr(struct inode *inode, char **buffer,
+ ssize_t *remaining_size)
+{
+ return 0;
+}
#endif /* CONFIG_FS_POSIX_ACL */
struct posix_acl *get_inode_acl(struct inode *inode, int type);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 2e7dd44926e4..74b7770880a7 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -109,5 +109,6 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
char *buffer, size_t size);
void simple_xattr_add(struct simple_xattrs *xattrs,
struct simple_xattr *new_xattr);
+int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name);
#endif /* _LINUX_XATTR_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 02/10] xattr: add listxattr helper
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 01/10] xattr: simplify listxattr helpers Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 03/10] xattr: remove unused argument Christian Brauner
` (10 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
Add a tiny helper to determine whether an xattr handler given a specific
dentry supports listing the requested xattr. We will use this helper in
various filesystems in later commits.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Remove second helper after architectural changes in the series.
- Christoph Hellwig <hch@lst.de>:
- Renamed helper to xattr_handler_can_list().
---
include/linux/xattr.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 74b7770880a7..39c496510a26 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -47,6 +47,22 @@ struct xattr_handler {
size_t size, int flags);
};
+/**
+ * xattr_handler_can_list - check whether xattr can be listed
+ * @handler: handler for this type of xattr
+ * @dentry: dentry whose inode xattr to list
+ *
+ * Determine whether the xattr associated with @dentry can be listed given
+ * @handler.
+ *
+ * Return: true if xattr can be listed, false if not.
+ */
+static inline bool xattr_handler_can_list(const struct xattr_handler *handler,
+ struct dentry *dentry)
+{
+ return handler && (!handler->list || handler->list(dentry));
+}
+
const char *xattr_full_name(const struct xattr_handler *, const char *);
struct xattr {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 03/10] xattr: remove unused argument
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 01/10] xattr: simplify listxattr helpers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 02/10] xattr: add listxattr helper Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 04/10] fs: drop unused posix acl handlers Christian Brauner
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
his helpers is really just used to check for user.* xattr support so
don't make it pointlessly generic.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Patch unchanged.
---
fs/nfsd/nfs4xdr.c | 3 +--
fs/xattr.c | 10 ++++------
include/linux/xattr.h | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 97edb32be77f..79814cfe8bcf 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3442,8 +3442,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
p = xdr_reserve_space(xdr, 4);
if (!p)
goto out_resource;
- err = xattr_supported_namespace(d_inode(dentry),
- XATTR_USER_PREFIX);
+ err = xattr_supports_user_prefix(d_inode(dentry));
*p++ = cpu_to_be32(err == 0);
}
diff --git a/fs/xattr.c b/fs/xattr.c
index 20a562d431fe..8743402a5e8b 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -159,11 +159,10 @@ xattr_permission(struct user_namespace *mnt_userns, struct inode *inode,
* Look for any handler that deals with the specified namespace.
*/
int
-xattr_supported_namespace(struct inode *inode, const char *prefix)
+xattr_supports_user_prefix(struct inode *inode)
{
const struct xattr_handler **handlers = inode->i_sb->s_xattr;
const struct xattr_handler *handler;
- size_t preflen;
if (!(inode->i_opflags & IOP_XATTR)) {
if (unlikely(is_bad_inode(inode)))
@@ -171,16 +170,15 @@ xattr_supported_namespace(struct inode *inode, const char *prefix)
return -EOPNOTSUPP;
}
- preflen = strlen(prefix);
-
for_each_xattr_handler(handlers, handler) {
- if (!strncmp(xattr_prefix(handler), prefix, preflen))
+ if (!strncmp(xattr_prefix(handler), XATTR_USER_PREFIX,
+ XATTR_USER_PREFIX_LEN))
return 0;
}
return -EOPNOTSUPP;
}
-EXPORT_SYMBOL(xattr_supported_namespace);
+EXPORT_SYMBOL(xattr_supports_user_prefix);
int
__vfs_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 39c496510a26..e24f052e8dcd 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -94,7 +94,7 @@ int vfs_getxattr_alloc(struct user_namespace *mnt_userns,
struct dentry *dentry, const char *name,
char **xattr_value, size_t size, gfp_t flags);
-int xattr_supported_namespace(struct inode *inode, const char *prefix);
+int xattr_supports_user_prefix(struct inode *inode);
static inline const char *xattr_prefix(const struct xattr_handler *handler)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 04/10] fs: drop unused posix acl handlers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (2 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 03/10] xattr: remove unused argument Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 05/10] fs: simplify ->listxattr() implementation Christian Brauner
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
Remove struct posix_acl_{access,default}_handler for all filesystems
that don't depend on the xattr handler in their inode->i_op->listxattr()
method in any way. There's nothing more to do than to simply remove the
handler. It's been effectively unused ever since we introduced the new
posix acl api.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Fold all filesystem changes except reiserfs into this patch.
---
fs/9p/xattr.c | 4 ----
fs/btrfs/xattr.c | 4 ----
fs/ceph/xattr.c | 4 ----
fs/cifs/xattr.c | 4 ----
fs/ecryptfs/inode.c | 4 ----
fs/erofs/xattr.c | 4 ----
fs/ext2/xattr.c | 4 ----
fs/ext4/xattr.c | 4 ----
fs/f2fs/xattr.c | 4 ----
fs/gfs2/xattr.c | 2 --
fs/jffs2/xattr.c | 4 ----
fs/jfs/xattr.c | 4 ----
fs/nfs/nfs3_fs.h | 1 -
fs/nfs/nfs3acl.c | 6 ------
fs/nfs/nfs3super.c | 3 ---
fs/ntfs3/xattr.c | 4 ----
fs/ocfs2/xattr.c | 2 --
fs/orangefs/xattr.c | 2 --
fs/overlayfs/super.c | 8 --------
fs/xfs/xfs_xattr.c | 4 ----
mm/shmem.c | 4 ----
21 files changed, 80 deletions(-)
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index b6984311e00a..1d2df17b450f 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -183,10 +183,6 @@ static struct xattr_handler v9fs_xattr_security_handler = {
const struct xattr_handler *v9fs_xattr_handlers[] = {
&v9fs_xattr_user_handler,
&v9fs_xattr_trusted_handler,
-#ifdef CONFIG_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
#ifdef CONFIG_9P_FS_SECURITY
&v9fs_xattr_security_handler,
#endif
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 0ed4b119a7ca..a6abe528c5d8 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -444,10 +444,6 @@ static const struct xattr_handler btrfs_btrfs_xattr_handler = {
const struct xattr_handler *btrfs_xattr_handlers[] = {
&btrfs_security_xattr_handler,
-#ifdef CONFIG_BTRFS_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&btrfs_trusted_xattr_handler,
&btrfs_user_xattr_handler,
&btrfs_btrfs_xattr_handler,
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index f31350cda960..22e22e8dc226 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1411,10 +1411,6 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
* attributes are handled directly.
*/
const struct xattr_handler *ceph_xattr_handlers[] = {
-#ifdef CONFIG_CEPH_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&ceph_other_xattr_handler,
NULL,
};
diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 5f2fb2fd2e37..1b50814eadbb 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -487,9 +487,5 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
&cifs_cifs_ntsd_full_xattr_handler,
&smb3_ntsd_full_xattr_handler, /* alias for above since avoiding "cifs" */
-#ifdef CONFIG_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
NULL
};
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index f3cd00fac9c3..5802b93b2cda 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1210,10 +1210,6 @@ static const struct xattr_handler ecryptfs_xattr_handler = {
};
const struct xattr_handler *ecryptfs_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&ecryptfs_xattr_handler,
NULL
};
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..2c98a15a92ed 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -469,10 +469,6 @@ const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
const struct xattr_handler *erofs_xattr_handlers[] = {
&erofs_xattr_user_handler,
-#ifdef CONFIG_EROFS_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&erofs_xattr_trusted_handler,
#ifdef CONFIG_EROFS_FS_SECURITY
&erofs_xattr_security_handler,
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 641abfa4b718..262951ffe8d0 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -113,10 +113,6 @@ static const struct xattr_handler *ext2_xattr_handler_map[] = {
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
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index a2f04a3808db..ba7f2557adb8 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -101,10 +101,6 @@ static const struct xattr_handler * const ext4_xattr_handler_map[] = {
const struct xattr_handler *ext4_xattr_handlers[] = {
&ext4_xattr_user_handler,
&ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT4_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
#ifdef CONFIG_EXT4_FS_SECURITY
&ext4_xattr_security_handler,
#endif
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index dc2e8637189e..ccb564e328af 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -204,10 +204,6 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
const struct xattr_handler *f2fs_xattr_handlers[] = {
&f2fs_xattr_user_handler,
-#ifdef CONFIG_F2FS_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&f2fs_xattr_trusted_handler,
#ifdef CONFIG_F2FS_FS_SECURITY
&f2fs_xattr_security_handler,
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 518c0677e12a..88c78dc526fa 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -1501,8 +1501,6 @@ const struct xattr_handler *gfs2_xattr_handlers_max[] = {
/* GFS2_FS_FORMAT_MIN */
&gfs2_xattr_user_handler,
&gfs2_xattr_security_handler,
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
NULL,
};
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index da3e18503c65..0eaec4a0f3b1 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -919,10 +919,6 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
&jffs2_user_xattr_handler,
#ifdef CONFIG_JFFS2_FS_SECURITY
&jffs2_security_xattr_handler,
-#endif
-#ifdef CONFIG_JFFS2_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
#endif
&jffs2_trusted_xattr_handler,
NULL
diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
index f9273f6901c8..dfdc0c1f6e25 100644
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -986,10 +986,6 @@ static const struct xattr_handler jfs_trusted_xattr_handler = {
};
const struct xattr_handler *jfs_xattr_handlers[] = {
-#ifdef CONFIG_JFS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&jfs_os2_xattr_handler,
&jfs_user_xattr_handler,
&jfs_security_xattr_handler,
diff --git a/fs/nfs/nfs3_fs.h b/fs/nfs/nfs3_fs.h
index df9ca56db347..a6d1314dbe56 100644
--- a/fs/nfs/nfs3_fs.h
+++ b/fs/nfs/nfs3_fs.h
@@ -17,7 +17,6 @@ extern int nfs3_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry
extern int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
struct posix_acl *dfacl);
extern ssize_t nfs3_listxattr(struct dentry *, char *, size_t);
-extern const struct xattr_handler *nfs3_xattr_handlers[];
#else
static inline int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
struct posix_acl *dfacl)
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 74d11e3c4205..aeb158e3bd99 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -300,12 +300,6 @@ int nfs3_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
goto out;
}
-const struct xattr_handler *nfs3_xattr_handlers[] = {
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
- NULL,
-};
-
static int
nfs3_list_one_acl(struct inode *inode, int type, const char *name, void *data,
size_t size, ssize_t *result)
diff --git a/fs/nfs/nfs3super.c b/fs/nfs/nfs3super.c
index 7c5809431e61..8a9be9e47f76 100644
--- a/fs/nfs/nfs3super.c
+++ b/fs/nfs/nfs3super.c
@@ -14,9 +14,6 @@ struct nfs_subversion nfs_v3 = {
.rpc_vers = &nfs_version3,
.rpc_ops = &nfs_v3_clientops,
.sops = &nfs_sops,
-#ifdef CONFIG_NFS_V3_ACL
- .xattr = nfs3_xattr_handlers,
-#endif
};
static int __init init_nfs_v3(void)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 616df209feea..1eb9f3d9ba8c 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -1033,10 +1033,6 @@ static const struct xattr_handler ntfs_other_xattr_handler = {
};
const struct xattr_handler *ntfs_xattr_handlers[] = {
-#ifdef CONFIG_NTFS3_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&ntfs_other_xattr_handler,
NULL,
};
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 95d0611c5fc7..482b2ef7ca54 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -89,8 +89,6 @@ static struct ocfs2_xattr_def_value_root def_xv = {
const struct xattr_handler *ocfs2_xattr_handlers[] = {
&ocfs2_xattr_user_handler,
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
&ocfs2_xattr_trusted_handler,
&ocfs2_xattr_security_handler,
NULL
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 9a5b757fbd2f..3203abc89b9f 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -555,8 +555,6 @@ static const struct xattr_handler orangefs_xattr_default_handler = {
};
const struct xattr_handler *orangefs_xattr_handlers[] = {
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
&orangefs_xattr_default_handler,
NULL
};
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..559d416e06a3 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1055,20 +1055,12 @@ static const struct xattr_handler ovl_other_xattr_handler = {
};
static const struct xattr_handler *ovl_trusted_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&ovl_own_trusted_xattr_handler,
&ovl_other_xattr_handler,
NULL
};
static const struct xattr_handler *ovl_user_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&ovl_own_user_xattr_handler,
&ovl_other_xattr_handler,
NULL
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 10aa1fd39d2b..379e1dc97225 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -179,10 +179,6 @@ const struct xattr_handler *xfs_xattr_handlers[] = {
&xfs_xattr_user_handler,
&xfs_xattr_trusted_handler,
&xfs_xattr_security_handler,
-#ifdef CONFIG_XFS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
NULL
};
diff --git a/mm/shmem.c b/mm/shmem.c
index 0005ab2c29af..06b91b524dfc 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3331,10 +3331,6 @@ static const struct xattr_handler shmem_trusted_xattr_handler = {
};
static const struct xattr_handler *shmem_xattr_handlers[] = {
-#ifdef CONFIG_TMPFS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
-#endif
&shmem_security_xattr_handler,
&shmem_trusted_xattr_handler,
NULL
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 05/10] fs: simplify ->listxattr() implementation
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (3 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 04/10] fs: drop unused posix acl handlers Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 06/10] reiserfs: rework " Christian Brauner
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd
The ext{2,4}, erofs, f2fs, and jffs2 filesystems use the same logic to
check whether a given xattr can be listed. Simplify them and avoid
open-coding the same check by calling the helper we introduced earlier.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-erofs@lists.ozlabs.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
- Rework this patch completey by keeping the legacy generic POSIX ACL
handlers around so that array-based handler indexing still works.
This means way less churn for filesystems.
---
fs/erofs/xattr.c | 8 ++------
fs/erofs/xattr.h | 14 +++++++++++---
fs/ext2/xattr.c | 17 ++++++++++-------
fs/ext4/xattr.c | 17 ++++++++++-------
fs/f2fs/xattr.c | 16 ++++++++++------
fs/jffs2/xattr.c | 21 ++++++++++++---------
6 files changed, 55 insertions(+), 38 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 2c98a15a92ed..4b73be57c9f4 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -492,13 +492,9 @@ static int xattr_entrylist(struct xattr_iter *_it,
unsigned int prefix_len;
const char *prefix;
- const struct xattr_handler *h =
- erofs_xattr_handler(entry->e_name_index);
-
- if (!h || (h->list && !h->list(it->dentry)))
+ prefix = erofs_xattr_prefix(entry->e_name_index, it->dentry);
+ if (!prefix)
return 1;
-
- prefix = xattr_prefix(h);
prefix_len = strlen(prefix);
if (!it->buffer) {
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 0a43c9ee9f8f..08658e414c33 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,8 +41,11 @@ extern const struct xattr_handler erofs_xattr_user_handler;
extern const struct xattr_handler erofs_xattr_trusted_handler;
extern const struct xattr_handler erofs_xattr_security_handler;
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+static inline const char *erofs_xattr_prefix(unsigned int idx,
+ struct dentry *dentry)
{
+ const struct xattr_handler *handler = NULL;
+
static const struct xattr_handler *xattr_handler_map[] = {
[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
#ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -57,8 +60,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
#endif
};
- return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
- xattr_handler_map[idx] : NULL;
+ if (idx && idx < ARRAY_SIZE(xattr_handler_map))
+ handler = xattr_handler_map[idx];
+
+ if (!xattr_handler_can_list(handler, dentry))
+ return NULL;
+
+ return xattr_prefix(handler);
}
extern const struct xattr_handler *erofs_xattr_handlers[];
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 262951ffe8d0..958976f809f5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,14 +121,18 @@ 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 inline const char *ext2_xattr_prefix(int name_index,
+ struct dentry *dentry)
{
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;
+
+ if (!xattr_handler_can_list(handler, dentry))
+ return NULL;
+
+ return xattr_prefix(handler);
}
static bool
@@ -329,11 +333,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;
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ba7f2557adb8..3fbeeb00fd78 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -169,14 +169,18 @@ static void ext4_xattr_block_csum_set(struct inode *inode,
bh->b_blocknr, BHDR(bh));
}
-static inline const struct xattr_handler *
-ext4_xattr_handler(int name_index)
+static inline const char *ext4_xattr_prefix(int name_index,
+ struct dentry *dentry)
{
const struct xattr_handler *handler = NULL;
if (name_index > 0 && name_index < ARRAY_SIZE(ext4_xattr_handler_map))
handler = ext4_xattr_handler_map[name_index];
- return handler;
+
+ if (!xattr_handler_can_list(handler, dentry))
+ return NULL;
+
+ return xattr_prefix(handler);
}
static int
@@ -691,11 +695,10 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
size_t rest = buffer_size;
for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
- const struct xattr_handler *handler =
- ext4_xattr_handler(entry->e_name_index);
+ const char *prefix;
- if (handler && (!handler->list || handler->list(dentry))) {
- const char *prefix = handler->prefix ?: handler->name;
+ prefix = ext4_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;
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ccb564e328af..9de984645253 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -212,13 +212,18 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
NULL,
};
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static inline const char *f2fs_xattr_prefix(int index,
+ struct dentry *dentry)
{
const struct xattr_handler *handler = NULL;
if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
handler = f2fs_xattr_handler_map[index];
- return handler;
+
+ if (!xattr_handler_can_list(handler, dentry))
+ return NULL;
+
+ return xattr_prefix(handler);
}
static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -569,12 +574,12 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
last_base_addr = (void *)base_addr + XATTR_SIZE(inode);
list_for_each_xattr(entry, base_addr) {
- const struct xattr_handler *handler =
- f2fs_xattr_handler(entry->e_name_index);
const char *prefix;
size_t prefix_len;
size_t size;
+ prefix = f2fs_xattr_prefix(entry->e_name_index, dentry);
+
if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
f2fs_err(F2FS_I_SB(inode), "inode (%lu) has corrupted xattr",
@@ -586,10 +591,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
goto cleanup;
}
- if (!handler || (handler->list && !handler->list(dentry)))
+ if (!prefix)
continue;
- prefix = xattr_prefix(handler);
prefix_len = strlen(prefix);
size = prefix_len + entry->e_name_len + 1;
if (buffer) {
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 0eaec4a0f3b1..1189a70d2007 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -924,8 +924,9 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
NULL
};
-static const struct xattr_handler *xprefix_to_handler(int xprefix) {
- const struct xattr_handler *ret;
+static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
+{
+ const struct xattr_handler *ret = NULL;
switch (xprefix) {
case JFFS2_XPREFIX_USER:
@@ -948,10 +949,13 @@ static const struct xattr_handler *xprefix_to_handler(int xprefix) {
ret = &jffs2_trusted_xattr_handler;
break;
default:
- ret = NULL;
- break;
+ return NULL;
}
- return ret;
+
+ if (!xattr_handler_can_list(ret, dentry))
+ return NULL;
+
+ return xattr_prefix(ret);
}
ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -962,7 +966,6 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
struct jffs2_inode_cache *ic = f->inocache;
struct jffs2_xattr_ref *ref, **pref;
struct jffs2_xattr_datum *xd;
- const struct xattr_handler *xhandle;
const char *prefix;
ssize_t prefix_len, len, rc;
int retry = 0;
@@ -994,10 +997,10 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
goto out;
}
}
- xhandle = xprefix_to_handler(xd->xprefix);
- if (!xhandle || (xhandle->list && !xhandle->list(dentry)))
+
+ prefix = jffs2_xattr_prefix(xd->xprefix, dentry);
+ if (!prefix)
continue;
- prefix = xhandle->prefix ?: xhandle->name;
prefix_len = strlen(prefix);
rc = prefix_len + xd->name_len + 1;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 06/10] reiserfs: rework ->listxattr() implementation
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (4 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 05/10] fs: simplify ->listxattr() implementation Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 07/10] fs: rename generic posix acl handlers Christian Brauner
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
reiserfs-devel
Rework reiserfs so it doesn't have to rely on the dummy xattr handlers
in its s_xattr list anymore as this is completely unused for setting and
getting posix acls.
Cc: reiserfs-devel@vger.kernel.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Rework patch to account for reiserfs quirks.
---
fs/reiserfs/xattr.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 8b2d52443f41..0b949dc45484 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -52,6 +52,7 @@
#include <linux/quotaops.h>
#include <linux/security.h>
#include <linux/posix_acl_xattr.h>
+#include <linux/xattr.h>
#define PRIVROOT_NAME ".reiserfs_priv"
#define XAROOT_NAME "xattrs"
@@ -770,23 +771,34 @@ reiserfs_xattr_get(struct inode *inode, const char *name, void *buffer,
(handler) != NULL; \
(handler) = *(handlers)++)
+static inline bool reiserfs_posix_acl_list(const char *name,
+ struct dentry *dentry)
+{
+ return (posix_acl_type(name) >= 0) &&
+ IS_POSIXACL(d_backing_inode(dentry));
+}
+
/* This is the implementation for the xattr plugin infrastructure */
-static inline const struct xattr_handler *
-find_xattr_handler_prefix(const struct xattr_handler **handlers,
- const char *name)
+static inline bool reiserfs_xattr_list(const struct xattr_handler **handlers,
+ const char *name, struct dentry *dentry)
{
- const struct xattr_handler *xah;
+ if (handlers) {
+ const struct xattr_handler *xah = NULL;
- if (!handlers)
- return NULL;
+ for_each_xattr_handler(handlers, xah) {
+ const char *prefix = xattr_prefix(xah);
- for_each_xattr_handler(handlers, xah) {
- const char *prefix = xattr_prefix(xah);
- if (strncmp(prefix, name, strlen(prefix)) == 0)
- break;
+ if (strncmp(prefix, name, strlen(prefix)))
+ continue;
+
+ if (!xattr_handler_can_list(xah, dentry))
+ return false;
+
+ return true;
+ }
}
- return xah;
+ return reiserfs_posix_acl_list(name, dentry);
}
struct listxattr_buf {
@@ -807,12 +819,8 @@ static bool listxattr_filler(struct dir_context *ctx, const char *name,
if (name[0] != '.' ||
(namelen != 1 && (name[1] != '.' || namelen != 2))) {
- const struct xattr_handler *handler;
-
- handler = find_xattr_handler_prefix(b->dentry->d_sb->s_xattr,
- name);
- if (!handler /* Unsupported xattr name */ ||
- (handler->list && !handler->list(b->dentry)))
+ if (!reiserfs_xattr_list(b->dentry->d_sb->s_xattr, name,
+ b->dentry))
return true;
size = namelen + 1;
if (b->buf) {
@@ -910,10 +918,6 @@ const struct xattr_handler *reiserfs_xattr_handlers[] = {
#endif
#ifdef CONFIG_REISERFS_FS_SECURITY
&reiserfs_xattr_security_handler,
-#endif
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
- &posix_acl_access_xattr_handler,
- &posix_acl_default_xattr_handler,
#endif
NULL
};
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 07/10] fs: rename generic posix acl handlers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (5 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 06/10] reiserfs: rework " Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:14 ` [PATCH v3 08/10] reiserfs: rework priv inode handling Christian Brauner
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
Reflect in their naming and document that they are kept around for
legacy reasons and shouldn't be used anymore by new code.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch unchanged.
Changes in v2:
- Patch introduced.
---
fs/erofs/xattr.h | 6 ++----
fs/ext2/xattr.c | 4 ++--
fs/ext4/xattr.c | 4 ++--
fs/f2fs/xattr.c | 4 ++--
fs/jffs2/xattr.c | 4 ++--
fs/ocfs2/xattr.c | 12 +++++-------
fs/posix_acl.c | 24 ++++++++++++++++++------
include/linux/posix_acl_xattr.h | 5 +++--
8 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 08658e414c33..97185cb649b6 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -49,10 +49,8 @@ static inline const char *erofs_xattr_prefix(unsigned int idx,
static const struct xattr_handler *xattr_handler_map[] = {
[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
#ifdef CONFIG_EROFS_FS_POSIX_ACL
- [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
- &posix_acl_access_xattr_handler,
- [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
- &posix_acl_default_xattr_handler,
+ [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+ [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
#endif
[EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
#ifdef CONFIG_EROFS_FS_SECURITY
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 958976f809f5..b126af5f8b15 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -101,8 +101,8 @@ static void ext2_xattr_rehash(struct ext2_xattr_header *,
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,
+ [EXT2_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+ [EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
#endif
[EXT2_XATTR_INDEX_TRUSTED] = &ext2_xattr_trusted_handler,
#ifdef CONFIG_EXT2_FS_SECURITY
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 3fbeeb00fd78..edca79a62bd9 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -88,8 +88,8 @@ static void ext4_xattr_rehash(struct ext4_xattr_header *);
static const struct xattr_handler * const ext4_xattr_handler_map[] = {
[EXT4_XATTR_INDEX_USER] = &ext4_xattr_user_handler,
#ifdef CONFIG_EXT4_FS_POSIX_ACL
- [EXT4_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler,
- [EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
+ [EXT4_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+ [EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
#endif
[EXT4_XATTR_INDEX_TRUSTED] = &ext4_xattr_trusted_handler,
#ifdef CONFIG_EXT4_FS_SECURITY
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 9de984645253..7eb9628478c8 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -192,8 +192,8 @@ const struct xattr_handler f2fs_xattr_security_handler = {
static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
#ifdef CONFIG_F2FS_FS_POSIX_ACL
- [F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler,
- [F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
+ [F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+ [F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
#endif
[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
#ifdef CONFIG_F2FS_FS_SECURITY
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 1189a70d2007..aa4048a27f31 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -939,10 +939,10 @@ static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
#endif
#ifdef CONFIG_JFFS2_FS_POSIX_ACL
case JFFS2_XPREFIX_ACL_ACCESS:
- ret = &posix_acl_access_xattr_handler;
+ ret = &nop_posix_acl_access;
break;
case JFFS2_XPREFIX_ACL_DEFAULT:
- ret = &posix_acl_default_xattr_handler;
+ ret = &nop_posix_acl_default;
break;
#endif
case JFFS2_XPREFIX_TRUSTED:
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 482b2ef7ca54..ff85e418d7e3 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -95,13 +95,11 @@ const struct xattr_handler *ocfs2_xattr_handlers[] = {
};
static const struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
- [OCFS2_XATTR_INDEX_USER] = &ocfs2_xattr_user_handler,
- [OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]
- = &posix_acl_access_xattr_handler,
- [OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]
- = &posix_acl_default_xattr_handler,
- [OCFS2_XATTR_INDEX_TRUSTED] = &ocfs2_xattr_trusted_handler,
- [OCFS2_XATTR_INDEX_SECURITY] = &ocfs2_xattr_security_handler,
+ [OCFS2_XATTR_INDEX_USER] = &ocfs2_xattr_user_handler,
+ [OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+ [OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
+ [OCFS2_XATTR_INDEX_TRUSTED] = &ocfs2_xattr_trusted_handler,
+ [OCFS2_XATTR_INDEX_SECURITY] = &ocfs2_xattr_security_handler,
};
struct ocfs2_xattr_info {
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index c0886dc8e714..7a4d89897c37 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -989,19 +989,31 @@ posix_acl_xattr_list(struct dentry *dentry)
return IS_POSIXACL(d_backing_inode(dentry));
}
-const struct xattr_handler posix_acl_access_xattr_handler = {
+/*
+ * nop_posix_acl_access - legacy xattr handler for access POSIX ACLs
+ *
+ * This is the legacy POSIX ACL access xattr handler. It is used by some
+ * filesystems to implement their ->listxattr() inode operation. New code
+ * should never use them.
+ */
+const struct xattr_handler nop_posix_acl_access = {
.name = XATTR_NAME_POSIX_ACL_ACCESS,
- .flags = ACL_TYPE_ACCESS,
.list = posix_acl_xattr_list,
};
-EXPORT_SYMBOL_GPL(posix_acl_access_xattr_handler);
+EXPORT_SYMBOL_GPL(nop_posix_acl_access);
-const struct xattr_handler posix_acl_default_xattr_handler = {
+/*
+ * nop_posix_acl_default - legacy xattr handler for default POSIX ACLs
+ *
+ * This is the legacy POSIX ACL default xattr handler. It is used by some
+ * filesystems to implement their ->listxattr() inode operation. New code
+ * should never use them.
+ */
+const struct xattr_handler nop_posix_acl_default = {
.name = XATTR_NAME_POSIX_ACL_DEFAULT,
- .flags = ACL_TYPE_DEFAULT,
.list = posix_acl_xattr_list,
};
-EXPORT_SYMBOL_GPL(posix_acl_default_xattr_handler);
+EXPORT_SYMBOL_GPL(nop_posix_acl_default);
int simple_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
struct posix_acl *acl, int type)
diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
index 54cd7a14330d..e86f3b731da2 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -68,7 +68,8 @@ static inline int posix_acl_type(const char *name)
return -1;
}
-extern const struct xattr_handler posix_acl_access_xattr_handler;
-extern const struct xattr_handler posix_acl_default_xattr_handler;
+/* These are legacy handlers. Don't use them for new code. */
+extern const struct xattr_handler nop_posix_acl_access;
+extern const struct xattr_handler nop_posix_acl_default;
#endif /* _POSIX_ACL_XATTR_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 08/10] reiserfs: rework priv inode handling
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (6 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 07/10] fs: rename generic posix acl handlers Christian Brauner
@ 2023-02-01 13:14 ` Christian Brauner
2023-02-01 13:15 ` [PATCH v3 09/10] ovl: check for ->listxattr() support Christian Brauner
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:14 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
reiserfs-devel
Reiserfs is the only filesystem that removes IOP_XATTR without also
using a set of dedicated inode operations at the same time that nop all
xattr related inode operations. This means we need to have a IOP_XATTR
check in vfs_listxattr() instead of just being able to check for
->listxatt() being implemented.
Introduce a dedicated set of nop inode operations that are used when
IOP_XATTR is removed, allowing us to remove that check from
vfs_listxattr(). This in turn allows us to completely decouple POSIX ACLs from
IOP_XATTR.
Cc: reiserfs-devel@vger.kernel.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch introduced.
---
fs/reiserfs/file.c | 7 +++++++
fs/reiserfs/inode.c | 6 ++----
fs/reiserfs/namei.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
fs/reiserfs/reiserfs.h | 2 ++
fs/reiserfs/xattr.c | 9 +++------
5 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c
index 467d13da198f..b54cc7048f02 100644
--- a/fs/reiserfs/file.c
+++ b/fs/reiserfs/file.c
@@ -261,3 +261,10 @@ const struct inode_operations reiserfs_file_inode_operations = {
.fileattr_get = reiserfs_fileattr_get,
.fileattr_set = reiserfs_fileattr_set,
};
+
+const struct inode_operations reiserfs_priv_file_inode_operations = {
+ .setattr = reiserfs_setattr,
+ .permission = reiserfs_permission,
+ .fileattr_get = reiserfs_fileattr_get,
+ .fileattr_set = reiserfs_fileattr_set,
+};
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index c7d1fa526dea..4ec357919588 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -2087,10 +2087,8 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
* Mark it private if we're creating the privroot
* or something under it.
*/
- if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
- inode->i_flags |= S_PRIVATE;
- inode->i_opflags &= ~IOP_XATTR;
- }
+ if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root)
+ reiserfs_init_priv_inode(inode);
if (reiserfs_posixacl(inode->i_sb)) {
reiserfs_write_unlock(inode->i_sb);
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 0b8aa99749f1..2f0c721c8ac9 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -378,13 +378,11 @@ static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry,
/*
* Propagate the private flag so we know we're
- * in the priv tree. Also clear IOP_XATTR
+ * in the priv tree. Also clear xattr support
* since we don't have xattrs on xattr files.
*/
- if (IS_PRIVATE(dir)) {
- inode->i_flags |= S_PRIVATE;
- inode->i_opflags &= ~IOP_XATTR;
- }
+ if (IS_PRIVATE(dir))
+ reiserfs_init_priv_inode(inode);
}
reiserfs_write_unlock(dir->i_sb);
if (retval == IO_ERROR) {
@@ -1649,6 +1647,48 @@ static int reiserfs_rename(struct user_namespace *mnt_userns,
return retval;
}
+static const struct inode_operations reiserfs_priv_dir_inode_operations = {
+ .create = reiserfs_create,
+ .lookup = reiserfs_lookup,
+ .link = reiserfs_link,
+ .unlink = reiserfs_unlink,
+ .symlink = reiserfs_symlink,
+ .mkdir = reiserfs_mkdir,
+ .rmdir = reiserfs_rmdir,
+ .mknod = reiserfs_mknod,
+ .rename = reiserfs_rename,
+ .setattr = reiserfs_setattr,
+ .permission = reiserfs_permission,
+ .fileattr_get = reiserfs_fileattr_get,
+ .fileattr_set = reiserfs_fileattr_set,
+};
+
+static const struct inode_operations reiserfs_priv_symlink_inode_operations = {
+ .get_link = page_get_link,
+ .setattr = reiserfs_setattr,
+ .permission = reiserfs_permission,
+};
+
+static const struct inode_operations reiserfs_priv_special_inode_operations = {
+ .setattr = reiserfs_setattr,
+ .permission = reiserfs_permission,
+};
+
+void reiserfs_init_priv_inode(struct inode *inode)
+{
+ inode->i_flags |= S_PRIVATE;
+ inode->i_opflags &= ~IOP_XATTR;
+
+ if (S_ISREG(inode->i_mode))
+ inode->i_op = &reiserfs_priv_file_inode_operations;
+ else if (S_ISDIR(inode->i_mode))
+ inode->i_op = &reiserfs_priv_dir_inode_operations;
+ else if (S_ISLNK(inode->i_mode))
+ inode->i_op = &reiserfs_priv_symlink_inode_operations;
+ else
+ inode->i_op = &reiserfs_priv_special_inode_operations;
+}
+
/* directories can handle most operations... */
const struct inode_operations reiserfs_dir_inode_operations = {
.create = reiserfs_create,
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index 3aa928ec527a..d4dd39a66d80 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -3106,6 +3106,7 @@ int reiserfs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
int __reiserfs_write_begin(struct page *page, unsigned from, unsigned len);
/* namei.c */
+void reiserfs_init_priv_inode(struct inode *inode);
void set_de_name_and_namelen(struct reiserfs_dir_entry *de);
int search_by_entry_key(struct super_block *sb, const struct cpu_key *key,
struct treepath *path, struct reiserfs_dir_entry *de);
@@ -3175,6 +3176,7 @@ void reiserfs_unmap_buffer(struct buffer_head *);
/* file.c */
extern const struct inode_operations reiserfs_file_inode_operations;
+extern const struct inode_operations reiserfs_priv_file_inode_operations;
extern const struct file_operations reiserfs_file_operations;
extern const struct address_space_operations reiserfs_address_space_operations;
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 0b949dc45484..11b32bbd656d 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -896,8 +896,7 @@ static int create_privroot(struct dentry *dentry)
return -EOPNOTSUPP;
}
- d_inode(dentry)->i_flags |= S_PRIVATE;
- d_inode(dentry)->i_opflags &= ~IOP_XATTR;
+ reiserfs_init_priv_inode(d_inode(dentry));
reiserfs_info(dentry->d_sb, "Created %s - reserved for xattr "
"storage.\n", PRIVROOT_NAME);
@@ -979,10 +978,8 @@ int reiserfs_lookup_privroot(struct super_block *s)
if (!IS_ERR(dentry)) {
REISERFS_SB(s)->priv_root = dentry;
d_set_d_op(dentry, &xattr_lookup_poison_ops);
- if (d_really_is_positive(dentry)) {
- d_inode(dentry)->i_flags |= S_PRIVATE;
- d_inode(dentry)->i_opflags &= ~IOP_XATTR;
- }
+ if (d_really_is_positive(dentry))
+ reiserfs_init_priv_inode(d_inode(dentry));
} else
err = PTR_ERR(dentry);
inode_unlock(d_inode(s->s_root));
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 09/10] ovl: check for ->listxattr() support
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (7 preceding siblings ...)
2023-02-01 13:14 ` [PATCH v3 08/10] reiserfs: rework priv inode handling Christian Brauner
@ 2023-02-01 13:15 ` Christian Brauner
2023-02-01 13:15 ` [PATCH v3 10/10] acl: don't depend on IOP_XATTR Christian Brauner
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:15 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
linux-unionfs
We have decoupled vfs_listxattr() from IOP_XATTR. Instead we just need
to check whether inode->i_op->listxattr is implemented.
Cc: linux-unionfs@vger.kernel.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch introduced.
---
fs/overlayfs/copy_up.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index c14e90764e35..f658cc8ea492 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -81,8 +81,7 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct de
int error = 0;
size_t slen;
- if (!(old->d_inode->i_opflags & IOP_XATTR) ||
- !(new->d_inode->i_opflags & IOP_XATTR))
+ if (!old->d_inode->i_op->listxattr || !new->d_inode->i_op->listxattr)
return 0;
list_size = vfs_listxattr(old, NULL, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 10/10] acl: don't depend on IOP_XATTR
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (8 preceding siblings ...)
2023-02-01 13:15 ` [PATCH v3 09/10] ovl: check for ->listxattr() support Christian Brauner
@ 2023-02-01 13:15 ` Christian Brauner
2023-02-01 13:30 ` [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christoph Hellwig
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:15 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig
Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)
All codepaths that don't want to implement POSIX ACLs should simply not
implement the associated inode operations instead of relying on
IOP_XATTR. That's the case for all filesystems today.
For vfs_listxattr() all filesystems that explicitly turn of xattrs for a
given inode all set inode->i_op to a dedicated set of inode operations
that doesn't implement ->listxattr(). We can remove the dependency of
vfs_listxattr() on IOP_XATTR.
Removing this dependency will allow us to decouple POSIX ACLs from
IOP_XATTR and they can still be listed even if no other xattr handlers
are implemented. Otherwise we would have to implement elaborate schemes
to raise IOP_XATTR even if sb->s_xattr is set to NULL.
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v3:
- Patch introduced.
---
fs/posix_acl.c | 12 ++++--------
fs/xattr.c | 25 ++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 7a4d89897c37..881a7fd1cacb 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1132,12 +1132,10 @@ int vfs_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
if (error)
goto out_inode_unlock;
- if (inode->i_opflags & IOP_XATTR)
+ if (likely(!is_bad_inode(inode)))
error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
- else if (unlikely(is_bad_inode(inode)))
- error = -EIO;
else
- error = -EOPNOTSUPP;
+ error = -EIO;
if (!error) {
fsnotify_xattr(dentry);
evm_inode_post_set_acl(dentry, acl_name, kacl);
@@ -1242,12 +1240,10 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
if (error)
goto out_inode_unlock;
- if (inode->i_opflags & IOP_XATTR)
+ if (likely(!is_bad_inode(inode)))
error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
- else if (unlikely(is_bad_inode(inode)))
- error = -EIO;
else
- error = -EOPNOTSUPP;
+ error = -EIO;
if (!error) {
fsnotify_xattr(dentry);
evm_inode_post_remove_acl(mnt_userns, dentry, acl_name);
diff --git a/fs/xattr.c b/fs/xattr.c
index 8743402a5e8b..56e37461014e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -457,6 +457,28 @@ vfs_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
}
EXPORT_SYMBOL_GPL(vfs_getxattr);
+/**
+ * vfs_listxattr - retrieve \0 separated list of xattr names
+ * @dentry: the dentry from whose inode the xattr names are retrieved
+ * @list: buffer to store xattr names into
+ * @size: size of the buffer
+ *
+ * This function returns the names of all xattrs associated with the
+ * inode of @dentry.
+ *
+ * Note, for legacy reasons the vfs_listxattr() function lists POSIX
+ * ACLs as well. Since POSIX ACLs are decoupled from IOP_XATTR the
+ * vfs_listxattr() function doesn't check for this flag since a
+ * filesystem could implement POSIX ACLs without implementing any other
+ * xattrs.
+ *
+ * However, since all codepaths that remove IOP_XATTR also assign of
+ * inode operations that either don't implement or implement a stub
+ * ->listxattr() operation.
+ *
+ * Return: On success, the size of the buffer that was used. On error a
+ * negative error code.
+ */
ssize_t
vfs_listxattr(struct dentry *dentry, char *list, size_t size)
{
@@ -466,7 +488,8 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
error = security_inode_listxattr(dentry);
if (error)
return error;
- if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
+
+ if (inode->i_op->listxattr) {
error = inode->i_op->listxattr(dentry, list, size);
} else {
error = security_inode_listsecurity(inode, list, size);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (9 preceding siblings ...)
2023-02-01 13:15 ` [PATCH v3 10/10] acl: don't depend on IOP_XATTR Christian Brauner
@ 2023-02-01 13:30 ` Christoph Hellwig
2023-02-01 13:42 ` Christian Brauner
2023-03-06 9:23 ` Christian Brauner
2023-04-26 23:07 ` [f2fs-dev] " patchwork-bot+f2fs
12 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2023-02-01 13:30 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee,
linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd,
reiserfs-devel, linux-unionfs
This version looks good to me, but I'd really prefer if a reiserfs
insider could look over the reiserfs patches.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
2023-02-01 13:30 ` [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christoph Hellwig
@ 2023-02-01 13:42 ` Christian Brauner
2023-03-06 9:26 ` Christian Brauner
0 siblings, 1 reply; 16+ messages in thread
From: Christian Brauner @ 2023-02-01 13:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-fsdevel, Al Viro, Seth Forshee, linux-f2fs-devel,
linux-erofs, linux-ext4, linux-mtd, reiserfs-devel, linux-unionfs
On Wed, Feb 01, 2023 at 02:30:20PM +0100, Christoph Hellwig wrote:
> This version looks good to me, but I'd really prefer if a reiserfs
> insider could look over the reiserfs patches.
I consider this material for v6.4 even with an -rc8 for v6.3. So there's
time but we shouldn't block it on reiserfs. Especially, since it's
marked deprecated.
Fwiw, I've tested reiserfs with xfstests on a kernel with and without
this series applied and there's no regressions. But it's overall pretty
buggy at least according to xfstests. Which is expected, I guess.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (10 preceding siblings ...)
2023-02-01 13:30 ` [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christoph Hellwig
@ 2023-03-06 9:23 ` Christian Brauner
2023-04-26 23:07 ` [f2fs-dev] " patchwork-bot+f2fs
12 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-03-06 9:23 UTC (permalink / raw)
To: linux-fsdevel, Christoph Hellwig, Christian Brauner
Cc: Al Viro, Seth Forshee, linux-f2fs-devel, linux-erofs, linux-ext4,
linux-mtd, reiserfs-devel, linux-unionfs
From: Christian Brauner (Microsoft) <brauner@kernel.org>
On Wed, 01 Feb 2023 14:14:51 +0100, Christian Brauner wrote:
> Hey everyone,
>
> After we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handlers for two reasons:
>
> (1) Because a few filesystems rely on the ->list() method of the generic
> POSIX ACL xattr handlers in their ->listxattr() inode operation.
> (2) POSIX ACLs are only available if IOP_XATTR is raised. The IOP_XATTR
> flag is raised in inode_init_always() based on whether the
> sb->s_xattr pointer is non-NULL. IOW, the registered xattr handlers
> of the filesystem are used to raise IOP_XATTR.
> If we were to remove the generic POSIX ACL xattr handlers from all
> filesystems we would risk regressing filesystems that only implement
> POSIX ACL support and no other xattrs (nfs3 comes to mind).
>
> [...]
With v6.3-rc1 out, I've applied this now. Please keep an eye out for any
regressions as this has been a delicate undertaking:
[01/10] xattr: simplify listxattr helpers
commit: f2620f166e2a4db08f016b7b30b904ab28c265e4
[02/10] xattr: add listxattr helper
commit: 2db8a948046cab3a2f707561592906a3d096972f
[03/10] xattr: remove unused argument
commit: 831be973aa21d1cf8948bf4b1d4e73e6d5d028c0
[04/10] fs: drop unused posix acl handlers
commit: 0c95c025a02e477b2d112350e1c78bb0cc994c51
[05/10] fs: simplify ->listxattr() implementation
commit: a5488f29835c0eb5561b46e71c23f6c39aab6c83
[06/10] reiserfs: rework ->listxattr() implementation
commit: 387b96a5891c075986afbf13e84cba357710068e
[07/10] fs: rename generic posix acl handlers
commit: d549b741740e63e87e661754e2d1b336fdc51d50
[08/10] reiserfs: rework priv inode handling
commit: d9f892b9bdc22b12bc960837a09f014d5a324975
[09/10] ovl: check for ->listxattr() support
commit: a1fbb607340d49f208e90cc0d7bdfff2141cce8d
[10/10] acl: don't depend on IOP_XATTR
commit: e499214ce3ef50c50522719e753a1ffc928c2ec1
Christian
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
2023-02-01 13:42 ` Christian Brauner
@ 2023-03-06 9:26 ` Christian Brauner
0 siblings, 0 replies; 16+ messages in thread
From: Christian Brauner @ 2023-03-06 9:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-fsdevel, Al Viro, Seth Forshee, linux-f2fs-devel,
linux-erofs, linux-ext4, linux-mtd, reiserfs-devel, linux-unionfs
On Wed, Feb 01, 2023 at 02:42:54PM +0100, Christian Brauner wrote:
> On Wed, Feb 01, 2023 at 02:30:20PM +0100, Christoph Hellwig wrote:
> > This version looks good to me, but I'd really prefer if a reiserfs
> > insider could look over the reiserfs patches.
>
> I consider this material for v6.4 even with an -rc8 for v6.3. So there's
> time but we shouldn't block it on reiserfs. Especially, since it's
> marked deprecated.
So I've applied this now. If there's still someone interested in
checking the reiserfs bits more than what we did with xfstests they
should please do so. But I don't want to hold up this series waiting for
that to happen.
>
> Fwiw, I've tested reiserfs with xfstests on a kernel with and without
> this series applied and there's no regressions. But it's overall pretty
> buggy at least according to xfstests. Which is expected, I guess.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
` (11 preceding siblings ...)
2023-03-06 9:23 ` Christian Brauner
@ 2023-04-26 23:07 ` patchwork-bot+f2fs
12 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+f2fs @ 2023-04-26 23:07 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-fsdevel, hch, linux-unionfs, reiserfs-devel,
linux-f2fs-devel, linux-mtd, viro, linux-ext4, linux-erofs,
sforshee
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Christian Brauner (Microsoft) <brauner@kernel.org>:
On Wed, 01 Feb 2023 14:14:51 +0100 you wrote:
> Hey everyone,
>
> After we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handlers for two reasons:
>
> (1) Because a few filesystems rely on the ->list() method of the generic
> POSIX ACL xattr handlers in their ->listxattr() inode operation.
> (2) POSIX ACLs are only available if IOP_XATTR is raised. The IOP_XATTR
> flag is raised in inode_init_always() based on whether the
> sb->s_xattr pointer is non-NULL. IOW, the registered xattr handlers
> of the filesystem are used to raise IOP_XATTR.
> If we were to remove the generic POSIX ACL xattr handlers from all
> filesystems we would risk regressing filesystems that only implement
> POSIX ACL support and no other xattrs (nfs3 comes to mind).
>
> [...]
Here is the summary with links:
- [f2fs-dev,v3,05/10] fs: simplify ->listxattr() implementation
https://git.kernel.org/jaegeuk/f2fs/c/a5488f29835c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-04-26 23:07 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-01 13:14 [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 01/10] xattr: simplify listxattr helpers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 02/10] xattr: add listxattr helper Christian Brauner
2023-02-01 13:14 ` [PATCH v3 03/10] xattr: remove unused argument Christian Brauner
2023-02-01 13:14 ` [PATCH v3 04/10] fs: drop unused posix acl handlers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 05/10] fs: simplify ->listxattr() implementation Christian Brauner
2023-02-01 13:14 ` [PATCH v3 06/10] reiserfs: rework " Christian Brauner
2023-02-01 13:14 ` [PATCH v3 07/10] fs: rename generic posix acl handlers Christian Brauner
2023-02-01 13:14 ` [PATCH v3 08/10] reiserfs: rework priv inode handling Christian Brauner
2023-02-01 13:15 ` [PATCH v3 09/10] ovl: check for ->listxattr() support Christian Brauner
2023-02-01 13:15 ` [PATCH v3 10/10] acl: don't depend on IOP_XATTR Christian Brauner
2023-02-01 13:30 ` [PATCH v3 00/10] acl: drop posix acl handlers from xattr handlers Christoph Hellwig
2023-02-01 13:42 ` Christian Brauner
2023-03-06 9:26 ` Christian Brauner
2023-03-06 9:23 ` Christian Brauner
2023-04-26 23:07 ` [f2fs-dev] " patchwork-bot+f2fs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).