[f2fs-dev] [PATCH 00/12] acl: remove remaining posix acl handlers

[f2fs-dev] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88

[Ocfs2-devel] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner via Ocfs2-devel]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

[PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88

[PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88

[PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handler around for two
reasons. First, because a few filesystems relied on the ->list() method
of the generic POSIX ACL xattr handlers in their ->listxattr() inode
operation. Second, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the removal of the legacy POSIX ACL handlers it is at least
possible for a filesystem to only implement POSIX ACLs but no other
xattrs. If that were to happen we would miss to raise IOP_XATTR because
sb->s_xattr would be NULL.

Fix these things and then get rid of the misleading and effectively
already unused generic POSIX ACL handlers.

For most filesystems it is a trivial removal of the generic POSIX ACL
handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
handler is used but rather easy to fix.

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.

This branch depends on [1] which hopefully should be merged soon and can
be pulled from [2] which already includes [1] so it's easy to test and
compile.

With this all remnants of the old POSIX ACL xattr handling will be gone.

Thanks!
Christian

[1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
[2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Christian Brauner (12):
      xattr: simplify listxattr helpers
      xattr, posix acl: add listxattr helpers
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      erofs: drop posix acl handlers
      ext2: drop posix acl handlers
      ext4: drop posix acl handlers
      f2fs: drop posix acl handlers
      jffs2: drop posix acl handlers
      ocfs2: drop posix acl handlers
      reiserfs: drop posix acl handlers
      acl: remove posix acl handlers

 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                |  49 ++++++++++++----
 fs/erofs/xattr.h                |  21 -------
 fs/ext2/xattr.c                 |  60 +++++++++++--------
 fs/ext4/xattr.c                 |  71 +++++++++++++----------
 fs/f2fs/xattr.c                 |  63 ++++++++++++--------
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  42 +++++++-------
 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                |  41 +++++++------
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/super.c            |   8 ---
 fs/posix_acl.c                  |  20 -------
 fs/reiserfs/xattr.c             |  38 ++++++------
 fs/xattr.c                      | 124 ++++++++++++++++++++--------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/posix_acl_xattr.h |   6 +-
 include/linux/xattr.h           |   8 ++-
 mm/shmem.c                      |   4 --
 28 files changed, 290 insertions(+), 314 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 01/12] xattr: simplify listxattr helpers

[Christian Brauner]

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.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/xattr.c | 114 +++++++++++++++++++++++++++++++------------------------------
 1 file changed, 58 insertions(+), 56 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index adab9a70b536..ed7fcaab8e1a 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,6 +949,46 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 	return error;
 }
 
+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;
+}
+
+static int posix_acl_listxattr(struct inode *inode, char **buffer,
+			       ssize_t *remaining_size)
+{
+#ifdef CONFIG_FS_POSIX_ACL
+	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;
+	}
+#endif
+	return 0;
+}
+
 /*
  * Combine the results of the list() operation from every xattr_handler in the
  * list.
@@ -957,33 +997,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 +1274,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 +1302,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)) {

-- 
2.34.1

Re: [PATCH 01/12] xattr: simplify listxattr helpers

[Christoph Hellwig]

On Wed, Jan 25, 2023 at 12:28:46PM +0100, Christian Brauner wrote:
> +static int xattr_list_one(char **buffer, ssize_t *remaining_size,
> +			  const char *name)
> +{
> +	size_t len = strlen(name) + 1;
> +	if (*buffer) {

Empty line after the variable declaraion(s), please.

> +static int posix_acl_listxattr(struct inode *inode, char **buffer,
> +			       ssize_t *remaining_size)
> +{
> +#ifdef CONFIG_FS_POSIX_ACL

This function should go into posix_acl.c, with a stub in the header
instead of the ifdef.

> +	err = posix_acl_listxattr(d_inode(dentry), &buffer, &remaining_size);
> +	if (err)
> +		return err;

And I suspect the call to posix_acl_listxattr should move into the caller in
the VFS instead of the method, just like ACLs are handled in
the get/set side.

[PATCH 02/12] xattr, posix acl: add listxattr helpers

[Christian Brauner]

Add two tiny helpers to determine whether a given dentry supports
listing the requested xattr or posix acl. We will use this helper in
various filesystems in later commits.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 include/linux/posix_acl_xattr.h | 5 +++++
 include/linux/xattr.h           | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
index 54cd7a14330d..905d532ccd6e 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -68,6 +68,11 @@ static inline int posix_acl_type(const char *name)
 	return -1;
 }
 
+static inline bool posix_acl_dentry_list(struct dentry *dentry)
+{
+	return IS_POSIXACL(d_backing_inode(dentry));
+}
+
 extern const struct xattr_handler posix_acl_access_xattr_handler;
 extern const struct xattr_handler posix_acl_default_xattr_handler;
 
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 2e7dd44926e4..d1150b7ba8d0 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -47,6 +47,12 @@ struct xattr_handler {
 		   size_t size, int flags);
 };
 
+static inline bool xattr_dentry_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

Re: [PATCH 02/12] xattr, posix acl: add listxattr helpers

[Christoph Hellwig]

On Wed, Jan 25, 2023 at 12:28:47PM +0100, Christian Brauner wrote:
> +static inline bool posix_acl_dentry_list(struct dentry *dentry)
> +{
> +	return IS_POSIXACL(d_backing_inode(dentry));
> +}

I find the open coded IS_POSIXACL much easier to read then this.
But if you want this helpers please add a comment on why it exists
and should be used.

> +static inline bool xattr_dentry_list(const struct xattr_handler *handler,
> +				     struct dentry *dentry)
> +{
> +	return handler && (!handler->list || handler->list(dentry));
> +}

This one could also benefit from a comment explaining what it does.
Also the name seems wrong, should be be something like
xattr_handler_can_list?  

[PATCH 03/12] xattr: remove unused argument

[Christian Brauner]

his helpers is really just used to check for user.* xattr support so
don't make it pointlessly generic.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 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 2b4ae858c89b..d78a73c732a3 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 ed7fcaab8e1a..b79f202c5857 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 d1150b7ba8d0..391ade703782 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -84,7 +84,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

Re: [PATCH 03/12] xattr: remove unused argument

[Christoph Hellwig]

On Wed, Jan 25, 2023 at 12:28:48PM +0100, Christian Brauner wrote:
> his helpers is really just used to check for user.* xattr support so
> don't make it pointlessly generic.

In the long run some kind of flag would be better than the handler
iteration, but now this change looks good enough:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 04/12] fs: drop unused posix acl handlers

[Christian Brauner]

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.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 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/gfs2/xattr.c      | 2 --
 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/orangefs/xattr.c  | 2 --
 fs/overlayfs/super.c | 8 --------
 fs/xfs/xfs_xattr.c   | 4 ----
 mm/shmem.c           | 4 ----
 15 files changed, 58 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/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/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/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 c301487be5fb..081a82edf7c0 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3333,10 +3333,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

Re: [PATCH 04/12] fs: drop unused posix acl handlers

[Christoph Hellwig]

On Wed, Jan 25, 2023 at 12:28:49PM +0100, Christian Brauner wrote:
> 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.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 05/12] erofs: drop posix acl handlers

[Christian Brauner]

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: <linux-erofs@lists.ozlabs.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/erofs/xattr.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 fs/erofs/xattr.h | 21 ---------------------
 2 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..a787e74d9a21 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,
@@ -480,6 +476,43 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
 	NULL,
 };
 
+static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
+{
+	const char *name = NULL;
+	const struct xattr_handler *handler = NULL;
+
+	switch (xattr_index) {
+	case EROFS_XATTR_INDEX_USER:
+		handler = &erofs_xattr_user_handler;
+		break;
+	case EROFS_XATTR_INDEX_TRUSTED:
+		handler = &erofs_xattr_trusted_handler;
+		break;
+#ifdef CONFIG_EROFS_FS_SECURITY
+	case EROFS_XATTR_INDEX_SECURITY:
+		handler = &erofs_xattr_security_handler;
+		break;
+#endif
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
+	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case EROFS_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;
+}
+
 struct listxattr_iter {
 	struct xattr_iter it;
 
@@ -496,13 +529,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..9376cbdc32d8 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -40,27 +40,6 @@ static inline unsigned int xattrblock_offset(struct erofs_sb_info *sbi,
 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 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,
-#endif
-		[EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
-#ifdef CONFIG_EROFS_FS_SECURITY
-		[EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
-#endif
-	};
-
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
-}
-
 extern const struct xattr_handler *erofs_xattr_handlers[];
 
 int erofs_getxattr(struct inode *, int, const char *, void *, size_t);

-- 
2.34.1

[PATCH 05/12] erofs: drop posix acl handlers

[Christian Brauner]

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: <linux-erofs@lists.ozlabs.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/erofs/xattr.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 fs/erofs/xattr.h | 21 ---------------------
 2 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..a787e74d9a21 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,
@@ -480,6 +476,43 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
 	NULL,
 };
 
+static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
+{
+	const char *name = NULL;
+	const struct xattr_handler *handler = NULL;
+
+	switch (xattr_index) {
+	case EROFS_XATTR_INDEX_USER:
+		handler = &erofs_xattr_user_handler;
+		break;
+	case EROFS_XATTR_INDEX_TRUSTED:
+		handler = &erofs_xattr_trusted_handler;
+		break;
+#ifdef CONFIG_EROFS_FS_SECURITY
+	case EROFS_XATTR_INDEX_SECURITY:
+		handler = &erofs_xattr_security_handler;
+		break;
+#endif
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
+	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case EROFS_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;
+}
+
 struct listxattr_iter {
 	struct xattr_iter it;
 
@@ -496,13 +529,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..9376cbdc32d8 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -40,27 +40,6 @@ static inline unsigned int xattrblock_offset(struct erofs_sb_info *sbi,
 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 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,
-#endif
-		[EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
-#ifdef CONFIG_EROFS_FS_SECURITY
-		[EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
-#endif
-	};
-
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
-}
-
 extern const struct xattr_handler *erofs_xattr_handlers[];
 
 int erofs_getxattr(struct inode *, int, const char *, void *, size_t);

-- 
2.34.1

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christoph Hellwig]

This review is not for erofs specifically, but for all file systems using
the same scheme.

> +static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
> +{
> +	const char *name = NULL;
> +	const struct xattr_handler *handler = NULL;
> +
> +	switch (xattr_index) {
> +	case EROFS_XATTR_INDEX_USER:
> +		handler = &erofs_xattr_user_handler;
> +		break;
> +	case EROFS_XATTR_INDEX_TRUSTED:
> +		handler = &erofs_xattr_trusted_handler;
> +		break;
> +#ifdef CONFIG_EROFS_FS_SECURITY
> +	case EROFS_XATTR_INDEX_SECURITY:
> +		handler = &erofs_xattr_security_handler;
> +		break;
> +#endif
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> +	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
> +		if (posix_acl_dentry_list(dentry))
> +			name = XATTR_NAME_POSIX_ACL_ACCESS;
> +		break;
> +	case EROFS_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);

I'm not a huge fan of all this duplicate logic in the file systems
that is more verbose and a bit confusing.  Until we remove the
xattr handlers entirely, I wonder if we just need to keep a
special ->list for posix xattrs, just to be able to keep the
old logic in all these file system.  That is a ->list that
works for xattr_dentry_list, but never actually lists anything.

That would remove all this boiler plate for now without minimal
core additions.  Eventually we can hopefully remove first ->list
and then the xattr handlers entirely, but until then this seems
like a step backwards.

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christoph Hellwig]

This review is not for erofs specifically, but for all file systems using
the same scheme.

> +static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
> +{
> +	const char *name = NULL;
> +	const struct xattr_handler *handler = NULL;
> +
> +	switch (xattr_index) {
> +	case EROFS_XATTR_INDEX_USER:
> +		handler = &erofs_xattr_user_handler;
> +		break;
> +	case EROFS_XATTR_INDEX_TRUSTED:
> +		handler = &erofs_xattr_trusted_handler;
> +		break;
> +#ifdef CONFIG_EROFS_FS_SECURITY
> +	case EROFS_XATTR_INDEX_SECURITY:
> +		handler = &erofs_xattr_security_handler;
> +		break;
> +#endif
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> +	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
> +		if (posix_acl_dentry_list(dentry))
> +			name = XATTR_NAME_POSIX_ACL_ACCESS;
> +		break;
> +	case EROFS_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);

I'm not a huge fan of all this duplicate logic in the file systems
that is more verbose and a bit confusing.  Until we remove the
xattr handlers entirely, I wonder if we just need to keep a
special ->list for posix xattrs, just to be able to keep the
old logic in all these file system.  That is a ->list that
works for xattr_dentry_list, but never actually lists anything.

That would remove all this boiler plate for now without minimal
core additions.  Eventually we can hopefully remove first ->list
and then the xattr handlers entirely, but until then this seems
like a step backwards.

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 07:43:29AM +0100, Christoph Hellwig wrote:
> This review is not for erofs specifically, but for all file systems using
> the same scheme.
> 
> > +static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
> > +{
> > +	const char *name = NULL;
> > +	const struct xattr_handler *handler = NULL;
> > +
> > +	switch (xattr_index) {
> > +	case EROFS_XATTR_INDEX_USER:
> > +		handler = &erofs_xattr_user_handler;
> > +		break;
> > +	case EROFS_XATTR_INDEX_TRUSTED:
> > +		handler = &erofs_xattr_trusted_handler;
> > +		break;
> > +#ifdef CONFIG_EROFS_FS_SECURITY
> > +	case EROFS_XATTR_INDEX_SECURITY:
> > +		handler = &erofs_xattr_security_handler;
> > +		break;
> > +#endif
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > +	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
> > +		if (posix_acl_dentry_list(dentry))
> > +			name = XATTR_NAME_POSIX_ACL_ACCESS;
> > +		break;
> > +	case EROFS_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);
> 
> I'm not a huge fan of all this duplicate logic in the file systems
> that is more verbose and a bit confusing.  Until we remove the

Yeah, it hasn't been my favorite part about this either.
But note how the few filesystems that receive that change use the same
logic by indexing an array and retrieving the handler and then clumsily
open-coding the same check that is now moved into xattr_dentry_list().

> xattr handlers entirely, I wonder if we just need to keep a
> special ->list for posix xattrs, just to be able to keep the
> old logic in all these file system.  That is a ->list that
> works for xattr_dentry_list, but never actually lists anything.

If we want the exact same logic to be followed as today then we need to
keep the dummy struct posix_acl_{access,default}_xattr_handler around.
I tried to avoid that for the first version because it felt a bit
disappointing but we can live with this. This way there's zero code changes
required for filesystems that use legacy array-based handler-indexing.

But we should probably still tweak this so that all these filesystems don't
open-code the !h || (h->list && !h->list(dentry) check like they do now. So
something like what I did below at [1]. Thoughts?

[1]:
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..fd83bbc4b98a 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -496,13 +496,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..a94932c4938c 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,7 +41,7 @@ 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)
 {
        static const struct xattr_handler *xattr_handler_map[] = {
                [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
@@ -57,8 +57,11 @@ 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) &&
+           xattr_dentry_list(xattr_handler_map[idx], dentry))
+               return xattr_prefix(handler);
+
+       return NULL;
 }

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 07:43:29AM +0100, Christoph Hellwig wrote:
> This review is not for erofs specifically, but for all file systems using
> the same scheme.
> 
> > +static const char *erofs_xattr_prefix(int xattr_index, struct dentry *dentry)
> > +{
> > +	const char *name = NULL;
> > +	const struct xattr_handler *handler = NULL;
> > +
> > +	switch (xattr_index) {
> > +	case EROFS_XATTR_INDEX_USER:
> > +		handler = &erofs_xattr_user_handler;
> > +		break;
> > +	case EROFS_XATTR_INDEX_TRUSTED:
> > +		handler = &erofs_xattr_trusted_handler;
> > +		break;
> > +#ifdef CONFIG_EROFS_FS_SECURITY
> > +	case EROFS_XATTR_INDEX_SECURITY:
> > +		handler = &erofs_xattr_security_handler;
> > +		break;
> > +#endif
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > +	case EROFS_XATTR_INDEX_POSIX_ACL_ACCESS:
> > +		if (posix_acl_dentry_list(dentry))
> > +			name = XATTR_NAME_POSIX_ACL_ACCESS;
> > +		break;
> > +	case EROFS_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);
> 
> I'm not a huge fan of all this duplicate logic in the file systems
> that is more verbose and a bit confusing.  Until we remove the

Yeah, it hasn't been my favorite part about this either.
But note how the few filesystems that receive that change use the same
logic by indexing an array and retrieving the handler and then clumsily
open-coding the same check that is now moved into xattr_dentry_list().

> xattr handlers entirely, I wonder if we just need to keep a
> special ->list for posix xattrs, just to be able to keep the
> old logic in all these file system.  That is a ->list that
> works for xattr_dentry_list, but never actually lists anything.

If we want the exact same logic to be followed as today then we need to
keep the dummy struct posix_acl_{access,default}_xattr_handler around.
I tried to avoid that for the first version because it felt a bit
disappointing but we can live with this. This way there's zero code changes
required for filesystems that use legacy array-based handler-indexing.

But we should probably still tweak this so that all these filesystems don't
open-code the !h || (h->list && !h->list(dentry) check like they do now. So
something like what I did below at [1]. Thoughts?

[1]:
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..fd83bbc4b98a 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -496,13 +496,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..a94932c4938c 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,7 +41,7 @@ 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)
 {
        static const struct xattr_handler *xattr_handler_map[] = {
                [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
@@ -57,8 +57,11 @@ 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) &&
+           xattr_dentry_list(xattr_handler_map[idx], dentry))
+               return xattr_prefix(handler);
+
+       return NULL;
 }

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:00:08AM +0100, Christian Brauner wrote:
> > > +	if (xattr_dentry_list(handler, dentry))
> > > +		name = xattr_prefix(handler);
> > 
> > I'm not a huge fan of all this duplicate logic in the file systems
> > that is more verbose and a bit confusing.  Until we remove the
> 
> Yeah, it hasn't been my favorite part about this either.
> But note how the few filesystems that receive that change use the same
> logic by indexing an array and retrieving the handler and then clumsily
> open-coding the same check that is now moved into xattr_dentry_list().

At least it allows for an array lookup.  And of course switching
to xattr_dentry_list instead of open coding it is always a good idea.

> If we want the exact same logic to be followed as today then we need to
> keep the dummy struct posix_acl_{access,default}_xattr_handler around.
> I tried to avoid that for the first version because it felt a bit
> disappointing but we can live with this. This way there's zero code changes
> required for filesystems that use legacy array-based handler-indexing.

Yes, I'd just leave those as-is using the handlers.  I don't really
like the result, but the changes in the series doesn't really look
better and causes extra churn.  In the long run struct xattr_handler
needs to go away and we'll need separate handlers for each type
of xattrs, but that's going to take a while.  Do you know where the
capabilities conversion is standing?

> But we should probably still tweak this so that all these filesystems don't
> open-code the !h || (h->list && !h->list(dentry) check like they do now. So
> something like what I did below at [1]. Thoughts?

Yes, that part is useful.

> +static inline const char *erofs_xattr_prefix(unsigned int idx, struct dentry *dentry)

Overly long line here, though.

Re: [PATCH 05/12] erofs: drop posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:00:08AM +0100, Christian Brauner wrote:
> > > +	if (xattr_dentry_list(handler, dentry))
> > > +		name = xattr_prefix(handler);
> > 
> > I'm not a huge fan of all this duplicate logic in the file systems
> > that is more verbose and a bit confusing.  Until we remove the
> 
> Yeah, it hasn't been my favorite part about this either.
> But note how the few filesystems that receive that change use the same
> logic by indexing an array and retrieving the handler and then clumsily
> open-coding the same check that is now moved into xattr_dentry_list().

At least it allows for an array lookup.  And of course switching
to xattr_dentry_list instead of open coding it is always a good idea.

> If we want the exact same logic to be followed as today then we need to
> keep the dummy struct posix_acl_{access,default}_xattr_handler around.
> I tried to avoid that for the first version because it felt a bit
> disappointing but we can live with this. This way there's zero code changes
> required for filesystems that use legacy array-based handler-indexing.

Yes, I'd just leave those as-is using the handlers.  I don't really
like the result, but the changes in the series doesn't really look
better and causes extra churn.  In the long run struct xattr_handler
needs to go away and we'll need separate handlers for each type
of xattrs, but that's going to take a while.  Do you know where the
capabilities conversion is standing?

> But we should probably still tweak this so that all these filesystems don't
> open-code the !h || (h->list && !h->list(dentry) check like they do now. So
> something like what I did below at [1]. Thoughts?

Yes, that part is useful.

> +static inline const char *erofs_xattr_prefix(unsigned int idx, struct dentry *dentry)

Overly long line here, though.

[PATCH 06/12] ext2: drop posix acl handlers

[Christian Brauner]

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(-)

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

Re: [PATCH 06/12] ext2: drop posix acl handlers

[Jan Kara]

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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 07/12] ext4: drop posix acl handlers

[Christian Brauner]

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: <linux-ext4@vger.kernel.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/ext4/xattr.c | 71 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 31 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 7decaaf27e82..5472b8eac672 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -83,26 +83,9 @@ static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
 				    size_t value_count);
 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,
-#endif
-	[EXT4_XATTR_INDEX_TRUSTED]	     = &ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT4_FS_SECURITY
-	[EXT4_XATTR_INDEX_SECURITY]	     = &ext4_xattr_security_handler,
-#endif
-	[EXT4_XATTR_INDEX_HURD]		     = &ext4_xattr_hurd_handler,
-};
-
 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
@@ -110,6 +93,43 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
 	NULL
 };
 
+static const char *ext4_xattr_prefix(int xattr_index, struct dentry *dentry)
+{
+	const char *name = NULL;
+	const struct xattr_handler *handler = NULL;
+
+	switch (xattr_index) {
+	case EXT4_XATTR_INDEX_USER:
+		handler = &ext4_xattr_user_handler;
+		break;
+	case EXT4_XATTR_INDEX_TRUSTED:
+		handler = &ext4_xattr_trusted_handler;
+		break;
+#ifdef CONFIG_EXT4_FS_SECURITY
+	case EXT4_XATTR_INDEX_SECURITY:
+		handler = &ext4_xattr_security_handler;
+		break;
+#endif
+#ifdef CONFIG_EXT4_FS_POSIX_ACL
+	case EXT4_XATTR_INDEX_POSIX_ACL_ACCESS:
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case EXT4_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;
+}
+
 #define EA_BLOCK_CACHE(inode)	(((struct ext4_sb_info *) \
 				inode->i_sb->s_fs_info)->s_ea_block_cache)
 
@@ -171,16 +191,6 @@ 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)
-{
-	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;
-}
-
 static int
 ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
 			 void *value_start)
@@ -679,11 +689,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;
 

-- 
2.34.1

[f2fs-dev] [PATCH 08/12] f2fs: drop posix acl handlers

[Christian Brauner]

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: <linux-f2fs-devel@lists.sourceforge.net>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/f2fs/xattr.c | 63 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index dc2e8637189e..e5bd071fac8c 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -189,25 +189,8 @@ const struct xattr_handler f2fs_xattr_security_handler = {
 	.set	= f2fs_xattr_generic_set,
 };
 
-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,
-#endif
-	[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
-#ifdef CONFIG_F2FS_FS_SECURITY
-	[F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
-#endif
-	[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
-};
-
 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,
@@ -216,13 +199,44 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static const char *f2fs_xattr_prefix(int xattr_index, struct dentry *dentry)
 {
+	const char *name = NULL;
 	const struct xattr_handler *handler = NULL;
 
-	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
-		handler = f2fs_xattr_handler_map[index];
-	return handler;
+	switch (xattr_index) {
+	case F2FS_XATTR_INDEX_USER:
+		handler = &f2fs_xattr_user_handler;
+		break;
+	case F2FS_XATTR_INDEX_TRUSTED:
+		handler = &f2fs_xattr_trusted_handler;
+		break;
+	case F2FS_XATTR_INDEX_ADVISE:
+		handler = &f2fs_xattr_advise_handler;
+		break;
+#ifdef CONFIG_F2FS_FS_SECURITY
+	case F2FS_XATTR_INDEX_SECURITY:
+		handler = &f2fs_xattr_security_handler;
+		break;
+#endif
+#ifdef CONFIG_F2FS_FS_POSIX_ACL
+	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case F2FS_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 struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -573,12 +587,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",
@@ -590,10 +604,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) {

-- 
2.34.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 08/12] f2fs: drop posix acl handlers

[Christian Brauner]

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: <linux-f2fs-devel@lists.sourceforge.net>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/f2fs/xattr.c | 63 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index dc2e8637189e..e5bd071fac8c 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -189,25 +189,8 @@ const struct xattr_handler f2fs_xattr_security_handler = {
 	.set	= f2fs_xattr_generic_set,
 };
 
-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,
-#endif
-	[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
-#ifdef CONFIG_F2FS_FS_SECURITY
-	[F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
-#endif
-	[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
-};
-
 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,
@@ -216,13 +199,44 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static const char *f2fs_xattr_prefix(int xattr_index, struct dentry *dentry)
 {
+	const char *name = NULL;
 	const struct xattr_handler *handler = NULL;
 
-	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
-		handler = f2fs_xattr_handler_map[index];
-	return handler;
+	switch (xattr_index) {
+	case F2FS_XATTR_INDEX_USER:
+		handler = &f2fs_xattr_user_handler;
+		break;
+	case F2FS_XATTR_INDEX_TRUSTED:
+		handler = &f2fs_xattr_trusted_handler;
+		break;
+	case F2FS_XATTR_INDEX_ADVISE:
+		handler = &f2fs_xattr_advise_handler;
+		break;
+#ifdef CONFIG_F2FS_FS_SECURITY
+	case F2FS_XATTR_INDEX_SECURITY:
+		handler = &f2fs_xattr_security_handler;
+		break;
+#endif
+#ifdef CONFIG_F2FS_FS_POSIX_ACL
+	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case F2FS_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 struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -573,12 +587,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",
@@ -590,10 +604,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) {

-- 
2.34.1

[PATCH 09/12] jffs2: drop posix acl handlers

[Christian Brauner]

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: <linux-mtd@lists.infradead.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/jffs2/xattr.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index da3e18503c65..40cf03dc34be 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -919,43 +919,46 @@ 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
 };
 
-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 char *name = NULL;
+	const struct xattr_handler *handler = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
-		ret = &jffs2_user_xattr_handler;
+		handler = &jffs2_user_xattr_handler;
+		break;
+	case JFFS2_XPREFIX_TRUSTED:
+		handler = &jffs2_trusted_xattr_handler;
 		break;
 #ifdef CONFIG_JFFS2_FS_SECURITY
 	case JFFS2_XPREFIX_SECURITY:
-		ret = &jffs2_security_xattr_handler;
+		handler = &jffs2_security_xattr_handler;
 		break;
 #endif
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	case JFFS2_XPREFIX_ACL_ACCESS:
-		ret = &posix_acl_access_xattr_handler;
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
 		break;
 	case JFFS2_XPREFIX_ACL_DEFAULT:
-		ret = &posix_acl_default_xattr_handler;
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_DEFAULT;
 		break;
 #endif
-	case JFFS2_XPREFIX_TRUSTED:
-		ret = &jffs2_trusted_xattr_handler;
-		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (xattr_dentry_list(handler, dentry))
+		name = xattr_prefix(handler);
+
+	return name;
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -966,7 +969,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;
@@ -998,10 +1000,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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 09/12] jffs2: drop posix acl handlers

[Christian Brauner]

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: <linux-mtd@lists.infradead.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/jffs2/xattr.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index da3e18503c65..40cf03dc34be 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -919,43 +919,46 @@ 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
 };
 
-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 char *name = NULL;
+	const struct xattr_handler *handler = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
-		ret = &jffs2_user_xattr_handler;
+		handler = &jffs2_user_xattr_handler;
+		break;
+	case JFFS2_XPREFIX_TRUSTED:
+		handler = &jffs2_trusted_xattr_handler;
 		break;
 #ifdef CONFIG_JFFS2_FS_SECURITY
 	case JFFS2_XPREFIX_SECURITY:
-		ret = &jffs2_security_xattr_handler;
+		handler = &jffs2_security_xattr_handler;
 		break;
 #endif
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	case JFFS2_XPREFIX_ACL_ACCESS:
-		ret = &posix_acl_access_xattr_handler;
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_ACCESS;
 		break;
 	case JFFS2_XPREFIX_ACL_DEFAULT:
-		ret = &posix_acl_default_xattr_handler;
+		if (posix_acl_dentry_list(dentry))
+			name = XATTR_NAME_POSIX_ACL_DEFAULT;
 		break;
 #endif
-	case JFFS2_XPREFIX_TRUSTED:
-		ret = &jffs2_trusted_xattr_handler;
-		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (xattr_dentry_list(handler, dentry))
+		name = xattr_prefix(handler);
+
+	return name;
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -966,7 +969,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;
@@ -998,10 +1000,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

[PATCH 10/12] ocfs2: drop posix acl handlers

[Christian Brauner]

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: <ocfs2-devel@oss.oracle.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/ocfs2/xattr.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 95d0611c5fc7..e55cd11d72fc 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -89,23 +89,11 @@ 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
 };
 
-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,
-};
-
 struct ocfs2_xattr_info {
 	int		xi_name_index;
 	const char	*xi_name;
@@ -528,13 +516,34 @@ static int ocfs2_read_xattr_block(struct inode *inode, u64 xb_blkno,
 	return rc;
 }
 
-static inline const char *ocfs2_xattr_prefix(int name_index)
+static const char *ocfs2_xattr_prefix(int xattr_index)
 {
+	const char *name = NULL;
 	const struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
-		handler = ocfs2_xattr_handler_map[name_index];
-	return handler ? xattr_prefix(handler) : NULL;
+	switch (xattr_index) {
+	case OCFS2_XATTR_INDEX_USER:
+		handler = &ocfs2_xattr_user_handler;
+		break;
+	case OCFS2_XATTR_INDEX_TRUSTED:
+		handler = &ocfs2_xattr_trusted_handler;
+		break;
+	case OCFS2_XATTR_INDEX_SECURITY:
+		handler = &ocfs2_xattr_security_handler;
+		break;
+	case OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS:
+		name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT:
+		name = XATTR_NAME_POSIX_ACL_DEFAULT;
+		break;
+	default:
+		return NULL;
+	}
+
+	name = xattr_prefix(handler);
+
+	return name;
 }
 
 static u32 ocfs2_xattr_name_hash(struct inode *inode,

-- 
2.34.1

[Ocfs2-devel] [PATCH 10/12] ocfs2: drop posix acl handlers

[Christian Brauner via Ocfs2-devel]

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: <ocfs2-devel@oss.oracle.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/ocfs2/xattr.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 95d0611c5fc7..e55cd11d72fc 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -89,23 +89,11 @@ 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
 };
 
-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,
-};
-
 struct ocfs2_xattr_info {
 	int		xi_name_index;
 	const char	*xi_name;
@@ -528,13 +516,34 @@ static int ocfs2_read_xattr_block(struct inode *inode, u64 xb_blkno,
 	return rc;
 }
 
-static inline const char *ocfs2_xattr_prefix(int name_index)
+static const char *ocfs2_xattr_prefix(int xattr_index)
 {
+	const char *name = NULL;
 	const struct xattr_handler *handler = NULL;
 
-	if (name_index > 0 && name_index < OCFS2_XATTR_MAX)
-		handler = ocfs2_xattr_handler_map[name_index];
-	return handler ? xattr_prefix(handler) : NULL;
+	switch (xattr_index) {
+	case OCFS2_XATTR_INDEX_USER:
+		handler = &ocfs2_xattr_user_handler;
+		break;
+	case OCFS2_XATTR_INDEX_TRUSTED:
+		handler = &ocfs2_xattr_trusted_handler;
+		break;
+	case OCFS2_XATTR_INDEX_SECURITY:
+		handler = &ocfs2_xattr_security_handler;
+		break;
+	case OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS:
+		name = XATTR_NAME_POSIX_ACL_ACCESS;
+		break;
+	case OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT:
+		name = XATTR_NAME_POSIX_ACL_DEFAULT;
+		break;
+	default:
+		return NULL;
+	}
+
+	name = xattr_prefix(handler);
+
+	return name;
 }
 
 static u32 ocfs2_xattr_name_hash(struct inode *inode,

-- 
2.34.1


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

[PATCH 11/12] reiserfs: drop posix acl handlers

[Christian Brauner]

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: <reiserfs-devel@vger.kernel.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/reiserfs/xattr.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 8b2d52443f41..cc6f42128031 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"
@@ -771,22 +772,26 @@ reiserfs_xattr_get(struct inode *inode, const char *name, void *buffer,
 			(handler) = *(handlers)++)
 
 /* 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;
+	const struct xattr_handler *xah = NULL;
 
-	if (!handlers)
-		return NULL;
+	if (handlers) {
+		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_dentry_list(xah, dentry))
+				return false;
+
+			return true;
+		}
 	}
 
-	return xah;
+	return (posix_acl_type(name) >= 0) && posix_acl_dentry_list(dentry);
 }
 
 struct listxattr_buf {
@@ -807,12 +812,7 @@ 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 +910,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

[PATCH 12/12] acl: remove posix acl handlers

[Christian Brauner]

Now that everything in the VFS and all filesystems just rely on the
dedicated inode methods to interact with posix acls we can remove the
dummy generic posix acl xattr handlers. This way no filesystem an be
falsely under the impression that they are needed to implement posix acl
support.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/posix_acl.c                  | 20 --------------------
 include/linux/posix_acl_xattr.h |  3 ---
 2 files changed, 23 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index d7bc81fc0840..8028b6ee38a0 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -958,26 +958,6 @@ set_posix_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 }
 EXPORT_SYMBOL(set_posix_acl);
 
-static bool
-posix_acl_xattr_list(struct dentry *dentry)
-{
-	return IS_POSIXACL(d_backing_inode(dentry));
-}
-
-const struct xattr_handler posix_acl_access_xattr_handler = {
-	.name = XATTR_NAME_POSIX_ACL_ACCESS,
-	.flags = ACL_TYPE_ACCESS,
-	.list = posix_acl_xattr_list,
-};
-EXPORT_SYMBOL_GPL(posix_acl_access_xattr_handler);
-
-const struct xattr_handler posix_acl_default_xattr_handler = {
-	.name = XATTR_NAME_POSIX_ACL_DEFAULT,
-	.flags = ACL_TYPE_DEFAULT,
-	.list = posix_acl_xattr_list,
-};
-EXPORT_SYMBOL_GPL(posix_acl_default_xattr_handler);
-
 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 905d532ccd6e..bb657f3336e8 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -73,7 +73,4 @@ static inline bool posix_acl_dentry_list(struct dentry *dentry)
 	return IS_POSIXACL(d_backing_inode(dentry));
 }
 
-extern const struct xattr_handler posix_acl_access_xattr_handler;
-extern const struct xattr_handler posix_acl_default_xattr_handler;
-
 #endif	/* _POSIX_ACL_XATTR_H */

-- 
2.34.1

Re: [PATCH 12/12] acl: remove posix acl handlers

[kernel test robot]

Hi Christian,

I love your patch! Yet something to improve:

[auto build test ERROR on 1b929c02afd37871d5afb9d498426f83432e71c2]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Brauner/xattr-simplify-listxattr-helpers/20230125-193230
base:   1b929c02afd37871d5afb9d498426f83432e71c2
patch link:    https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-12-6cf155b492b6%40kernel.org
patch subject: [PATCH 12/12] acl: remove posix acl handlers
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20230131/202301311947.dXUqGsyN-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/e85a197723c6dd4770a12ded9ea9cae44c75a2c3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Brauner/xattr-simplify-listxattr-helpers/20230125-193230
        git checkout e85a197723c6dd4770a12ded9ea9cae44c75a2c3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/fuse/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/fuse/xattr.c:239:10: error: 'posix_acl_access_xattr_handler' undeclared here (not in a function); did you mean 'posix_acl_xattr_header'?
     239 |         &posix_acl_access_xattr_handler,
         |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |          posix_acl_xattr_header
>> fs/fuse/xattr.c:240:10: error: 'posix_acl_default_xattr_handler' undeclared here (not in a function); did you mean 'posix_acl_xattr_header'?
     240 |         &posix_acl_default_xattr_handler,
         |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |          posix_acl_xattr_header


vim +239 fs/fuse/xattr.c

60bcc88ad185d51 Seth Forshee      2016-08-29  237  
60bcc88ad185d51 Seth Forshee      2016-08-29  238  const struct xattr_handler *fuse_acl_xattr_handlers[] = {
60bcc88ad185d51 Seth Forshee      2016-08-29 @239  	&posix_acl_access_xattr_handler,
60bcc88ad185d51 Seth Forshee      2016-08-29 @240  	&posix_acl_default_xattr_handler,
60bcc88ad185d51 Seth Forshee      2016-08-29  241  	&fuse_xattr_handler,
60bcc88ad185d51 Seth Forshee      2016-08-29  242  	NULL
60bcc88ad185d51 Seth Forshee      2016-08-29  243  };
e45b2546e23c2d1 Eric W. Biederman 2018-05-04  244  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Wed, Jan 25, 2023 at 12:28:45PM +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 handler around for two
> reasons. First, because a few filesystems relied on the ->list() method
> of the generic POSIX ACL xattr handlers in their ->listxattr() inode
> operation. Second, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the removal of the legacy POSIX ACL handlers it is at least
> possible for a filesystem to only implement POSIX ACLs but no other
> xattrs. If that were to happen we would miss to raise IOP_XATTR because
> sb->s_xattr would be NULL.
> 
> Fix these things and then get rid of the misleading and effectively
> already unused generic POSIX ACL handlers.
> 
> For most filesystems it is a trivial removal of the generic POSIX ACL
> handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
> handler is used but rather easy to fix.
> 
> 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.
> 
> This branch depends on [1] which hopefully should be merged soon and can
> be pulled from [2] which already includes [1] so it's easy to test and
> compile.
> 
> With this all remnants of the old POSIX ACL xattr handling will be gone.
> 
> Thanks!
> Christian
> 
> [1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
> [2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1
> 
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> Christian Brauner (12):
>       xattr: simplify listxattr helpers
>       xattr, posix acl: add listxattr helpers
>       xattr: remove unused argument
>       fs: drop unused posix acl handlers
>       erofs: drop posix acl handlers
>       ext2: drop posix acl handlers
>       ext4: drop posix acl handlers
>       f2fs: drop posix acl handlers
>       jffs2: drop posix acl handlers
>       ocfs2: drop posix acl handlers
>       reiserfs: drop posix acl handlers
>       acl: remove posix acl handlers

I just realized that b4 dropped a patch when I created a series to
track. So this is missing an important bit. Christoph, Al, the
SB_I_XATTR bit is the best thing that I came up with so far. It works
well enough and I find the handler based logic weird anyway. But I'm
open to other ideas.

From e661c29b999f3d5d6293046ea1fe9e7d40cfa453 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 18 Jan 2023 10:00:00 +0100
Subject: [PATCH 01/14] fs: add SB_I_XATTR

Last cycle we introduced a dedicated POSIX ACLs api which only relies on
the associated inode operations. It does not depend on the xattr handler
infrastructure anymore.

However, a few filesystems still rely on the ->list() method of the
generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
This is a very limited set of filesystems. For most of them there is no
dependence on the generic POSIX ACL xattr handler in any way.

In addition, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the incoming removal of the legacy POSIX ACL handlers it is at
least possible for a filesystem to only implement POSIX ACLs but no
other xattrs. If that were to happen we would miss to raise IOP_XATTR
because sb->s_xattr would be NULL. While there currently is no such
filesystem we should still make sure that this just works should it ever
happen in the future.

We could try and use raise IOP_XATTR if we detect that the filesystems
implements the associated inode operations. But this doesn't work at the
time inode_init_always() is called because the inode->i_op field is not
yet initialized with the correct filesystem inode operations.

We could of course make the filesystems responsible for raising IOP_XATR
but this is messy as quite a few filesystems initialize inode->i_op in
multiple locations meaning we would have to scatter the checks to raise
IOP_XATTR across the filesystems. This is doable but ugly.

Another possibility would be to just check for SB_POSIXACL in
sb->s_flags and then raise IOP_XATTR. However, this doesn't work as for
some filesystems this flag can be turned on or off. For example, both
btrfs and ext4 support the "acl" and "noacl" mount option to turn
SB_POSIXACL on or off.

So if the filesystem is mounted with "noacl" then we cannot use
SB_POSIXACL as it might not be raised because the filesystem temporarily
disabled POSIX ACLs.

We could try and get rid of IOP_XATTR but it is used in subtle ways to
communicate that a given inode doesn't support xattrs even though the
superblock does in general and is especially prevalent in the lsm world.
This could potentially be solved by having each inode have a separate
xattr handler field instead of the superblock but would bloat each inode
and would be a more substantial change.

The option that chosen here is to introduce SB_I_XATTR which is raised
in sb->s_iflags whenever the superblock supports xattrs. That
encompasses POSIX ACLs and handler based xattrs. The flag will become
even more useful once fscaps have a dedicated inode operation and we're
really not short of flags anyway.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/9p/vfs_super.c    | 2 ++
 fs/afs/super.c       | 4 +++-
 fs/btrfs/super.c     | 1 +
 fs/ceph/super.c      | 1 +
 fs/cifs/cifsfs.c     | 1 +
 fs/ecryptfs/main.c   | 1 +
 fs/erofs/super.c     | 1 +
 fs/ext2/super.c      | 1 +
 fs/ext4/super.c      | 1 +
 fs/f2fs/super.c      | 1 +
 fs/fuse/inode.c      | 4 ++++
 fs/gfs2/ops_fstype.c | 1 +
 fs/hfs/super.c       | 1 +
 fs/hfsplus/super.c   | 1 +
 fs/inode.c           | 2 +-
 fs/jffs2/super.c     | 1 +
 fs/jfs/super.c       | 1 +
 fs/kernfs/mount.c    | 1 +
 fs/libfs.c           | 2 ++
 fs/nfs/super.c       | 3 +++
 fs/ntfs3/super.c     | 1 +
 fs/ocfs2/super.c     | 1 +
 fs/orangefs/super.c  | 1 +
 fs/overlayfs/super.c | 2 +-
 fs/reiserfs/super.c  | 1 +
 fs/squashfs/super.c  | 1 +
 fs/ubifs/super.c     | 1 +
 fs/xfs/xfs_super.c   | 1 +
 include/linux/fs.h   | 2 ++
 mm/shmem.c           | 2 ++
 30 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 266c4693e20c..003191b6ad97 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -93,6 +93,8 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 		sb->s_flags |= SB_POSIXACL;
 #endif
 
+	sb->s_iflags |= SB_I_XATTR;
+
 	return 0;
 }
 
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..379e6d7ac827 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -457,8 +457,10 @@ static int afs_fill_super(struct super_block *sb, struct afs_fs_context *ctx)
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_magic		= AFS_FS_MAGIC;
 	sb->s_op		= &afs_super_ops;
-	if (!as->dyn_root)
+	if (!as->dyn_root) {
 		sb->s_xattr	= afs_xattr_handlers;
+		sb->s_iflags	|= SB_I_XATTR;
+	}
 	ret = super_setup_bdi(sb);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f52ee85f6f..9af31008aafa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1141,6 +1141,7 @@ static int btrfs_fill_super(struct super_block *sb,
 #endif
 	sb->s_flags |= SB_I_VERSION;
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	err = super_setup_bdi(sb);
 	if (err) {
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 3fc48b43cab0..5aeb4fa0c4b0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1125,6 +1125,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
+	s->s_iflags |= SB_I_XATTR;
 
 	ret = set_anon_super_fc(s, fc);
 	if (ret != 0)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 10e00c624922..6a938b93d75d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -235,6 +235,7 @@ cifs_read_super(struct super_block *sb)
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	rc = super_setup_bdi(sb);
 	if (rc)
 		goto out_no_root;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067f..4ae69d23bc47 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -521,6 +521,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
 	s->s_xattr = ecryptfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 	s->s_d_op = &ecryptfs_dops;
 
 	err = "Reading sb failed";
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 481788c24a68..ea349cea9229 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -772,6 +772,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	sb->s_time_gran = 1;
 	sb->s_xattr = erofs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &erofs_export_ops;
 
 	if (test_opt(&sbi->opt, POSIX_ACL))
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..ee6e0c4978d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1171,6 +1171,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ext2_sops;
 	sb->s_export_op = &ext2_export_ops;
 	sb->s_xattr = ext2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..2706930beed2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5104,6 +5104,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 
 	/* i_version is always enabled now */
 	sb->s_flags |= SB_I_VERSION;
+	sb->s_iflags |= SB_I_XATTR;
 
 	if (ext4_check_feature_compatibility(sb, es, silent))
 		goto failed_mount;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f812b9ce985..6fff82f0efdf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4217,6 +4217,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
 	memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index de9b9ec5ce81..c9952bb9f9d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1181,6 +1181,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			if ((flags & FUSE_POSIX_ACL)) {
 				fc->default_permissions = 1;
 				fc->posix_acl = 1;
+				fm->sb->s_iflags |= SB_I_XATTR;
 			}
 			if (flags & FUSE_CACHE_SYMLINKS)
 				fc->cache_symlinks = 1;
@@ -1426,6 +1427,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
+	sb->s_iflags |= SB_I_XATTR;
 }
 
 static int fuse_fill_super_submount(struct super_block *sb,
@@ -1443,6 +1445,8 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	sb->s_bdi = bdi_get(parent_sb->s_bdi);
 
 	sb->s_xattr = parent_sb->s_xattr;
+	if (sb->s_xattr)
+		sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = parent_sb->s_time_gran;
 	sb->s_blocksize = parent_sb->s_blocksize;
 	sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c0cf1d2d0ef5..71c4fa1cdc1c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -506,6 +506,7 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
 	default:
 		BUG();
 	}
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* Set up the buffer cache and SB for real */
 	if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6f..6220ce2af0d0 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -401,6 +401,7 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
+	sb->s_iflags |= SB_I_XATTR;
 	mutex_init(&sbi->bitmap_lock);
 
 	res = hfs_mdb_get(sb);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89ebf9f..c5dd14057a78 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -490,6 +490,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE);
 	}
 	sb->s_xattr = hfsplus_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 	inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
 	if (IS_ERR(inode)) {
diff --git a/fs/inode.c b/fs/inode.c
index f453eb58fd03..51824bde0b35 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -169,7 +169,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_ino = 0;
 	inode->__i_nlink = 1;
 	inode->i_opflags = 0;
-	if (sb->s_xattr)
+	if (sb->s_iflags & SB_I_XATTR)
 		inode->i_opflags |= IOP_XATTR;
 	i_uid_write(inode, 0);
 	i_gid_write(inode, 0);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..1ae1ee573280 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,6 +285,7 @@ static int jffs2_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	sb->s_iflags |= SB_I_XATTR;
 	return jffs2_do_fill_super(sb, fc);
 }
 
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index d2f82cb7db1b..b78042d3f3c0 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -532,6 +532,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &jfs_super_operations;
 	sb->s_export_op = &jfs_export_operations;
 	sb->s_xattr = jfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
 	sb->s_qcop = &jfs_quotactl_ops;
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index e08e8d999807..cd3391419692 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -251,6 +251,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_magic = kfc->magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	if (info->root->flags & KERNFS_ROOT_SUPPORT_EXPORTOP)
 		sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..bd219120f248 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -328,6 +328,8 @@ static int pseudo_fs_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ctx->magic;
 	s->s_op = ctx->ops ?: &simple_super_operations;
 	s->s_xattr = ctx->xattr;
+	if (s->s_xattr)
+		s->s_iflags |= SB_I_XATTR;
 	s->s_time_gran = 1;
 	root = new_inode(s);
 	if (!root)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 05ae23657527..23c746745129 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,9 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 		break;
 	}
 
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..93e66112fac5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -950,6 +950,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_export_op = &ntfs_export_ops;
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_d_op = sbi->options->nocase ? &ntfs_dentry_ops : NULL;
 
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..514f5bcc9722 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2020,6 +2020,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->dq_op = &ocfs2_quota_operations;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 	sb->s_xattr = ocfs2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = 1;
 	sb->s_flags |= SB_NOATIME;
 	/* this is needed to support O_LARGEFILE */
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d..ec3e2d4ddb92 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -433,6 +433,7 @@ static int orangefs_fill_sb(struct super_block *sb,
 
 	/* Hang the xattr handlers off the superblock */
 	sb->s_xattr = orangefs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_magic = ORANGEFS_SUPER_MAGIC;
 	sb->s_op = &orangefs_s_ops;
 	sb->s_d_op = &orangefs_dentry_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..1002c8f332b9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -2060,7 +2060,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
 	sb->s_flags |= SB_POSIXACL;
-	sb->s_iflags |= SB_I_SKIP_SYNC;
+	sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_XATTR;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e731..1655b384e0b4 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2041,6 +2041,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 
 	s->s_xattr = reiserfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 
 	if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) {
 		SWARN(silent, s, "clm-7000",
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index e090fae48e68..7ab4fbc1f663 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -338,6 +338,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Handle xattrs */
 	sb->s_xattr = squashfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	xattr_id_table_start = le64_to_cpu(sblk->xattr_id_table_start);
 	if (xattr_id_table_start == SQUASHFS_INVALID_BLK) {
 		next_table = msblk->bytes_used;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d0c9a09988bc..be9e96cf9d5a 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2215,6 +2215,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	fscrypt_set_ops(sb, &ubifs_crypt_operations);
 
 	mutex_lock(&c->umount_mutex);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c4b73e9b29d..5a94d3eeab8a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1461,6 +1461,7 @@ xfs_fs_fill_super(
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &xfs_export_operations;
 #ifdef CONFIG_XFS_QUOTA
 	sb->s_qcop = &xfs_quotactl_operations;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 066555ad1bf8..700be4b969ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1449,6 +1449,8 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED	0x00000800	/* superblock shouldn't be reused */
 
+#define SB_I_XATTR	0x00001000	/* superblock supports xattrs */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
diff --git a/mm/shmem.c b/mm/shmem.c
index c301487be5fb..f9604b64624e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3817,6 +3817,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_TMPFS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
 	uuid_gen(&sb->s_uuid);
 
 	inode = shmem_get_inode(sb, NULL, S_IFDIR | sbinfo->mode, 0, VM_NORESERVE);
-- 
2.34.1

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Wed, Jan 25, 2023 at 12:28:45PM +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 handler around for two
> reasons. First, because a few filesystems relied on the ->list() method
> of the generic POSIX ACL xattr handlers in their ->listxattr() inode
> operation. Second, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the removal of the legacy POSIX ACL handlers it is at least
> possible for a filesystem to only implement POSIX ACLs but no other
> xattrs. If that were to happen we would miss to raise IOP_XATTR because
> sb->s_xattr would be NULL.
> 
> Fix these things and then get rid of the misleading and effectively
> already unused generic POSIX ACL handlers.
> 
> For most filesystems it is a trivial removal of the generic POSIX ACL
> handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
> handler is used but rather easy to fix.
> 
> 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.
> 
> This branch depends on [1] which hopefully should be merged soon and can
> be pulled from [2] which already includes [1] so it's easy to test and
> compile.
> 
> With this all remnants of the old POSIX ACL xattr handling will be gone.
> 
> Thanks!
> Christian
> 
> [1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
> [2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1
> 
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> Christian Brauner (12):
>       xattr: simplify listxattr helpers
>       xattr, posix acl: add listxattr helpers
>       xattr: remove unused argument
>       fs: drop unused posix acl handlers
>       erofs: drop posix acl handlers
>       ext2: drop posix acl handlers
>       ext4: drop posix acl handlers
>       f2fs: drop posix acl handlers
>       jffs2: drop posix acl handlers
>       ocfs2: drop posix acl handlers
>       reiserfs: drop posix acl handlers
>       acl: remove posix acl handlers

I just realized that b4 dropped a patch when I created a series to
track. So this is missing an important bit. Christoph, Al, the
SB_I_XATTR bit is the best thing that I came up with so far. It works
well enough and I find the handler based logic weird anyway. But I'm
open to other ideas.

From e661c29b999f3d5d6293046ea1fe9e7d40cfa453 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 18 Jan 2023 10:00:00 +0100
Subject: [PATCH 01/14] fs: add SB_I_XATTR

Last cycle we introduced a dedicated POSIX ACLs api which only relies on
the associated inode operations. It does not depend on the xattr handler
infrastructure anymore.

However, a few filesystems still rely on the ->list() method of the
generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
This is a very limited set of filesystems. For most of them there is no
dependence on the generic POSIX ACL xattr handler in any way.

In addition, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the incoming removal of the legacy POSIX ACL handlers it is at
least possible for a filesystem to only implement POSIX ACLs but no
other xattrs. If that were to happen we would miss to raise IOP_XATTR
because sb->s_xattr would be NULL. While there currently is no such
filesystem we should still make sure that this just works should it ever
happen in the future.

We could try and use raise IOP_XATTR if we detect that the filesystems
implements the associated inode operations. But this doesn't work at the
time inode_init_always() is called because the inode->i_op field is not
yet initialized with the correct filesystem inode operations.

We could of course make the filesystems responsible for raising IOP_XATR
but this is messy as quite a few filesystems initialize inode->i_op in
multiple locations meaning we would have to scatter the checks to raise
IOP_XATTR across the filesystems. This is doable but ugly.

Another possibility would be to just check for SB_POSIXACL in
sb->s_flags and then raise IOP_XATTR. However, this doesn't work as for
some filesystems this flag can be turned on or off. For example, both
btrfs and ext4 support the "acl" and "noacl" mount option to turn
SB_POSIXACL on or off.

So if the filesystem is mounted with "noacl" then we cannot use
SB_POSIXACL as it might not be raised because the filesystem temporarily
disabled POSIX ACLs.

We could try and get rid of IOP_XATTR but it is used in subtle ways to
communicate that a given inode doesn't support xattrs even though the
superblock does in general and is especially prevalent in the lsm world.
This could potentially be solved by having each inode have a separate
xattr handler field instead of the superblock but would bloat each inode
and would be a more substantial change.

The option that chosen here is to introduce SB_I_XATTR which is raised
in sb->s_iflags whenever the superblock supports xattrs. That
encompasses POSIX ACLs and handler based xattrs. The flag will become
even more useful once fscaps have a dedicated inode operation and we're
really not short of flags anyway.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/9p/vfs_super.c    | 2 ++
 fs/afs/super.c       | 4 +++-
 fs/btrfs/super.c     | 1 +
 fs/ceph/super.c      | 1 +
 fs/cifs/cifsfs.c     | 1 +
 fs/ecryptfs/main.c   | 1 +
 fs/erofs/super.c     | 1 +
 fs/ext2/super.c      | 1 +
 fs/ext4/super.c      | 1 +
 fs/f2fs/super.c      | 1 +
 fs/fuse/inode.c      | 4 ++++
 fs/gfs2/ops_fstype.c | 1 +
 fs/hfs/super.c       | 1 +
 fs/hfsplus/super.c   | 1 +
 fs/inode.c           | 2 +-
 fs/jffs2/super.c     | 1 +
 fs/jfs/super.c       | 1 +
 fs/kernfs/mount.c    | 1 +
 fs/libfs.c           | 2 ++
 fs/nfs/super.c       | 3 +++
 fs/ntfs3/super.c     | 1 +
 fs/ocfs2/super.c     | 1 +
 fs/orangefs/super.c  | 1 +
 fs/overlayfs/super.c | 2 +-
 fs/reiserfs/super.c  | 1 +
 fs/squashfs/super.c  | 1 +
 fs/ubifs/super.c     | 1 +
 fs/xfs/xfs_super.c   | 1 +
 include/linux/fs.h   | 2 ++
 mm/shmem.c           | 2 ++
 30 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 266c4693e20c..003191b6ad97 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -93,6 +93,8 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 		sb->s_flags |= SB_POSIXACL;
 #endif
 
+	sb->s_iflags |= SB_I_XATTR;
+
 	return 0;
 }
 
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..379e6d7ac827 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -457,8 +457,10 @@ static int afs_fill_super(struct super_block *sb, struct afs_fs_context *ctx)
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_magic		= AFS_FS_MAGIC;
 	sb->s_op		= &afs_super_ops;
-	if (!as->dyn_root)
+	if (!as->dyn_root) {
 		sb->s_xattr	= afs_xattr_handlers;
+		sb->s_iflags	|= SB_I_XATTR;
+	}
 	ret = super_setup_bdi(sb);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f52ee85f6f..9af31008aafa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1141,6 +1141,7 @@ static int btrfs_fill_super(struct super_block *sb,
 #endif
 	sb->s_flags |= SB_I_VERSION;
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	err = super_setup_bdi(sb);
 	if (err) {
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 3fc48b43cab0..5aeb4fa0c4b0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1125,6 +1125,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
+	s->s_iflags |= SB_I_XATTR;
 
 	ret = set_anon_super_fc(s, fc);
 	if (ret != 0)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 10e00c624922..6a938b93d75d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -235,6 +235,7 @@ cifs_read_super(struct super_block *sb)
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	rc = super_setup_bdi(sb);
 	if (rc)
 		goto out_no_root;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067f..4ae69d23bc47 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -521,6 +521,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
 	s->s_xattr = ecryptfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 	s->s_d_op = &ecryptfs_dops;
 
 	err = "Reading sb failed";
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 481788c24a68..ea349cea9229 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -772,6 +772,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	sb->s_time_gran = 1;
 	sb->s_xattr = erofs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &erofs_export_ops;
 
 	if (test_opt(&sbi->opt, POSIX_ACL))
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..ee6e0c4978d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1171,6 +1171,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ext2_sops;
 	sb->s_export_op = &ext2_export_ops;
 	sb->s_xattr = ext2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..2706930beed2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5104,6 +5104,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 
 	/* i_version is always enabled now */
 	sb->s_flags |= SB_I_VERSION;
+	sb->s_iflags |= SB_I_XATTR;
 
 	if (ext4_check_feature_compatibility(sb, es, silent))
 		goto failed_mount;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f812b9ce985..6fff82f0efdf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4217,6 +4217,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
 	memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index de9b9ec5ce81..c9952bb9f9d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1181,6 +1181,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			if ((flags & FUSE_POSIX_ACL)) {
 				fc->default_permissions = 1;
 				fc->posix_acl = 1;
+				fm->sb->s_iflags |= SB_I_XATTR;
 			}
 			if (flags & FUSE_CACHE_SYMLINKS)
 				fc->cache_symlinks = 1;
@@ -1426,6 +1427,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
+	sb->s_iflags |= SB_I_XATTR;
 }
 
 static int fuse_fill_super_submount(struct super_block *sb,
@@ -1443,6 +1445,8 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	sb->s_bdi = bdi_get(parent_sb->s_bdi);
 
 	sb->s_xattr = parent_sb->s_xattr;
+	if (sb->s_xattr)
+		sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = parent_sb->s_time_gran;
 	sb->s_blocksize = parent_sb->s_blocksize;
 	sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c0cf1d2d0ef5..71c4fa1cdc1c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -506,6 +506,7 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
 	default:
 		BUG();
 	}
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* Set up the buffer cache and SB for real */
 	if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6f..6220ce2af0d0 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -401,6 +401,7 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
+	sb->s_iflags |= SB_I_XATTR;
 	mutex_init(&sbi->bitmap_lock);
 
 	res = hfs_mdb_get(sb);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89ebf9f..c5dd14057a78 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -490,6 +490,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE);
 	}
 	sb->s_xattr = hfsplus_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 	inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
 	if (IS_ERR(inode)) {
diff --git a/fs/inode.c b/fs/inode.c
index f453eb58fd03..51824bde0b35 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -169,7 +169,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_ino = 0;
 	inode->__i_nlink = 1;
 	inode->i_opflags = 0;
-	if (sb->s_xattr)
+	if (sb->s_iflags & SB_I_XATTR)
 		inode->i_opflags |= IOP_XATTR;
 	i_uid_write(inode, 0);
 	i_gid_write(inode, 0);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..1ae1ee573280 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,6 +285,7 @@ static int jffs2_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	sb->s_iflags |= SB_I_XATTR;
 	return jffs2_do_fill_super(sb, fc);
 }
 
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index d2f82cb7db1b..b78042d3f3c0 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -532,6 +532,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &jfs_super_operations;
 	sb->s_export_op = &jfs_export_operations;
 	sb->s_xattr = jfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
 	sb->s_qcop = &jfs_quotactl_ops;
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index e08e8d999807..cd3391419692 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -251,6 +251,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_magic = kfc->magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	if (info->root->flags & KERNFS_ROOT_SUPPORT_EXPORTOP)
 		sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..bd219120f248 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -328,6 +328,8 @@ static int pseudo_fs_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ctx->magic;
 	s->s_op = ctx->ops ?: &simple_super_operations;
 	s->s_xattr = ctx->xattr;
+	if (s->s_xattr)
+		s->s_iflags |= SB_I_XATTR;
 	s->s_time_gran = 1;
 	root = new_inode(s);
 	if (!root)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 05ae23657527..23c746745129 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,9 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 		break;
 	}
 
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..93e66112fac5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -950,6 +950,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_export_op = &ntfs_export_ops;
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_d_op = sbi->options->nocase ? &ntfs_dentry_ops : NULL;
 
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..514f5bcc9722 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2020,6 +2020,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->dq_op = &ocfs2_quota_operations;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 	sb->s_xattr = ocfs2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = 1;
 	sb->s_flags |= SB_NOATIME;
 	/* this is needed to support O_LARGEFILE */
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d..ec3e2d4ddb92 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -433,6 +433,7 @@ static int orangefs_fill_sb(struct super_block *sb,
 
 	/* Hang the xattr handlers off the superblock */
 	sb->s_xattr = orangefs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_magic = ORANGEFS_SUPER_MAGIC;
 	sb->s_op = &orangefs_s_ops;
 	sb->s_d_op = &orangefs_dentry_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..1002c8f332b9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -2060,7 +2060,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
 	sb->s_flags |= SB_POSIXACL;
-	sb->s_iflags |= SB_I_SKIP_SYNC;
+	sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_XATTR;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e731..1655b384e0b4 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2041,6 +2041,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 
 	s->s_xattr = reiserfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 
 	if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) {
 		SWARN(silent, s, "clm-7000",
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index e090fae48e68..7ab4fbc1f663 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -338,6 +338,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Handle xattrs */
 	sb->s_xattr = squashfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	xattr_id_table_start = le64_to_cpu(sblk->xattr_id_table_start);
 	if (xattr_id_table_start == SQUASHFS_INVALID_BLK) {
 		next_table = msblk->bytes_used;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d0c9a09988bc..be9e96cf9d5a 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2215,6 +2215,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	fscrypt_set_ops(sb, &ubifs_crypt_operations);
 
 	mutex_lock(&c->umount_mutex);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c4b73e9b29d..5a94d3eeab8a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1461,6 +1461,7 @@ xfs_fs_fill_super(
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &xfs_export_operations;
 #ifdef CONFIG_XFS_QUOTA
 	sb->s_qcop = &xfs_quotactl_operations;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 066555ad1bf8..700be4b969ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1449,6 +1449,8 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED	0x00000800	/* superblock shouldn't be reused */
 
+#define SB_I_XATTR	0x00001000	/* superblock supports xattrs */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
diff --git a/mm/shmem.c b/mm/shmem.c
index c301487be5fb..f9604b64624e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3817,6 +3817,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_TMPFS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
 	uuid_gen(&sb->s_uuid);
 
 	inode = shmem_get_inode(sb, NULL, S_IFDIR | sbinfo->mode, 0, VM_NORESERVE);
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [f2fs-dev] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Wed, Jan 25, 2023 at 12:28:45PM +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 handler around for two
> reasons. First, because a few filesystems relied on the ->list() method
> of the generic POSIX ACL xattr handlers in their ->listxattr() inode
> operation. Second, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the removal of the legacy POSIX ACL handlers it is at least
> possible for a filesystem to only implement POSIX ACLs but no other
> xattrs. If that were to happen we would miss to raise IOP_XATTR because
> sb->s_xattr would be NULL.
> 
> Fix these things and then get rid of the misleading and effectively
> already unused generic POSIX ACL handlers.
> 
> For most filesystems it is a trivial removal of the generic POSIX ACL
> handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
> handler is used but rather easy to fix.
> 
> 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.
> 
> This branch depends on [1] which hopefully should be merged soon and can
> be pulled from [2] which already includes [1] so it's easy to test and
> compile.
> 
> With this all remnants of the old POSIX ACL xattr handling will be gone.
> 
> Thanks!
> Christian
> 
> [1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
> [2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1
> 
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> Christian Brauner (12):
>       xattr: simplify listxattr helpers
>       xattr, posix acl: add listxattr helpers
>       xattr: remove unused argument
>       fs: drop unused posix acl handlers
>       erofs: drop posix acl handlers
>       ext2: drop posix acl handlers
>       ext4: drop posix acl handlers
>       f2fs: drop posix acl handlers
>       jffs2: drop posix acl handlers
>       ocfs2: drop posix acl handlers
>       reiserfs: drop posix acl handlers
>       acl: remove posix acl handlers

I just realized that b4 dropped a patch when I created a series to
track. So this is missing an important bit. Christoph, Al, the
SB_I_XATTR bit is the best thing that I came up with so far. It works
well enough and I find the handler based logic weird anyway. But I'm
open to other ideas.

From e661c29b999f3d5d6293046ea1fe9e7d40cfa453 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 18 Jan 2023 10:00:00 +0100
Subject: [PATCH 01/14] fs: add SB_I_XATTR

Last cycle we introduced a dedicated POSIX ACLs api which only relies on
the associated inode operations. It does not depend on the xattr handler
infrastructure anymore.

However, a few filesystems still rely on the ->list() method of the
generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
This is a very limited set of filesystems. For most of them there is no
dependence on the generic POSIX ACL xattr handler in any way.

In addition, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the incoming removal of the legacy POSIX ACL handlers it is at
least possible for a filesystem to only implement POSIX ACLs but no
other xattrs. If that were to happen we would miss to raise IOP_XATTR
because sb->s_xattr would be NULL. While there currently is no such
filesystem we should still make sure that this just works should it ever
happen in the future.

We could try and use raise IOP_XATTR if we detect that the filesystems
implements the associated inode operations. But this doesn't work at the
time inode_init_always() is called because the inode->i_op field is not
yet initialized with the correct filesystem inode operations.

We could of course make the filesystems responsible for raising IOP_XATR
but this is messy as quite a few filesystems initialize inode->i_op in
multiple locations meaning we would have to scatter the checks to raise
IOP_XATTR across the filesystems. This is doable but ugly.

Another possibility would be to just check for SB_POSIXACL in
sb->s_flags and then raise IOP_XATTR. However, this doesn't work as for
some filesystems this flag can be turned on or off. For example, both
btrfs and ext4 support the "acl" and "noacl" mount option to turn
SB_POSIXACL on or off.

So if the filesystem is mounted with "noacl" then we cannot use
SB_POSIXACL as it might not be raised because the filesystem temporarily
disabled POSIX ACLs.

We could try and get rid of IOP_XATTR but it is used in subtle ways to
communicate that a given inode doesn't support xattrs even though the
superblock does in general and is especially prevalent in the lsm world.
This could potentially be solved by having each inode have a separate
xattr handler field instead of the superblock but would bloat each inode
and would be a more substantial change.

The option that chosen here is to introduce SB_I_XATTR which is raised
in sb->s_iflags whenever the superblock supports xattrs. That
encompasses POSIX ACLs and handler based xattrs. The flag will become
even more useful once fscaps have a dedicated inode operation and we're
really not short of flags anyway.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/9p/vfs_super.c    | 2 ++
 fs/afs/super.c       | 4 +++-
 fs/btrfs/super.c     | 1 +
 fs/ceph/super.c      | 1 +
 fs/cifs/cifsfs.c     | 1 +
 fs/ecryptfs/main.c   | 1 +
 fs/erofs/super.c     | 1 +
 fs/ext2/super.c      | 1 +
 fs/ext4/super.c      | 1 +
 fs/f2fs/super.c      | 1 +
 fs/fuse/inode.c      | 4 ++++
 fs/gfs2/ops_fstype.c | 1 +
 fs/hfs/super.c       | 1 +
 fs/hfsplus/super.c   | 1 +
 fs/inode.c           | 2 +-
 fs/jffs2/super.c     | 1 +
 fs/jfs/super.c       | 1 +
 fs/kernfs/mount.c    | 1 +
 fs/libfs.c           | 2 ++
 fs/nfs/super.c       | 3 +++
 fs/ntfs3/super.c     | 1 +
 fs/ocfs2/super.c     | 1 +
 fs/orangefs/super.c  | 1 +
 fs/overlayfs/super.c | 2 +-
 fs/reiserfs/super.c  | 1 +
 fs/squashfs/super.c  | 1 +
 fs/ubifs/super.c     | 1 +
 fs/xfs/xfs_super.c   | 1 +
 include/linux/fs.h   | 2 ++
 mm/shmem.c           | 2 ++
 30 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 266c4693e20c..003191b6ad97 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -93,6 +93,8 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 		sb->s_flags |= SB_POSIXACL;
 #endif
 
+	sb->s_iflags |= SB_I_XATTR;
+
 	return 0;
 }
 
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..379e6d7ac827 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -457,8 +457,10 @@ static int afs_fill_super(struct super_block *sb, struct afs_fs_context *ctx)
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_magic		= AFS_FS_MAGIC;
 	sb->s_op		= &afs_super_ops;
-	if (!as->dyn_root)
+	if (!as->dyn_root) {
 		sb->s_xattr	= afs_xattr_handlers;
+		sb->s_iflags	|= SB_I_XATTR;
+	}
 	ret = super_setup_bdi(sb);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f52ee85f6f..9af31008aafa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1141,6 +1141,7 @@ static int btrfs_fill_super(struct super_block *sb,
 #endif
 	sb->s_flags |= SB_I_VERSION;
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	err = super_setup_bdi(sb);
 	if (err) {
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 3fc48b43cab0..5aeb4fa0c4b0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1125,6 +1125,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
+	s->s_iflags |= SB_I_XATTR;
 
 	ret = set_anon_super_fc(s, fc);
 	if (ret != 0)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 10e00c624922..6a938b93d75d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -235,6 +235,7 @@ cifs_read_super(struct super_block *sb)
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	rc = super_setup_bdi(sb);
 	if (rc)
 		goto out_no_root;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067f..4ae69d23bc47 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -521,6 +521,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
 	s->s_xattr = ecryptfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 	s->s_d_op = &ecryptfs_dops;
 
 	err = "Reading sb failed";
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 481788c24a68..ea349cea9229 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -772,6 +772,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	sb->s_time_gran = 1;
 	sb->s_xattr = erofs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &erofs_export_ops;
 
 	if (test_opt(&sbi->opt, POSIX_ACL))
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..ee6e0c4978d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1171,6 +1171,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ext2_sops;
 	sb->s_export_op = &ext2_export_ops;
 	sb->s_xattr = ext2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..2706930beed2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5104,6 +5104,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 
 	/* i_version is always enabled now */
 	sb->s_flags |= SB_I_VERSION;
+	sb->s_iflags |= SB_I_XATTR;
 
 	if (ext4_check_feature_compatibility(sb, es, silent))
 		goto failed_mount;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f812b9ce985..6fff82f0efdf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4217,6 +4217,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
 	memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index de9b9ec5ce81..c9952bb9f9d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1181,6 +1181,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			if ((flags & FUSE_POSIX_ACL)) {
 				fc->default_permissions = 1;
 				fc->posix_acl = 1;
+				fm->sb->s_iflags |= SB_I_XATTR;
 			}
 			if (flags & FUSE_CACHE_SYMLINKS)
 				fc->cache_symlinks = 1;
@@ -1426,6 +1427,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
+	sb->s_iflags |= SB_I_XATTR;
 }
 
 static int fuse_fill_super_submount(struct super_block *sb,
@@ -1443,6 +1445,8 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	sb->s_bdi = bdi_get(parent_sb->s_bdi);
 
 	sb->s_xattr = parent_sb->s_xattr;
+	if (sb->s_xattr)
+		sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = parent_sb->s_time_gran;
 	sb->s_blocksize = parent_sb->s_blocksize;
 	sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c0cf1d2d0ef5..71c4fa1cdc1c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -506,6 +506,7 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
 	default:
 		BUG();
 	}
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* Set up the buffer cache and SB for real */
 	if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6f..6220ce2af0d0 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -401,6 +401,7 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
+	sb->s_iflags |= SB_I_XATTR;
 	mutex_init(&sbi->bitmap_lock);
 
 	res = hfs_mdb_get(sb);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89ebf9f..c5dd14057a78 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -490,6 +490,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE);
 	}
 	sb->s_xattr = hfsplus_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 	inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
 	if (IS_ERR(inode)) {
diff --git a/fs/inode.c b/fs/inode.c
index f453eb58fd03..51824bde0b35 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -169,7 +169,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_ino = 0;
 	inode->__i_nlink = 1;
 	inode->i_opflags = 0;
-	if (sb->s_xattr)
+	if (sb->s_iflags & SB_I_XATTR)
 		inode->i_opflags |= IOP_XATTR;
 	i_uid_write(inode, 0);
 	i_gid_write(inode, 0);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..1ae1ee573280 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,6 +285,7 @@ static int jffs2_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	sb->s_iflags |= SB_I_XATTR;
 	return jffs2_do_fill_super(sb, fc);
 }
 
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index d2f82cb7db1b..b78042d3f3c0 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -532,6 +532,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &jfs_super_operations;
 	sb->s_export_op = &jfs_export_operations;
 	sb->s_xattr = jfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
 	sb->s_qcop = &jfs_quotactl_ops;
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index e08e8d999807..cd3391419692 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -251,6 +251,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_magic = kfc->magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	if (info->root->flags & KERNFS_ROOT_SUPPORT_EXPORTOP)
 		sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..bd219120f248 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -328,6 +328,8 @@ static int pseudo_fs_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ctx->magic;
 	s->s_op = ctx->ops ?: &simple_super_operations;
 	s->s_xattr = ctx->xattr;
+	if (s->s_xattr)
+		s->s_iflags |= SB_I_XATTR;
 	s->s_time_gran = 1;
 	root = new_inode(s);
 	if (!root)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 05ae23657527..23c746745129 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,9 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 		break;
 	}
 
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..93e66112fac5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -950,6 +950,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_export_op = &ntfs_export_ops;
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_d_op = sbi->options->nocase ? &ntfs_dentry_ops : NULL;
 
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..514f5bcc9722 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2020,6 +2020,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->dq_op = &ocfs2_quota_operations;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 	sb->s_xattr = ocfs2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = 1;
 	sb->s_flags |= SB_NOATIME;
 	/* this is needed to support O_LARGEFILE */
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d..ec3e2d4ddb92 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -433,6 +433,7 @@ static int orangefs_fill_sb(struct super_block *sb,
 
 	/* Hang the xattr handlers off the superblock */
 	sb->s_xattr = orangefs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_magic = ORANGEFS_SUPER_MAGIC;
 	sb->s_op = &orangefs_s_ops;
 	sb->s_d_op = &orangefs_dentry_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..1002c8f332b9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -2060,7 +2060,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
 	sb->s_flags |= SB_POSIXACL;
-	sb->s_iflags |= SB_I_SKIP_SYNC;
+	sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_XATTR;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e731..1655b384e0b4 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2041,6 +2041,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 
 	s->s_xattr = reiserfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 
 	if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) {
 		SWARN(silent, s, "clm-7000",
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index e090fae48e68..7ab4fbc1f663 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -338,6 +338,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Handle xattrs */
 	sb->s_xattr = squashfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	xattr_id_table_start = le64_to_cpu(sblk->xattr_id_table_start);
 	if (xattr_id_table_start == SQUASHFS_INVALID_BLK) {
 		next_table = msblk->bytes_used;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d0c9a09988bc..be9e96cf9d5a 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2215,6 +2215,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	fscrypt_set_ops(sb, &ubifs_crypt_operations);
 
 	mutex_lock(&c->umount_mutex);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c4b73e9b29d..5a94d3eeab8a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1461,6 +1461,7 @@ xfs_fs_fill_super(
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &xfs_export_operations;
 #ifdef CONFIG_XFS_QUOTA
 	sb->s_qcop = &xfs_quotactl_operations;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 066555ad1bf8..700be4b969ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1449,6 +1449,8 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED	0x00000800	/* superblock shouldn't be reused */
 
+#define SB_I_XATTR	0x00001000	/* superblock supports xattrs */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
diff --git a/mm/shmem.c b/mm/shmem.c
index c301487be5fb..f9604b64624e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3817,6 +3817,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_TMPFS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
 	uuid_gen(&sb->s_uuid);
 
 	inode = shmem_get_inode(sb, NULL, S_IFDIR | sbinfo->mode, 0, VM_NORESERVE);
-- 
2.34.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Wed, Jan 25, 2023 at 12:28:45PM +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 handler around for two
> reasons. First, because a few filesystems relied on the ->list() method
> of the generic POSIX ACL xattr handlers in their ->listxattr() inode
> operation. Second, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the removal of the legacy POSIX ACL handlers it is at least
> possible for a filesystem to only implement POSIX ACLs but no other
> xattrs. If that were to happen we would miss to raise IOP_XATTR because
> sb->s_xattr would be NULL.
> 
> Fix these things and then get rid of the misleading and effectively
> already unused generic POSIX ACL handlers.
> 
> For most filesystems it is a trivial removal of the generic POSIX ACL
> handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
> handler is used but rather easy to fix.
> 
> 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.
> 
> This branch depends on [1] which hopefully should be merged soon and can
> be pulled from [2] which already includes [1] so it's easy to test and
> compile.
> 
> With this all remnants of the old POSIX ACL xattr handling will be gone.
> 
> Thanks!
> Christian
> 
> [1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
> [2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1
> 
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> Christian Brauner (12):
>       xattr: simplify listxattr helpers
>       xattr, posix acl: add listxattr helpers
>       xattr: remove unused argument
>       fs: drop unused posix acl handlers
>       erofs: drop posix acl handlers
>       ext2: drop posix acl handlers
>       ext4: drop posix acl handlers
>       f2fs: drop posix acl handlers
>       jffs2: drop posix acl handlers
>       ocfs2: drop posix acl handlers
>       reiserfs: drop posix acl handlers
>       acl: remove posix acl handlers

I just realized that b4 dropped a patch when I created a series to
track. So this is missing an important bit. Christoph, Al, the
SB_I_XATTR bit is the best thing that I came up with so far. It works
well enough and I find the handler based logic weird anyway. But I'm
open to other ideas.

From e661c29b999f3d5d6293046ea1fe9e7d40cfa453 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 18 Jan 2023 10:00:00 +0100
Subject: [PATCH 01/14] fs: add SB_I_XATTR

Last cycle we introduced a dedicated POSIX ACLs api which only relies on
the associated inode operations. It does not depend on the xattr handler
infrastructure anymore.

However, a few filesystems still rely on the ->list() method of the
generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
This is a very limited set of filesystems. For most of them there is no
dependence on the generic POSIX ACL xattr handler in any way.

In addition, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the incoming removal of the legacy POSIX ACL handlers it is at
least possible for a filesystem to only implement POSIX ACLs but no
other xattrs. If that were to happen we would miss to raise IOP_XATTR
because sb->s_xattr would be NULL. While there currently is no such
filesystem we should still make sure that this just works should it ever
happen in the future.

We could try and use raise IOP_XATTR if we detect that the filesystems
implements the associated inode operations. But this doesn't work at the
time inode_init_always() is called because the inode->i_op field is not
yet initialized with the correct filesystem inode operations.

We could of course make the filesystems responsible for raising IOP_XATR
but this is messy as quite a few filesystems initialize inode->i_op in
multiple locations meaning we would have to scatter the checks to raise
IOP_XATTR across the filesystems. This is doable but ugly.

Another possibility would be to just check for SB_POSIXACL in
sb->s_flags and then raise IOP_XATTR. However, this doesn't work as for
some filesystems this flag can be turned on or off. For example, both
btrfs and ext4 support the "acl" and "noacl" mount option to turn
SB_POSIXACL on or off.

So if the filesystem is mounted with "noacl" then we cannot use
SB_POSIXACL as it might not be raised because the filesystem temporarily
disabled POSIX ACLs.

We could try and get rid of IOP_XATTR but it is used in subtle ways to
communicate that a given inode doesn't support xattrs even though the
superblock does in general and is especially prevalent in the lsm world.
This could potentially be solved by having each inode have a separate
xattr handler field instead of the superblock but would bloat each inode
and would be a more substantial change.

The option that chosen here is to introduce SB_I_XATTR which is raised
in sb->s_iflags whenever the superblock supports xattrs. That
encompasses POSIX ACLs and handler based xattrs. The flag will become
even more useful once fscaps have a dedicated inode operation and we're
really not short of flags anyway.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/9p/vfs_super.c    | 2 ++
 fs/afs/super.c       | 4 +++-
 fs/btrfs/super.c     | 1 +
 fs/ceph/super.c      | 1 +
 fs/cifs/cifsfs.c     | 1 +
 fs/ecryptfs/main.c   | 1 +
 fs/erofs/super.c     | 1 +
 fs/ext2/super.c      | 1 +
 fs/ext4/super.c      | 1 +
 fs/f2fs/super.c      | 1 +
 fs/fuse/inode.c      | 4 ++++
 fs/gfs2/ops_fstype.c | 1 +
 fs/hfs/super.c       | 1 +
 fs/hfsplus/super.c   | 1 +
 fs/inode.c           | 2 +-
 fs/jffs2/super.c     | 1 +
 fs/jfs/super.c       | 1 +
 fs/kernfs/mount.c    | 1 +
 fs/libfs.c           | 2 ++
 fs/nfs/super.c       | 3 +++
 fs/ntfs3/super.c     | 1 +
 fs/ocfs2/super.c     | 1 +
 fs/orangefs/super.c  | 1 +
 fs/overlayfs/super.c | 2 +-
 fs/reiserfs/super.c  | 1 +
 fs/squashfs/super.c  | 1 +
 fs/ubifs/super.c     | 1 +
 fs/xfs/xfs_super.c   | 1 +
 include/linux/fs.h   | 2 ++
 mm/shmem.c           | 2 ++
 30 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 266c4693e20c..003191b6ad97 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -93,6 +93,8 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 		sb->s_flags |= SB_POSIXACL;
 #endif
 
+	sb->s_iflags |= SB_I_XATTR;
+
 	return 0;
 }
 
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..379e6d7ac827 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -457,8 +457,10 @@ static int afs_fill_super(struct super_block *sb, struct afs_fs_context *ctx)
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_magic		= AFS_FS_MAGIC;
 	sb->s_op		= &afs_super_ops;
-	if (!as->dyn_root)
+	if (!as->dyn_root) {
 		sb->s_xattr	= afs_xattr_handlers;
+		sb->s_iflags	|= SB_I_XATTR;
+	}
 	ret = super_setup_bdi(sb);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f52ee85f6f..9af31008aafa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1141,6 +1141,7 @@ static int btrfs_fill_super(struct super_block *sb,
 #endif
 	sb->s_flags |= SB_I_VERSION;
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	err = super_setup_bdi(sb);
 	if (err) {
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 3fc48b43cab0..5aeb4fa0c4b0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1125,6 +1125,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
+	s->s_iflags |= SB_I_XATTR;
 
 	ret = set_anon_super_fc(s, fc);
 	if (ret != 0)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 10e00c624922..6a938b93d75d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -235,6 +235,7 @@ cifs_read_super(struct super_block *sb)
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	rc = super_setup_bdi(sb);
 	if (rc)
 		goto out_no_root;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067f..4ae69d23bc47 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -521,6 +521,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
 	s->s_xattr = ecryptfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 	s->s_d_op = &ecryptfs_dops;
 
 	err = "Reading sb failed";
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 481788c24a68..ea349cea9229 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -772,6 +772,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	sb->s_time_gran = 1;
 	sb->s_xattr = erofs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &erofs_export_ops;
 
 	if (test_opt(&sbi->opt, POSIX_ACL))
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..ee6e0c4978d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1171,6 +1171,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ext2_sops;
 	sb->s_export_op = &ext2_export_ops;
 	sb->s_xattr = ext2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..2706930beed2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5104,6 +5104,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 
 	/* i_version is always enabled now */
 	sb->s_flags |= SB_I_VERSION;
+	sb->s_iflags |= SB_I_XATTR;
 
 	if (ext4_check_feature_compatibility(sb, es, silent))
 		goto failed_mount;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f812b9ce985..6fff82f0efdf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4217,6 +4217,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
 	memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index de9b9ec5ce81..c9952bb9f9d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1181,6 +1181,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			if ((flags & FUSE_POSIX_ACL)) {
 				fc->default_permissions = 1;
 				fc->posix_acl = 1;
+				fm->sb->s_iflags |= SB_I_XATTR;
 			}
 			if (flags & FUSE_CACHE_SYMLINKS)
 				fc->cache_symlinks = 1;
@@ -1426,6 +1427,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
+	sb->s_iflags |= SB_I_XATTR;
 }
 
 static int fuse_fill_super_submount(struct super_block *sb,
@@ -1443,6 +1445,8 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	sb->s_bdi = bdi_get(parent_sb->s_bdi);
 
 	sb->s_xattr = parent_sb->s_xattr;
+	if (sb->s_xattr)
+		sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = parent_sb->s_time_gran;
 	sb->s_blocksize = parent_sb->s_blocksize;
 	sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c0cf1d2d0ef5..71c4fa1cdc1c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -506,6 +506,7 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
 	default:
 		BUG();
 	}
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* Set up the buffer cache and SB for real */
 	if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6f..6220ce2af0d0 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -401,6 +401,7 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
+	sb->s_iflags |= SB_I_XATTR;
 	mutex_init(&sbi->bitmap_lock);
 
 	res = hfs_mdb_get(sb);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89ebf9f..c5dd14057a78 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -490,6 +490,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE);
 	}
 	sb->s_xattr = hfsplus_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 	inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
 	if (IS_ERR(inode)) {
diff --git a/fs/inode.c b/fs/inode.c
index f453eb58fd03..51824bde0b35 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -169,7 +169,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_ino = 0;
 	inode->__i_nlink = 1;
 	inode->i_opflags = 0;
-	if (sb->s_xattr)
+	if (sb->s_iflags & SB_I_XATTR)
 		inode->i_opflags |= IOP_XATTR;
 	i_uid_write(inode, 0);
 	i_gid_write(inode, 0);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..1ae1ee573280 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,6 +285,7 @@ static int jffs2_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	sb->s_iflags |= SB_I_XATTR;
 	return jffs2_do_fill_super(sb, fc);
 }
 
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index d2f82cb7db1b..b78042d3f3c0 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -532,6 +532,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &jfs_super_operations;
 	sb->s_export_op = &jfs_export_operations;
 	sb->s_xattr = jfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
 	sb->s_qcop = &jfs_quotactl_ops;
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index e08e8d999807..cd3391419692 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -251,6 +251,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_magic = kfc->magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	if (info->root->flags & KERNFS_ROOT_SUPPORT_EXPORTOP)
 		sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..bd219120f248 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -328,6 +328,8 @@ static int pseudo_fs_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ctx->magic;
 	s->s_op = ctx->ops ?: &simple_super_operations;
 	s->s_xattr = ctx->xattr;
+	if (s->s_xattr)
+		s->s_iflags |= SB_I_XATTR;
 	s->s_time_gran = 1;
 	root = new_inode(s);
 	if (!root)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 05ae23657527..23c746745129 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,9 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 		break;
 	}
 
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..93e66112fac5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -950,6 +950,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_export_op = &ntfs_export_ops;
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_d_op = sbi->options->nocase ? &ntfs_dentry_ops : NULL;
 
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..514f5bcc9722 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2020,6 +2020,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->dq_op = &ocfs2_quota_operations;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 	sb->s_xattr = ocfs2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = 1;
 	sb->s_flags |= SB_NOATIME;
 	/* this is needed to support O_LARGEFILE */
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d..ec3e2d4ddb92 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -433,6 +433,7 @@ static int orangefs_fill_sb(struct super_block *sb,
 
 	/* Hang the xattr handlers off the superblock */
 	sb->s_xattr = orangefs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_magic = ORANGEFS_SUPER_MAGIC;
 	sb->s_op = &orangefs_s_ops;
 	sb->s_d_op = &orangefs_dentry_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..1002c8f332b9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -2060,7 +2060,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
 	sb->s_flags |= SB_POSIXACL;
-	sb->s_iflags |= SB_I_SKIP_SYNC;
+	sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_XATTR;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e731..1655b384e0b4 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2041,6 +2041,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 
 	s->s_xattr = reiserfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 
 	if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) {
 		SWARN(silent, s, "clm-7000",
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index e090fae48e68..7ab4fbc1f663 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -338,6 +338,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Handle xattrs */
 	sb->s_xattr = squashfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	xattr_id_table_start = le64_to_cpu(sblk->xattr_id_table_start);
 	if (xattr_id_table_start == SQUASHFS_INVALID_BLK) {
 		next_table = msblk->bytes_used;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d0c9a09988bc..be9e96cf9d5a 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2215,6 +2215,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	fscrypt_set_ops(sb, &ubifs_crypt_operations);
 
 	mutex_lock(&c->umount_mutex);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c4b73e9b29d..5a94d3eeab8a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1461,6 +1461,7 @@ xfs_fs_fill_super(
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &xfs_export_operations;
 #ifdef CONFIG_XFS_QUOTA
 	sb->s_qcop = &xfs_quotactl_operations;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 066555ad1bf8..700be4b969ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1449,6 +1449,8 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED	0x00000800	/* superblock shouldn't be reused */
 
+#define SB_I_XATTR	0x00001000	/* superblock supports xattrs */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
diff --git a/mm/shmem.c b/mm/shmem.c
index c301487be5fb..f9604b64624e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3817,6 +3817,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_TMPFS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
 	uuid_gen(&sb->s_uuid);
 
 	inode = shmem_get_inode(sb, NULL, S_IFDIR | sbinfo->mode, 0, VM_NORESERVE);
-- 
2.34.1

Re: [Ocfs2-devel] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner via Ocfs2-devel]

On Wed, Jan 25, 2023 at 12:28:45PM +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 handler around for two
> reasons. First, because a few filesystems relied on the ->list() method
> of the generic POSIX ACL xattr handlers in their ->listxattr() inode
> operation. Second, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the removal of the legacy POSIX ACL handlers it is at least
> possible for a filesystem to only implement POSIX ACLs but no other
> xattrs. If that were to happen we would miss to raise IOP_XATTR because
> sb->s_xattr would be NULL.
> 
> Fix these things and then get rid of the misleading and effectively
> already unused generic POSIX ACL handlers.
> 
> For most filesystems it is a trivial removal of the generic POSIX ACL
> handlers. Only for erofs, ext2, ext4, f2fs, jffs2, reiserfs, oc2fs the
> handler is used but rather easy to fix.
> 
> 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.
> 
> This branch depends on [1] which hopefully should be merged soon and can
> be pulled from [2] which already includes [1] so it's easy to test and
> compile.
> 
> With this all remnants of the old POSIX ACL xattr handling will be gone.
> 
> Thanks!
> Christian
> 
> [1]: https://lore.kernel.org/lkml/20230125100040.374709-1-brauner@kernel.org
> [2]: ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.acl.remove.generic.xattr.handlers.v1
> 
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> Christian Brauner (12):
>       xattr: simplify listxattr helpers
>       xattr, posix acl: add listxattr helpers
>       xattr: remove unused argument
>       fs: drop unused posix acl handlers
>       erofs: drop posix acl handlers
>       ext2: drop posix acl handlers
>       ext4: drop posix acl handlers
>       f2fs: drop posix acl handlers
>       jffs2: drop posix acl handlers
>       ocfs2: drop posix acl handlers
>       reiserfs: drop posix acl handlers
>       acl: remove posix acl handlers

I just realized that b4 dropped a patch when I created a series to
track. So this is missing an important bit. Christoph, Al, the
SB_I_XATTR bit is the best thing that I came up with so far. It works
well enough and I find the handler based logic weird anyway. But I'm
open to other ideas.

From e661c29b999f3d5d6293046ea1fe9e7d40cfa453 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 18 Jan 2023 10:00:00 +0100
Subject: [PATCH 01/14] fs: add SB_I_XATTR

Last cycle we introduced a dedicated POSIX ACLs api which only relies on
the associated inode operations. It does not depend on the xattr handler
infrastructure anymore.

However, a few filesystems still rely on the ->list() method of the
generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
This is a very limited set of filesystems. For most of them there is no
dependence on the generic POSIX ACL xattr handler in any way.

In addition, during inode initalization in inode_init_always() the
registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
inode->i_opflags.

With the incoming removal of the legacy POSIX ACL handlers it is at
least possible for a filesystem to only implement POSIX ACLs but no
other xattrs. If that were to happen we would miss to raise IOP_XATTR
because sb->s_xattr would be NULL. While there currently is no such
filesystem we should still make sure that this just works should it ever
happen in the future.

We could try and use raise IOP_XATTR if we detect that the filesystems
implements the associated inode operations. But this doesn't work at the
time inode_init_always() is called because the inode->i_op field is not
yet initialized with the correct filesystem inode operations.

We could of course make the filesystems responsible for raising IOP_XATR
but this is messy as quite a few filesystems initialize inode->i_op in
multiple locations meaning we would have to scatter the checks to raise
IOP_XATTR across the filesystems. This is doable but ugly.

Another possibility would be to just check for SB_POSIXACL in
sb->s_flags and then raise IOP_XATTR. However, this doesn't work as for
some filesystems this flag can be turned on or off. For example, both
btrfs and ext4 support the "acl" and "noacl" mount option to turn
SB_POSIXACL on or off.

So if the filesystem is mounted with "noacl" then we cannot use
SB_POSIXACL as it might not be raised because the filesystem temporarily
disabled POSIX ACLs.

We could try and get rid of IOP_XATTR but it is used in subtle ways to
communicate that a given inode doesn't support xattrs even though the
superblock does in general and is especially prevalent in the lsm world.
This could potentially be solved by having each inode have a separate
xattr handler field instead of the superblock but would bloat each inode
and would be a more substantial change.

The option that chosen here is to introduce SB_I_XATTR which is raised
in sb->s_iflags whenever the superblock supports xattrs. That
encompasses POSIX ACLs and handler based xattrs. The flag will become
even more useful once fscaps have a dedicated inode operation and we're
really not short of flags anyway.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/9p/vfs_super.c    | 2 ++
 fs/afs/super.c       | 4 +++-
 fs/btrfs/super.c     | 1 +
 fs/ceph/super.c      | 1 +
 fs/cifs/cifsfs.c     | 1 +
 fs/ecryptfs/main.c   | 1 +
 fs/erofs/super.c     | 1 +
 fs/ext2/super.c      | 1 +
 fs/ext4/super.c      | 1 +
 fs/f2fs/super.c      | 1 +
 fs/fuse/inode.c      | 4 ++++
 fs/gfs2/ops_fstype.c | 1 +
 fs/hfs/super.c       | 1 +
 fs/hfsplus/super.c   | 1 +
 fs/inode.c           | 2 +-
 fs/jffs2/super.c     | 1 +
 fs/jfs/super.c       | 1 +
 fs/kernfs/mount.c    | 1 +
 fs/libfs.c           | 2 ++
 fs/nfs/super.c       | 3 +++
 fs/ntfs3/super.c     | 1 +
 fs/ocfs2/super.c     | 1 +
 fs/orangefs/super.c  | 1 +
 fs/overlayfs/super.c | 2 +-
 fs/reiserfs/super.c  | 1 +
 fs/squashfs/super.c  | 1 +
 fs/ubifs/super.c     | 1 +
 fs/xfs/xfs_super.c   | 1 +
 include/linux/fs.h   | 2 ++
 mm/shmem.c           | 2 ++
 30 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 266c4693e20c..003191b6ad97 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -93,6 +93,8 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 		sb->s_flags |= SB_POSIXACL;
 #endif
 
+	sb->s_iflags |= SB_I_XATTR;
+
 	return 0;
 }
 
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..379e6d7ac827 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -457,8 +457,10 @@ static int afs_fill_super(struct super_block *sb, struct afs_fs_context *ctx)
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_magic		= AFS_FS_MAGIC;
 	sb->s_op		= &afs_super_ops;
-	if (!as->dyn_root)
+	if (!as->dyn_root) {
 		sb->s_xattr	= afs_xattr_handlers;
+		sb->s_iflags	|= SB_I_XATTR;
+	}
 	ret = super_setup_bdi(sb);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f52ee85f6f..9af31008aafa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1141,6 +1141,7 @@ static int btrfs_fill_super(struct super_block *sb,
 #endif
 	sb->s_flags |= SB_I_VERSION;
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	err = super_setup_bdi(sb);
 	if (err) {
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 3fc48b43cab0..5aeb4fa0c4b0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1125,6 +1125,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
+	s->s_iflags |= SB_I_XATTR;
 
 	ret = set_anon_super_fc(s, fc);
 	if (ret != 0)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 10e00c624922..6a938b93d75d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -235,6 +235,7 @@ cifs_read_super(struct super_block *sb)
 	sb->s_magic = CIFS_SUPER_MAGIC;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	rc = super_setup_bdi(sb);
 	if (rc)
 		goto out_no_root;
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067f..4ae69d23bc47 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -521,6 +521,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	sbi = NULL;
 	s->s_op = &ecryptfs_sops;
 	s->s_xattr = ecryptfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 	s->s_d_op = &ecryptfs_dops;
 
 	err = "Reading sb failed";
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 481788c24a68..ea349cea9229 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -772,6 +772,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	sb->s_time_gran = 1;
 	sb->s_xattr = erofs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &erofs_export_ops;
 
 	if (test_opt(&sbi->opt, POSIX_ACL))
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 69c88facfe90..ee6e0c4978d1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1171,6 +1171,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ext2_sops;
 	sb->s_export_op = &ext2_export_ops;
 	sb->s_xattr = ext2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..2706930beed2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5104,6 +5104,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 
 	/* i_version is always enabled now */
 	sb->s_flags |= SB_I_VERSION;
+	sb->s_iflags |= SB_I_XATTR;
 
 	if (ext4_check_feature_compatibility(sb, es, silent))
 		goto failed_mount;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1f812b9ce985..6fff82f0efdf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4217,6 +4217,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
 	memcpy(&sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 	sb->s_iflags |= SB_I_CGROUPWB;
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* init f2fs-specific super block info */
 	sbi->valid_super_block = valid_super_block;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index de9b9ec5ce81..c9952bb9f9d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1181,6 +1181,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			if ((flags & FUSE_POSIX_ACL)) {
 				fc->default_permissions = 1;
 				fc->posix_acl = 1;
+				fm->sb->s_iflags |= SB_I_XATTR;
 			}
 			if (flags & FUSE_CACHE_SYMLINKS)
 				fc->cache_symlinks = 1;
@@ -1426,6 +1427,7 @@ static void fuse_sb_defaults(struct super_block *sb)
 	if (sb->s_user_ns != &init_user_ns)
 		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 	sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
+	sb->s_iflags |= SB_I_XATTR;
 }
 
 static int fuse_fill_super_submount(struct super_block *sb,
@@ -1443,6 +1445,8 @@ static int fuse_fill_super_submount(struct super_block *sb,
 	sb->s_bdi = bdi_get(parent_sb->s_bdi);
 
 	sb->s_xattr = parent_sb->s_xattr;
+	if (sb->s_xattr)
+		sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = parent_sb->s_time_gran;
 	sb->s_blocksize = parent_sb->s_blocksize;
 	sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c0cf1d2d0ef5..71c4fa1cdc1c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -506,6 +506,7 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
 	default:
 		BUG();
 	}
+	sb->s_iflags |= SB_I_XATTR;
 
 	/* Set up the buffer cache and SB for real */
 	if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6f..6220ce2af0d0 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -401,6 +401,7 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
+	sb->s_iflags |= SB_I_XATTR;
 	mutex_init(&sbi->bitmap_lock);
 
 	res = hfs_mdb_get(sb);
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89ebf9f..c5dd14057a78 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -490,6 +490,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE);
 	}
 	sb->s_xattr = hfsplus_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 
 	inode = hfsplus_iget(sb, HFSPLUS_ALLOC_CNID);
 	if (IS_ERR(inode)) {
diff --git a/fs/inode.c b/fs/inode.c
index f453eb58fd03..51824bde0b35 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -169,7 +169,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_ino = 0;
 	inode->__i_nlink = 1;
 	inode->i_opflags = 0;
-	if (sb->s_xattr)
+	if (sb->s_iflags & SB_I_XATTR)
 		inode->i_opflags |= IOP_XATTR;
 	i_uid_write(inode, 0);
 	i_gid_write(inode, 0);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..1ae1ee573280 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,6 +285,7 @@ static int jffs2_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	sb->s_iflags |= SB_I_XATTR;
 	return jffs2_do_fill_super(sb, fc);
 }
 
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index d2f82cb7db1b..b78042d3f3c0 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -532,6 +532,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &jfs_super_operations;
 	sb->s_export_op = &jfs_export_operations;
 	sb->s_xattr = jfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
 	sb->s_qcop = &jfs_quotactl_ops;
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index e08e8d999807..cd3391419692 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -251,6 +251,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
 	sb->s_magic = kfc->magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	if (info->root->flags & KERNFS_ROOT_SUPPORT_EXPORTOP)
 		sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..bd219120f248 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -328,6 +328,8 @@ static int pseudo_fs_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ctx->magic;
 	s->s_op = ctx->ops ?: &simple_super_operations;
 	s->s_xattr = ctx->xattr;
+	if (s->s_xattr)
+		s->s_iflags |= SB_I_XATTR;
 	s->s_time_gran = 1;
 	root = new_inode(s);
 	if (!root)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 05ae23657527..23c746745129 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1077,6 +1077,9 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
 		break;
 	}
 
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
+
 	sb->s_magic = NFS_SUPER_MAGIC;
 
 	/* We probably want something more informative here */
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index ef4ea3f21905..93e66112fac5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -950,6 +950,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_export_op = &ntfs_export_ops;
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_d_op = sbi->options->nocase ? &ntfs_dentry_ops : NULL;
 
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0b0e6a132101..514f5bcc9722 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -2020,6 +2020,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	sb->dq_op = &ocfs2_quota_operations;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 	sb->s_xattr = ocfs2_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_time_gran = 1;
 	sb->s_flags |= SB_NOATIME;
 	/* this is needed to support O_LARGEFILE */
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d..ec3e2d4ddb92 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -433,6 +433,7 @@ static int orangefs_fill_sb(struct super_block *sb,
 
 	/* Hang the xattr handlers off the superblock */
 	sb->s_xattr = orangefs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_magic = ORANGEFS_SUPER_MAGIC;
 	sb->s_op = &orangefs_s_ops;
 	sb->s_d_op = &orangefs_dentry_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..1002c8f332b9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -2060,7 +2060,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
 	sb->s_flags |= SB_POSIXACL;
-	sb->s_iflags |= SB_I_SKIP_SYNC;
+	sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_XATTR;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e731..1655b384e0b4 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2041,6 +2041,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 
 	s->s_xattr = reiserfs_xattr_handlers;
+	s->s_iflags |= SB_I_XATTR;
 
 	if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) {
 		SWARN(silent, s, "clm-7000",
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index e090fae48e68..7ab4fbc1f663 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -338,6 +338,7 @@ static int squashfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Handle xattrs */
 	sb->s_xattr = squashfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	xattr_id_table_start = le64_to_cpu(sblk->xattr_id_table_start);
 	if (xattr_id_table_start == SQUASHFS_INVALID_BLK) {
 		next_table = msblk->bytes_used;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index d0c9a09988bc..be9e96cf9d5a 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2215,6 +2215,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	fscrypt_set_ops(sb, &ubifs_crypt_operations);
 
 	mutex_lock(&c->umount_mutex);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0c4b73e9b29d..5a94d3eeab8a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1461,6 +1461,7 @@ xfs_fs_fill_super(
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_iflags |= SB_I_XATTR;
 	sb->s_export_op = &xfs_export_operations;
 #ifdef CONFIG_XFS_QUOTA
 	sb->s_qcop = &xfs_quotactl_operations;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 066555ad1bf8..700be4b969ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1449,6 +1449,8 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_TS_EXPIRY_WARNED 0x00000400 /* warned about timestamp range expiry */
 #define SB_I_RETIRED	0x00000800	/* superblock shouldn't be reused */
 
+#define SB_I_XATTR	0x00001000	/* superblock supports xattrs */
+
 /* Possible states of 'frozen' field */
 enum {
 	SB_UNFROZEN = 0,		/* FS is unfrozen */
diff --git a/mm/shmem.c b/mm/shmem.c
index c301487be5fb..f9604b64624e 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3817,6 +3817,8 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc)
 #ifdef CONFIG_TMPFS_POSIX_ACL
 	sb->s_flags |= SB_POSIXACL;
 #endif
+	if (sb->s_xattr || (sb->s_flags & SB_POSIXACL))
+		sb->s_iflags |= SB_I_XATTR;
 	uuid_gen(&sb->s_uuid);
 
 	inode = shmem_get_inode(sb, NULL, S_IFDIR | sbinfo->mode, 0, VM_NORESERVE);
-- 
2.34.1


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> However, a few filesystems still rely on the ->list() method of the
> generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> This is a very limited set of filesystems. For most of them there is no
> dependence on the generic POSIX ACL xattr handler in any way.
> 
> In addition, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the incoming removal of the legacy POSIX ACL handlers it is at
> least possible for a filesystem to only implement POSIX ACLs but no
> other xattrs. If that were to happen we would miss to raise IOP_XATTR
> because sb->s_xattr would be NULL. While there currently is no such
> filesystem we should still make sure that this just works should it ever
> happen in the future.

Now the real questions is: do we care?  Once Posix ACLs use an
entirely separate path, nothing should rely on IOP_XATTR for them.
So instead I think we're better off auditing all users of IOP_XATTR
and making sure that nothing relies on them for ACLs, as we've very
much split the VFS concept of ACLs from that from xattrs otherwise.

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> However, a few filesystems still rely on the ->list() method of the
> generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> This is a very limited set of filesystems. For most of them there is no
> dependence on the generic POSIX ACL xattr handler in any way.
> 
> In addition, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the incoming removal of the legacy POSIX ACL handlers it is at
> least possible for a filesystem to only implement POSIX ACLs but no
> other xattrs. If that were to happen we would miss to raise IOP_XATTR
> because sb->s_xattr would be NULL. While there currently is no such
> filesystem we should still make sure that this just works should it ever
> happen in the future.

Now the real questions is: do we care?  Once Posix ACLs use an
entirely separate path, nothing should rely on IOP_XATTR for them.
So instead I think we're better off auditing all users of IOP_XATTR
and making sure that nothing relies on them for ACLs, as we've very
much split the VFS concept of ACLs from that from xattrs otherwise.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> However, a few filesystems still rely on the ->list() method of the
> generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> This is a very limited set of filesystems. For most of them there is no
> dependence on the generic POSIX ACL xattr handler in any way.
> 
> In addition, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the incoming removal of the legacy POSIX ACL handlers it is at
> least possible for a filesystem to only implement POSIX ACLs but no
> other xattrs. If that were to happen we would miss to raise IOP_XATTR
> because sb->s_xattr would be NULL. While there currently is no such
> filesystem we should still make sure that this just works should it ever
> happen in the future.

Now the real questions is: do we care?  Once Posix ACLs use an
entirely separate path, nothing should rely on IOP_XATTR for them.
So instead I think we're better off auditing all users of IOP_XATTR
and making sure that nothing relies on them for ACLs, as we've very
much split the VFS concept of ACLs from that from xattrs otherwise.

Re: [f2fs-dev] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christoph Hellwig]

On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> However, a few filesystems still rely on the ->list() method of the
> generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> This is a very limited set of filesystems. For most of them there is no
> dependence on the generic POSIX ACL xattr handler in any way.
> 
> In addition, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the incoming removal of the legacy POSIX ACL handlers it is at
> least possible for a filesystem to only implement POSIX ACLs but no
> other xattrs. If that were to happen we would miss to raise IOP_XATTR
> because sb->s_xattr would be NULL. While there currently is no such
> filesystem we should still make sure that this just works should it ever
> happen in the future.

Now the real questions is: do we care?  Once Posix ACLs use an
entirely separate path, nothing should rely on IOP_XATTR for them.
So instead I think we're better off auditing all users of IOP_XATTR
and making sure that nothing relies on them for ACLs, as we've very
much split the VFS concept of ACLs from that from xattrs otherwise.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [Ocfs2-devel] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christoph Hellwig via Ocfs2-devel]

On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> However, a few filesystems still rely on the ->list() method of the
> generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> This is a very limited set of filesystems. For most of them there is no
> dependence on the generic POSIX ACL xattr handler in any way.
> 
> In addition, during inode initalization in inode_init_always() the
> registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> inode->i_opflags.
> 
> With the incoming removal of the legacy POSIX ACL handlers it is at
> least possible for a filesystem to only implement POSIX ACLs but no
> other xattrs. If that were to happen we would miss to raise IOP_XATTR
> because sb->s_xattr would be NULL. While there currently is no such
> filesystem we should still make sure that this just works should it ever
> happen in the future.

Now the real questions is: do we care?  Once Posix ACLs use an
entirely separate path, nothing should rely on IOP_XATTR for them.
So instead I think we're better off auditing all users of IOP_XATTR
and making sure that nothing relies on them for ACLs, as we've very
much split the VFS concept of ACLs from that from xattrs otherwise.

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 10:16:15AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> > However, a few filesystems still rely on the ->list() method of the
> > generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> > This is a very limited set of filesystems. For most of them there is no
> > dependence on the generic POSIX ACL xattr handler in any way.
> > 
> > In addition, during inode initalization in inode_init_always() the
> > registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> > inode->i_opflags.
> > 
> > With the incoming removal of the legacy POSIX ACL handlers it is at
> > least possible for a filesystem to only implement POSIX ACLs but no
> > other xattrs. If that were to happen we would miss to raise IOP_XATTR
> > because sb->s_xattr would be NULL. While there currently is no such
> > filesystem we should still make sure that this just works should it ever
> > happen in the future.
> 
> Now the real questions is: do we care?  Once Posix ACLs use an
> entirely separate path, nothing should rely on IOP_XATTR for them.
> So instead I think we're better off auditing all users of IOP_XATTR
> and making sure that nothing relies on them for ACLs, as we've very
> much split the VFS concept of ACLs from that from xattrs otherwise.

I had a patch like that but some filesystems create inodes that
explicitly remove IOP_XATTR to prevent any xattrs from being set on
specific inodes. I remember this for at least reiserfs and btrfs.

So we would probably need IOP_NOACL that can be raised by a filesystem
to explicitly opt out of them for specific inodes. That's probably fine
and avoids having to introduce something like SB_I_XATTR.

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 10:16:15AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> > However, a few filesystems still rely on the ->list() method of the
> > generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> > This is a very limited set of filesystems. For most of them there is no
> > dependence on the generic POSIX ACL xattr handler in any way.
> > 
> > In addition, during inode initalization in inode_init_always() the
> > registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> > inode->i_opflags.
> > 
> > With the incoming removal of the legacy POSIX ACL handlers it is at
> > least possible for a filesystem to only implement POSIX ACLs but no
> > other xattrs. If that were to happen we would miss to raise IOP_XATTR
> > because sb->s_xattr would be NULL. While there currently is no such
> > filesystem we should still make sure that this just works should it ever
> > happen in the future.
> 
> Now the real questions is: do we care?  Once Posix ACLs use an
> entirely separate path, nothing should rely on IOP_XATTR for them.
> So instead I think we're better off auditing all users of IOP_XATTR
> and making sure that nothing relies on them for ACLs, as we've very
> much split the VFS concept of ACLs from that from xattrs otherwise.

I had a patch like that but some filesystems create inodes that
explicitly remove IOP_XATTR to prevent any xattrs from being set on
specific inodes. I remember this for at least reiserfs and btrfs.

So we would probably need IOP_NOACL that can be raised by a filesystem
to explicitly opt out of them for specific inodes. That's probably fine
and avoids having to introduce something like SB_I_XATTR.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 10:16:15AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> > However, a few filesystems still rely on the ->list() method of the
> > generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> > This is a very limited set of filesystems. For most of them there is no
> > dependence on the generic POSIX ACL xattr handler in any way.
> > 
> > In addition, during inode initalization in inode_init_always() the
> > registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> > inode->i_opflags.
> > 
> > With the incoming removal of the legacy POSIX ACL handlers it is at
> > least possible for a filesystem to only implement POSIX ACLs but no
> > other xattrs. If that were to happen we would miss to raise IOP_XATTR
> > because sb->s_xattr would be NULL. While there currently is no such
> > filesystem we should still make sure that this just works should it ever
> > happen in the future.
> 
> Now the real questions is: do we care?  Once Posix ACLs use an
> entirely separate path, nothing should rely on IOP_XATTR for them.
> So instead I think we're better off auditing all users of IOP_XATTR
> and making sure that nothing relies on them for ACLs, as we've very
> much split the VFS concept of ACLs from that from xattrs otherwise.

I had a patch like that but some filesystems create inodes that
explicitly remove IOP_XATTR to prevent any xattrs from being set on
specific inodes. I remember this for at least reiserfs and btrfs.

So we would probably need IOP_NOACL that can be raised by a filesystem
to explicitly opt out of them for specific inodes. That's probably fine
and avoids having to introduce something like SB_I_XATTR.

Re: [f2fs-dev] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner]

On Mon, Jan 30, 2023 at 10:16:15AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> > However, a few filesystems still rely on the ->list() method of the
> > generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> > This is a very limited set of filesystems. For most of them there is no
> > dependence on the generic POSIX ACL xattr handler in any way.
> > 
> > In addition, during inode initalization in inode_init_always() the
> > registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> > inode->i_opflags.
> > 
> > With the incoming removal of the legacy POSIX ACL handlers it is at
> > least possible for a filesystem to only implement POSIX ACLs but no
> > other xattrs. If that were to happen we would miss to raise IOP_XATTR
> > because sb->s_xattr would be NULL. While there currently is no such
> > filesystem we should still make sure that this just works should it ever
> > happen in the future.
> 
> Now the real questions is: do we care?  Once Posix ACLs use an
> entirely separate path, nothing should rely on IOP_XATTR for them.
> So instead I think we're better off auditing all users of IOP_XATTR
> and making sure that nothing relies on them for ACLs, as we've very
> much split the VFS concept of ACLs from that from xattrs otherwise.

I had a patch like that but some filesystems create inodes that
explicitly remove IOP_XATTR to prevent any xattrs from being set on
specific inodes. I remember this for at least reiserfs and btrfs.

So we would probably need IOP_NOACL that can be raised by a filesystem
to explicitly opt out of them for specific inodes. That's probably fine
and avoids having to introduce something like SB_I_XATTR.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [Ocfs2-devel] [PATCH 00/12] acl: remove remaining posix acl handlers

[Christian Brauner via Ocfs2-devel]

On Mon, Jan 30, 2023 at 10:16:15AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 10:10:52AM +0100, Christian Brauner wrote:
> > However, a few filesystems still rely on the ->list() method of the
> > generix POSIX ACL xattr handlers in their ->listxattr() inode operation.
> > This is a very limited set of filesystems. For most of them there is no
> > dependence on the generic POSIX ACL xattr handler in any way.
> > 
> > In addition, during inode initalization in inode_init_always() the
> > registered xattr handlers in sb->s_xattr are used to raise IOP_XATTR in
> > inode->i_opflags.
> > 
> > With the incoming removal of the legacy POSIX ACL handlers it is at
> > least possible for a filesystem to only implement POSIX ACLs but no
> > other xattrs. If that were to happen we would miss to raise IOP_XATTR
> > because sb->s_xattr would be NULL. While there currently is no such
> > filesystem we should still make sure that this just works should it ever
> > happen in the future.
> 
> Now the real questions is: do we care?  Once Posix ACLs use an
> entirely separate path, nothing should rely on IOP_XATTR for them.
> So instead I think we're better off auditing all users of IOP_XATTR
> and making sure that nothing relies on them for ACLs, as we've very
> much split the VFS concept of ACLs from that from xattrs otherwise.

I had a patch like that but some filesystems create inodes that
explicitly remove IOP_XATTR to prevent any xattrs from being set on
specific inodes. I remember this for at least reiserfs and btrfs.

So we would probably need IOP_NOACL that can be raised by a filesystem
to explicitly opt out of them for specific inodes. That's probably fine
and avoids having to introduce something like SB_I_XATTR.

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel