All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-01-30 16:41 ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd,
	reiserfs-devel

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handlers around in the
filesystems xattr handler registered at sb->s_xattr for two reasons.
First, because a few filesystems rely 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.

This series adresses both issues and makes it possible to remove the
generic POSIX ACL xattr handlers from the sb->s_xattr list of all
filesystems. This is a crucial step as the generic POSIX ACL xattr
handlers aren't used for POSIX ACLs anymore. They are never used apart
from the two cases above.

To fix this we make POSIX ACLs independent of IOP_XATTR. For filesystems
like btrfs or reiserfs that want to disable xattrs and POSIX ACLs for
some inodes we give them a dedicated IOP_NOACL flag they can raise as
discussed in the previous iteration.

The series also simplifies a the ->listxattr() implementation for all
filesystems that rely on the ->list() method of the xattr handlers.

After this we've removed the generic POSIX ACL xattr handlers completely
from sb->s_xattr.

All filesystems with reasonable integration into xfstests have been
tested with:

        ./check -g acl,attr,cap,idmapped,io_uring,perms,unlink

All tests pass without regression on xfstests for-next branch.

Since erofs doesn't have integration into xfstests yet afaict I have
tested it with the testuite available in erofs-utils. They also all pass
without any regressions.

Thanks!
Christian

[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>
---
Changes in v2:
- Please see changelogs of the individual patches.
- Christoph & Christian:
  Remove SB_I_XATTR and instead introduce IOP_NOACL so filesystems can
  opt out of POSIX ACLs for specific inodes. Decouple POSIX ACLs from
  IOP_XATTR.
- Keep generic posix acl xattr handlers so filesystems that use array
  based indexing on xattr handlers can continue to do so.
- Minor fixes.
- Link to v1: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-0-6cf155b492b6@kernel.org

---
Christian Brauner (8):
      fs: don't use IOP_XATTR for posix acls
      xattr: simplify listxattr helpers
      xattr: add listxattr helper
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      fs: simplify ->listxattr() implementation
      reiserfs: rework ->listxattr() implementation
      fs: rename generic posix acl handlers

 fs/9p/xattr.c                   |   4 --
 fs/bad_inode.c                  |   3 +-
 fs/btrfs/inode.c                |   2 +-
 fs/btrfs/xattr.c                |   4 --
 fs/ceph/xattr.c                 |   4 --
 fs/cifs/xattr.c                 |   4 --
 fs/ecryptfs/inode.c             |   4 --
 fs/erofs/xattr.c                |  12 +----
 fs/erofs/xattr.h                |  20 +++++---
 fs/ext2/xattr.c                 |  25 +++++-----
 fs/ext4/xattr.c                 |  25 +++++-----
 fs/f2fs/xattr.c                 |  24 +++++-----
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  29 ++++++------
 fs/jfs/xattr.c                  |   4 --
 fs/libfs.c                      |   3 +-
 fs/nfs/nfs3_fs.h                |   1 -
 fs/nfs/nfs3acl.c                |   6 ---
 fs/nfs/nfs3super.c              |   3 --
 fs/nfsd/nfs4xdr.c               |   3 +-
 fs/ntfs3/xattr.c                |   4 --
 fs/ocfs2/xattr.c                |  14 ++----
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/copy_up.c          |   4 +-
 fs/overlayfs/super.c            |   8 ----
 fs/posix_acl.c                  |  53 +++++++++++++++++----
 fs/reiserfs/inode.c             |   2 +-
 fs/reiserfs/namei.c             |   4 +-
 fs/reiserfs/reiserfs.h          |   2 +-
 fs/reiserfs/xattr.c             |  74 +++++++++++++++--------------
 fs/xattr.c                      | 101 +++++++++++++++-------------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/fs.h              |   1 +
 include/linux/posix_acl.h       |   7 +++
 include/linux/posix_acl_xattr.h |   5 +-
 include/linux/xattr.h           |  30 +++++++++++-
 mm/shmem.c                      |   4 --
 37 files changed, 245 insertions(+), 256 deletions(-)
---
base-commit: ab072681eabe1ce0a9a32d4baa1a27a2d046bc4a
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88


^ permalink raw reply	[flat|nested] 31+ messages in thread
* [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-01-30 16:41 ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Christian Brauner (Microsoft),
	reiserfs-devel, linux-f2fs-devel, linux-mtd, Al Viro, linux-ext4,
	linux-erofs, Seth Forshee

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handlers around in the
filesystems xattr handler registered at sb->s_xattr for two reasons.
First, because a few filesystems rely 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.

This series adresses both issues and makes it possible to remove the
generic POSIX ACL xattr handlers from the sb->s_xattr list of all
filesystems. This is a crucial step as the generic POSIX ACL xattr
handlers aren't used for POSIX ACLs anymore. They are never used apart
from the two cases above.

To fix this we make POSIX ACLs independent of IOP_XATTR. For filesystems
like btrfs or reiserfs that want to disable xattrs and POSIX ACLs for
some inodes we give them a dedicated IOP_NOACL flag they can raise as
discussed in the previous iteration.

The series also simplifies a the ->listxattr() implementation for all
filesystems that rely on the ->list() method of the xattr handlers.

After this we've removed the generic POSIX ACL xattr handlers completely
from sb->s_xattr.

All filesystems with reasonable integration into xfstests have been
tested with:

        ./check -g acl,attr,cap,idmapped,io_uring,perms,unlink

All tests pass without regression on xfstests for-next branch.

Since erofs doesn't have integration into xfstests yet afaict I have
tested it with the testuite available in erofs-utils. They also all pass
without any regressions.

Thanks!
Christian

[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>
---
Changes in v2:
- Please see changelogs of the individual patches.
- Christoph & Christian:
  Remove SB_I_XATTR and instead introduce IOP_NOACL so filesystems can
  opt out of POSIX ACLs for specific inodes. Decouple POSIX ACLs from
  IOP_XATTR.
- Keep generic posix acl xattr handlers so filesystems that use array
  based indexing on xattr handlers can continue to do so.
- Minor fixes.
- Link to v1: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-0-6cf155b492b6@kernel.org

---
Christian Brauner (8):
      fs: don't use IOP_XATTR for posix acls
      xattr: simplify listxattr helpers
      xattr: add listxattr helper
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      fs: simplify ->listxattr() implementation
      reiserfs: rework ->listxattr() implementation
      fs: rename generic posix acl handlers

 fs/9p/xattr.c                   |   4 --
 fs/bad_inode.c                  |   3 +-
 fs/btrfs/inode.c                |   2 +-
 fs/btrfs/xattr.c                |   4 --
 fs/ceph/xattr.c                 |   4 --
 fs/cifs/xattr.c                 |   4 --
 fs/ecryptfs/inode.c             |   4 --
 fs/erofs/xattr.c                |  12 +----
 fs/erofs/xattr.h                |  20 +++++---
 fs/ext2/xattr.c                 |  25 +++++-----
 fs/ext4/xattr.c                 |  25 +++++-----
 fs/f2fs/xattr.c                 |  24 +++++-----
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  29 ++++++------
 fs/jfs/xattr.c                  |   4 --
 fs/libfs.c                      |   3 +-
 fs/nfs/nfs3_fs.h                |   1 -
 fs/nfs/nfs3acl.c                |   6 ---
 fs/nfs/nfs3super.c              |   3 --
 fs/nfsd/nfs4xdr.c               |   3 +-
 fs/ntfs3/xattr.c                |   4 --
 fs/ocfs2/xattr.c                |  14 ++----
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/copy_up.c          |   4 +-
 fs/overlayfs/super.c            |   8 ----
 fs/posix_acl.c                  |  53 +++++++++++++++++----
 fs/reiserfs/inode.c             |   2 +-
 fs/reiserfs/namei.c             |   4 +-
 fs/reiserfs/reiserfs.h          |   2 +-
 fs/reiserfs/xattr.c             |  74 +++++++++++++++--------------
 fs/xattr.c                      | 101 +++++++++++++++-------------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/fs.h              |   1 +
 include/linux/posix_acl.h       |   7 +++
 include/linux/posix_acl_xattr.h |   5 +-
 include/linux/xattr.h           |  30 +++++++++++-
 mm/shmem.c                      |   4 --
 37 files changed, 245 insertions(+), 256 deletions(-)
---
base-commit: ab072681eabe1ce0a9a32d4baa1a27a2d046bc4a
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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-01-30 16:41 ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Christian Brauner (Microsoft),
	reiserfs-devel, linux-f2fs-devel, linux-mtd, Al Viro, linux-ext4,
	linux-erofs, Seth Forshee

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handlers around in the
filesystems xattr handler registered at sb->s_xattr for two reasons.
First, because a few filesystems rely 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.

This series adresses both issues and makes it possible to remove the
generic POSIX ACL xattr handlers from the sb->s_xattr list of all
filesystems. This is a crucial step as the generic POSIX ACL xattr
handlers aren't used for POSIX ACLs anymore. They are never used apart
from the two cases above.

To fix this we make POSIX ACLs independent of IOP_XATTR. For filesystems
like btrfs or reiserfs that want to disable xattrs and POSIX ACLs for
some inodes we give them a dedicated IOP_NOACL flag they can raise as
discussed in the previous iteration.

The series also simplifies a the ->listxattr() implementation for all
filesystems that rely on the ->list() method of the xattr handlers.

After this we've removed the generic POSIX ACL xattr handlers completely
from sb->s_xattr.

All filesystems with reasonable integration into xfstests have been
tested with:

        ./check -g acl,attr,cap,idmapped,io_uring,perms,unlink

All tests pass without regression on xfstests for-next branch.

Since erofs doesn't have integration into xfstests yet afaict I have
tested it with the testuite available in erofs-utils. They also all pass
without any regressions.

Thanks!
Christian

[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>
---
Changes in v2:
- Please see changelogs of the individual patches.
- Christoph & Christian:
  Remove SB_I_XATTR and instead introduce IOP_NOACL so filesystems can
  opt out of POSIX ACLs for specific inodes. Decouple POSIX ACLs from
  IOP_XATTR.
- Keep generic posix acl xattr handlers so filesystems that use array
  based indexing on xattr handlers can continue to do so.
- Minor fixes.
- Link to v1: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-0-6cf155b492b6@kernel.org

---
Christian Brauner (8):
      fs: don't use IOP_XATTR for posix acls
      xattr: simplify listxattr helpers
      xattr: add listxattr helper
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      fs: simplify ->listxattr() implementation
      reiserfs: rework ->listxattr() implementation
      fs: rename generic posix acl handlers

 fs/9p/xattr.c                   |   4 --
 fs/bad_inode.c                  |   3 +-
 fs/btrfs/inode.c                |   2 +-
 fs/btrfs/xattr.c                |   4 --
 fs/ceph/xattr.c                 |   4 --
 fs/cifs/xattr.c                 |   4 --
 fs/ecryptfs/inode.c             |   4 --
 fs/erofs/xattr.c                |  12 +----
 fs/erofs/xattr.h                |  20 +++++---
 fs/ext2/xattr.c                 |  25 +++++-----
 fs/ext4/xattr.c                 |  25 +++++-----
 fs/f2fs/xattr.c                 |  24 +++++-----
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  29 ++++++------
 fs/jfs/xattr.c                  |   4 --
 fs/libfs.c                      |   3 +-
 fs/nfs/nfs3_fs.h                |   1 -
 fs/nfs/nfs3acl.c                |   6 ---
 fs/nfs/nfs3super.c              |   3 --
 fs/nfsd/nfs4xdr.c               |   3 +-
 fs/ntfs3/xattr.c                |   4 --
 fs/ocfs2/xattr.c                |  14 ++----
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/copy_up.c          |   4 +-
 fs/overlayfs/super.c            |   8 ----
 fs/posix_acl.c                  |  53 +++++++++++++++++----
 fs/reiserfs/inode.c             |   2 +-
 fs/reiserfs/namei.c             |   4 +-
 fs/reiserfs/reiserfs.h          |   2 +-
 fs/reiserfs/xattr.c             |  74 +++++++++++++++--------------
 fs/xattr.c                      | 101 +++++++++++++++-------------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/fs.h              |   1 +
 include/linux/posix_acl.h       |   7 +++
 include/linux/posix_acl_xattr.h |   5 +-
 include/linux/xattr.h           |  30 +++++++++++-
 mm/shmem.c                      |   4 --
 37 files changed, 245 insertions(+), 256 deletions(-)
---
base-commit: ab072681eabe1ce0a9a32d4baa1a27a2d046bc4a
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88


^ permalink raw reply	[flat|nested] 31+ messages in thread
* [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-01-30 16:41 ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd,
	reiserfs-devel

Hey everyone,

after we finished the introduction of the new posix acl api last cycle
we still left the generic POSIX ACL xattr handlers around in the
filesystems xattr handler registered at sb->s_xattr for two reasons.
First, because a few filesystems rely 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.

This series adresses both issues and makes it possible to remove the
generic POSIX ACL xattr handlers from the sb->s_xattr list of all
filesystems. This is a crucial step as the generic POSIX ACL xattr
handlers aren't used for POSIX ACLs anymore. They are never used apart
from the two cases above.

To fix this we make POSIX ACLs independent of IOP_XATTR. For filesystems
like btrfs or reiserfs that want to disable xattrs and POSIX ACLs for
some inodes we give them a dedicated IOP_NOACL flag they can raise as
discussed in the previous iteration.

The series also simplifies a the ->listxattr() implementation for all
filesystems that rely on the ->list() method of the xattr handlers.

After this we've removed the generic POSIX ACL xattr handlers completely
from sb->s_xattr.

All filesystems with reasonable integration into xfstests have been
tested with:

        ./check -g acl,attr,cap,idmapped,io_uring,perms,unlink

All tests pass without regression on xfstests for-next branch.

Since erofs doesn't have integration into xfstests yet afaict I have
tested it with the testuite available in erofs-utils. They also all pass
without any regressions.

Thanks!
Christian

[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>
---
Changes in v2:
- Please see changelogs of the individual patches.
- Christoph & Christian:
  Remove SB_I_XATTR and instead introduce IOP_NOACL so filesystems can
  opt out of POSIX ACLs for specific inodes. Decouple POSIX ACLs from
  IOP_XATTR.
- Keep generic posix acl xattr handlers so filesystems that use array
  based indexing on xattr handlers can continue to do so.
- Minor fixes.
- Link to v1: https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v1-0-6cf155b492b6@kernel.org

---
Christian Brauner (8):
      fs: don't use IOP_XATTR for posix acls
      xattr: simplify listxattr helpers
      xattr: add listxattr helper
      xattr: remove unused argument
      fs: drop unused posix acl handlers
      fs: simplify ->listxattr() implementation
      reiserfs: rework ->listxattr() implementation
      fs: rename generic posix acl handlers

 fs/9p/xattr.c                   |   4 --
 fs/bad_inode.c                  |   3 +-
 fs/btrfs/inode.c                |   2 +-
 fs/btrfs/xattr.c                |   4 --
 fs/ceph/xattr.c                 |   4 --
 fs/cifs/xattr.c                 |   4 --
 fs/ecryptfs/inode.c             |   4 --
 fs/erofs/xattr.c                |  12 +----
 fs/erofs/xattr.h                |  20 +++++---
 fs/ext2/xattr.c                 |  25 +++++-----
 fs/ext4/xattr.c                 |  25 +++++-----
 fs/f2fs/xattr.c                 |  24 +++++-----
 fs/gfs2/xattr.c                 |   2 -
 fs/jffs2/xattr.c                |  29 ++++++------
 fs/jfs/xattr.c                  |   4 --
 fs/libfs.c                      |   3 +-
 fs/nfs/nfs3_fs.h                |   1 -
 fs/nfs/nfs3acl.c                |   6 ---
 fs/nfs/nfs3super.c              |   3 --
 fs/nfsd/nfs4xdr.c               |   3 +-
 fs/ntfs3/xattr.c                |   4 --
 fs/ocfs2/xattr.c                |  14 ++----
 fs/orangefs/xattr.c             |   2 -
 fs/overlayfs/copy_up.c          |   4 +-
 fs/overlayfs/super.c            |   8 ----
 fs/posix_acl.c                  |  53 +++++++++++++++++----
 fs/reiserfs/inode.c             |   2 +-
 fs/reiserfs/namei.c             |   4 +-
 fs/reiserfs/reiserfs.h          |   2 +-
 fs/reiserfs/xattr.c             |  74 +++++++++++++++--------------
 fs/xattr.c                      | 101 +++++++++++++++-------------------------
 fs/xfs/xfs_xattr.c              |   4 --
 include/linux/fs.h              |   1 +
 include/linux/posix_acl.h       |   7 +++
 include/linux/posix_acl_xattr.h |   5 +-
 include/linux/xattr.h           |  30 +++++++++++-
 mm/shmem.c                      |   4 --
 37 files changed, 245 insertions(+), 256 deletions(-)
---
base-commit: ab072681eabe1ce0a9a32d4baa1a27a2d046bc4a
change-id: 20230125-fs-acl-remove-generic-xattr-handlers-4cfed8558d88


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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (2 preceding siblings ...)
  (?)
@ 2023-01-30 16:41 ` Christian Brauner
  2023-01-30 16:50   ` Christoph Hellwig
  -1 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

The POSIX ACL api doesn't use the xattr handler infrastructure anymore.
If we keep relying on IOP_XATTR we will have to find a way to raise
IOP_XATTR during inode_init_always() if a filesystem doesn't implement
any xattrs other than POSIX ACLs. That's not done today but is in
principle possible. A prior version introduced SB_I_XATTR to this end.
Instead of this affecting all filesystems let those filesystems that
explicitly disable xattrs for some inodes disable POSIX ACLs by raising
IOP_NOACL.

Link: https://lore.kernel.org/linux-ext4/20230130091615.GB5178@lst.de
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Patch introduced.
---
 fs/bad_inode.c         |  3 ++-
 fs/btrfs/inode.c       |  2 +-
 fs/libfs.c             |  3 ++-
 fs/overlayfs/copy_up.c |  4 ++--
 fs/posix_acl.c         |  4 ++--
 fs/reiserfs/inode.c    |  2 +-
 fs/reiserfs/namei.c    |  4 ++--
 fs/reiserfs/xattr.c    |  4 ++--
 fs/xattr.c             |  2 +-
 include/linux/fs.h     |  1 +
 include/linux/xattr.h  | 11 +++++++++++
 11 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 92737166203f..524e5e35b5d6 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -16,6 +16,7 @@
 #include <linux/namei.h>
 #include <linux/poll.h>
 #include <linux/fiemap.h>
+#include <linux/xattr.h>
 
 static int bad_file_open(struct inode *inode, struct file *filp)
 {
@@ -212,7 +213,7 @@ void make_bad_inode(struct inode *inode)
 	inode->i_atime = inode->i_mtime = inode->i_ctime =
 		current_time(inode);
 	inode->i_op = &bad_inode_ops;	
-	inode->i_opflags &= ~IOP_XATTR;
+	inode_xattr_disable(inode);
 	inode->i_fop = &bad_file_ops;	
 }
 EXPORT_SYMBOL(make_bad_inode);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 98a800b8bd43..c015e554d186 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5840,7 +5840,7 @@ static struct inode *new_simple_dir(struct super_block *s,
 	 * associated with the dentry
 	 */
 	inode->i_op = &simple_dir_inode_operations;
-	inode->i_opflags &= ~IOP_XATTR;
+	inode_xattr_disable(inode);
 	inode->i_fop = &simple_dir_operations;
 	inode->i_mode = S_IFDIR | S_IRUGO | S_IWUSR | S_IXUGO;
 	inode->i_mtime = current_time(inode);
diff --git a/fs/libfs.c b/fs/libfs.c
index aada4e7c8713..78052d91d60f 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -23,6 +23,7 @@
 #include <linux/fsnotify.h>
 #include <linux/unicode.h>
 #include <linux/fscrypt.h>
+#include <linux/xattr.h>
 
 #include <linux/uaccess.h>
 
@@ -1375,7 +1376,7 @@ void make_empty_dir_inode(struct inode *inode)
 	inode->i_blocks = 0;
 
 	inode->i_op = &empty_dir_inode_operations;
-	inode->i_opflags &= ~IOP_XATTR;
+	inode_xattr_disable(inode);
 	inode->i_fop = &empty_dir_operations;
 }
 
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index c14e90764e35..0b48f0aa9558 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -81,8 +81,8 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct de
 	int error = 0;
 	size_t slen;
 
-	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
-	    !(new->d_inode->i_opflags & IOP_XATTR))
+	if (inode_xattr_disabled(old->d_inode) ||
+	    inode_xattr_disabled(new->d_inode))
 		return 0;
 
 	list_size = vfs_listxattr(old, NULL, 0);
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index d7bc81fc0840..315d3926a13a 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1095,7 +1095,7 @@ int vfs_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 	if (error)
 		goto out_inode_unlock;
 
-	if (inode->i_opflags & IOP_XATTR)
+	if (!(inode->i_opflags & IOP_NOACL))
 		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
 	else if (unlikely(is_bad_inode(inode)))
 		error = -EIO;
@@ -1205,7 +1205,7 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 	if (error)
 		goto out_inode_unlock;
 
-	if (inode->i_opflags & IOP_XATTR)
+	if (!(inode->i_opflags & IOP_NOACL))
 		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
 	else if (unlikely(is_bad_inode(inode)))
 		error = -EIO;
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index c7d1fa526dea..2a7037b165f0 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -2089,7 +2089,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 	 */
 	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
 		inode->i_flags |= S_PRIVATE;
-		inode->i_opflags &= ~IOP_XATTR;
+		inode_xattr_disable(inode);
 	}
 
 	if (reiserfs_posixacl(inode->i_sb)) {
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 0b8aa99749f1..4e2f121d6819 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -378,12 +378,12 @@ static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry,
 
 		/*
 		 * Propagate the private flag so we know we're
-		 * in the priv tree.  Also clear IOP_XATTR
+		 * in the priv tree.  Also clear xattrs
 		 * since we don't have xattrs on xattr files.
 		 */
 		if (IS_PRIVATE(dir)) {
 			inode->i_flags |= S_PRIVATE;
-			inode->i_opflags &= ~IOP_XATTR;
+			inode_xattr_disable(inode);
 		}
 	}
 	reiserfs_write_unlock(dir->i_sb);
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 8b2d52443f41..2c326b57d4bc 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -889,7 +889,7 @@ static int create_privroot(struct dentry *dentry)
 	}
 
 	d_inode(dentry)->i_flags |= S_PRIVATE;
-	d_inode(dentry)->i_opflags &= ~IOP_XATTR;
+	inode_xattr_disable(d_inode(dentry));
 	reiserfs_info(dentry->d_sb, "Created %s - reserved for xattr "
 		      "storage.\n", PRIVROOT_NAME);
 
@@ -977,7 +977,7 @@ int reiserfs_lookup_privroot(struct super_block *s)
 		d_set_d_op(dentry, &xattr_lookup_poison_ops);
 		if (d_really_is_positive(dentry)) {
 			d_inode(dentry)->i_flags |= S_PRIVATE;
-			d_inode(dentry)->i_opflags &= ~IOP_XATTR;
+			inode_xattr_disable(d_inode(dentry));
 		}
 	} else
 		err = PTR_ERR(dentry);
diff --git a/fs/xattr.c b/fs/xattr.c
index adab9a70b536..89b6c122056d 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -468,7 +468,7 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
 	error = security_inode_listxattr(dentry);
 	if (error)
 		return error;
-	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
+	if (inode->i_op->listxattr && !inode_xattr_disabled(inode)) {
 		error = inode->i_op->listxattr(dentry, list, size);
 	} else {
 		error = security_inode_listsecurity(inode, list, size);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c1769a2c5d70..f4cbac68598a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -582,6 +582,7 @@ is_uncached_acl(struct posix_acl *acl)
 #define IOP_NOFOLLOW	0x0004
 #define IOP_XATTR	0x0008
 #define IOP_DEFAULT_READLINK	0x0010
+#define IOP_NOACL	0x0020
 
 struct fsnotify_mark_connector;
 
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 2e7dd44926e4..23bbe98cfc16 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -109,5 +109,16 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
 			  char *buffer, size_t size);
 void simple_xattr_add(struct simple_xattrs *xattrs,
 		      struct simple_xattr *new_xattr);
+static inline void inode_xattr_disable(struct inode *inode)
+{
+	inode->i_opflags &= ~IOP_XATTR;
+	inode->i_opflags |= IOP_NOACL;
+}
+
+static inline bool inode_xattr_disabled(struct inode *inode)
+{
+	return !(inode->i_opflags & IOP_XATTR) &&
+	       (inode->i_opflags & IOP_NOACL);
+}
 
 #endif	/* _LINUX_XATTR_H */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 2/8] xattr: simplify listxattr helpers
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (3 preceding siblings ...)
  (?)
@ 2023-01-30 16:41 ` Christian Brauner
  2023-01-30 16:51   ` Christoph Hellwig
  -1 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

The generic_listxattr() and simple_xattr_list() helpers list xattrs and
contain duplicated code. Add two helpers that both generic_listxattr()
and simple_xattr_list() can use.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
  - Leave newline after variable declaration in xattr_list_one().
  - Move posix_acl_listxattr() into fs/posix_acl.c.
---
 fs/posix_acl.c            | 25 +++++++++++++
 fs/xattr.c                | 89 ++++++++++++++++++-----------------------------
 include/linux/posix_acl.h |  7 ++++
 include/linux/xattr.h     |  1 +
 4 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 315d3926a13a..d5e9db0e4d66 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -958,6 +958,31 @@ set_posix_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 }
 EXPORT_SYMBOL(set_posix_acl);
 
+int posix_acl_listxattr(struct inode *inode, char **buffer,
+			ssize_t *remaining_size)
+{
+	int err;
+
+	if (!IS_POSIXACL(inode))
+		return 0;
+
+	if (inode->i_acl) {
+		err = xattr_list_one(buffer, remaining_size,
+				     XATTR_NAME_POSIX_ACL_ACCESS);
+		if (err)
+			return err;
+	}
+
+	if (inode->i_default_acl) {
+		err = xattr_list_one(buffer, remaining_size,
+				     XATTR_NAME_POSIX_ACL_DEFAULT);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static bool
 posix_acl_xattr_list(struct dentry *dentry)
 {
diff --git a/fs/xattr.c b/fs/xattr.c
index 89b6c122056d..0e8ce76cb76a 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -949,6 +949,21 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 	return error;
 }
 
+int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
+{
+	size_t len;
+
+	len = strlen(name) + 1;
+	if (*buffer) {
+		if (*remaining_size < len)
+			return -ERANGE;
+		memcpy(*buffer, name, len);
+		*buffer += len;
+	}
+	*remaining_size -= len;
+	return 0;
+}
+
 /*
  * Combine the results of the list() operation from every xattr_handler in the
  * list.
@@ -957,33 +972,22 @@ ssize_t
 generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 {
 	const struct xattr_handler *handler, **handlers = dentry->d_sb->s_xattr;
-	unsigned int size = 0;
-
-	if (!buffer) {
-		for_each_xattr_handler(handlers, handler) {
-			if (!handler->name ||
-			    (handler->list && !handler->list(dentry)))
-				continue;
-			size += strlen(handler->name) + 1;
-		}
-	} else {
-		char *buf = buffer;
-		size_t len;
-
-		for_each_xattr_handler(handlers, handler) {
-			if (!handler->name ||
-			    (handler->list && !handler->list(dentry)))
-				continue;
-			len = strlen(handler->name);
-			if (len + 1 > buffer_size)
-				return -ERANGE;
-			memcpy(buf, handler->name, len + 1);
-			buf += len + 1;
-			buffer_size -= len + 1;
-		}
-		size = buf - buffer;
+	ssize_t remaining_size = buffer_size;
+	int err = 0;
+
+	err = posix_acl_listxattr(d_inode(dentry), &buffer, &remaining_size);
+	if (err)
+		return err;
+
+	for_each_xattr_handler(handlers, handler) {
+		if (!handler->name || (handler->list && !handler->list(dentry)))
+			continue;
+		err = xattr_list_one(&buffer, &remaining_size, handler->name);
+		if (err)
+			return err;
 	}
-	return size;
+
+	return err ? err : buffer_size - remaining_size;
 }
 EXPORT_SYMBOL(generic_listxattr);
 
@@ -1245,20 +1249,6 @@ static bool xattr_is_trusted(const char *name)
 	return !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN);
 }
 
-static int xattr_list_one(char **buffer, ssize_t *remaining_size,
-			  const char *name)
-{
-	size_t len = strlen(name) + 1;
-	if (*buffer) {
-		if (*remaining_size < len)
-			return -ERANGE;
-		memcpy(*buffer, name, len);
-		*buffer += len;
-	}
-	*remaining_size -= len;
-	return 0;
-}
-
 /**
  * simple_xattr_list - list all xattr objects
  * @inode: inode from which to get the xattrs
@@ -1287,22 +1277,9 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
 	ssize_t remaining_size = size;
 	int err = 0;
 
-#ifdef CONFIG_FS_POSIX_ACL
-	if (IS_POSIXACL(inode)) {
-		if (inode->i_acl) {
-			err = xattr_list_one(&buffer, &remaining_size,
-					     XATTR_NAME_POSIX_ACL_ACCESS);
-			if (err)
-				return err;
-		}
-		if (inode->i_default_acl) {
-			err = xattr_list_one(&buffer, &remaining_size,
-					     XATTR_NAME_POSIX_ACL_DEFAULT);
-			if (err)
-				return err;
-		}
-	}
-#endif
+	err = posix_acl_listxattr(inode, &buffer, &remaining_size);
+	if (err)
+		return err;
 
 	read_lock(&xattrs->lock);
 	for (rbp = rb_first(&xattrs->rb_root); rbp; rbp = rb_next(rbp)) {
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index ee608d22ecb9..bb229b98761a 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -106,6 +106,8 @@ struct posix_acl *vfs_get_acl(struct user_namespace *mnt_userns,
 			      struct dentry *dentry, const char *acl_name);
 int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 		   const char *acl_name);
+int posix_acl_listxattr(struct inode *inode, char **buffer,
+			ssize_t *remaining_size);
 #else
 static inline int posix_acl_chmod(struct user_namespace *mnt_userns,
 				  struct dentry *dentry, umode_t mode)
@@ -153,6 +155,11 @@ static inline int vfs_remove_acl(struct user_namespace *mnt_userns,
 {
 	return -EOPNOTSUPP;
 }
+int posix_acl_listxattr(struct inode *inode, char **buffer,
+			ssize_t *remaining_size)
+{
+	return 0;
+}
 #endif /* CONFIG_FS_POSIX_ACL */
 
 struct posix_acl *get_inode_acl(struct inode *inode, int type);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 23bbe98cfc16..db022d6548fb 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -120,5 +120,6 @@ static inline bool inode_xattr_disabled(struct inode *inode)
 	return !(inode->i_opflags & IOP_XATTR) &&
 	       (inode->i_opflags & IOP_NOACL);
 }
+int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name);
 
 #endif	/* _LINUX_XATTR_H */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 3/8] xattr: add listxattr helper
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (4 preceding siblings ...)
  (?)
@ 2023-01-30 16:41 ` Christian Brauner
  2023-01-30 16:51   ` Christoph Hellwig
  -1 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:41 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

Add a tiny helper to determine whether an xattr handler given a specific
dentry supports listing the requested xattr. We will use this helper in
various filesystems in later commits.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Remove second helper after architectural changes in the series.
- Christoph Hellwig <hch@lst.de>:
  - Renamed helper to xattr_handler_can_list().
---
 include/linux/xattr.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index db022d6548fb..91d5b9de933a 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -47,6 +47,22 @@ struct xattr_handler {
 		   size_t size, int flags);
 };
 
+/**
+ * xattr_handler_can_list - check whether xattr can be listed
+ * @handler: handler for this type of xattr
+ * @dentry: dentry whose inode xattr to list
+ *
+ * Determine whether the xattr associated with @dentry can be listed given
+ * @handler.
+ *
+ * Return: true if xattr can be listed, false if not.
+ */
+static inline bool xattr_handler_can_list(const struct xattr_handler *handler,
+					  struct dentry *dentry)
+{
+	return handler && (!handler->list || handler->list(dentry));
+}
+
 const char *xattr_full_name(const struct xattr_handler *, const char *);
 
 struct xattr {

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 4/8] xattr: remove unused argument
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (5 preceding siblings ...)
  (?)
@ 2023-01-30 16:42 ` Christian Brauner
  -1 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Patch unchanged.
---
 fs/nfsd/nfs4xdr.c     |  3 +--
 fs/xattr.c            | 10 ++++------
 include/linux/xattr.h |  2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 97edb32be77f..79814cfe8bcf 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3442,8 +3442,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		p = xdr_reserve_space(xdr, 4);
 		if (!p)
 			goto out_resource;
-		err = xattr_supported_namespace(d_inode(dentry),
-						XATTR_USER_PREFIX);
+		err = xattr_supports_user_prefix(d_inode(dentry));
 		*p++ = cpu_to_be32(err == 0);
 	}
 
diff --git a/fs/xattr.c b/fs/xattr.c
index 0e8ce76cb76a..9de2f211ee6e 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 91d5b9de933a..de3f507397fb 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -94,7 +94,7 @@ int vfs_getxattr_alloc(struct user_namespace *mnt_userns,
 		       struct dentry *dentry, const char *name,
 		       char **xattr_value, size_t size, gfp_t flags);
 
-int xattr_supported_namespace(struct inode *inode, const char *prefix);
+int xattr_supports_user_prefix(struct inode *inode);
 
 static inline const char *xattr_prefix(const struct xattr_handler *handler)
 {

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 5/8] fs: drop unused posix acl handlers
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (6 preceding siblings ...)
  (?)
@ 2023-01-30 16:42 ` Christian Brauner
  -1 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

Remove struct posix_acl_{access,default}_handler for all filesystems
that don't depend on the xattr handler in their inode->i_op->listxattr()
method in any way. There's nothing more to do than to simply remove the
handler. It's been effectively unused ever since we introduced the new
posix acl api.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Fold all filesystem changes except reiserfs into this patch.
---
 fs/9p/xattr.c        | 4 ----
 fs/btrfs/xattr.c     | 4 ----
 fs/ceph/xattr.c      | 4 ----
 fs/cifs/xattr.c      | 4 ----
 fs/ecryptfs/inode.c  | 4 ----
 fs/erofs/xattr.c     | 4 ----
 fs/ext2/xattr.c      | 4 ----
 fs/ext4/xattr.c      | 4 ----
 fs/f2fs/xattr.c      | 4 ----
 fs/gfs2/xattr.c      | 2 --
 fs/jffs2/xattr.c     | 4 ----
 fs/jfs/xattr.c       | 4 ----
 fs/nfs/nfs3_fs.h     | 1 -
 fs/nfs/nfs3acl.c     | 6 ------
 fs/nfs/nfs3super.c   | 3 ---
 fs/ntfs3/xattr.c     | 4 ----
 fs/ocfs2/xattr.c     | 2 --
 fs/orangefs/xattr.c  | 2 --
 fs/overlayfs/super.c | 8 --------
 fs/xfs/xfs_xattr.c   | 4 ----
 mm/shmem.c           | 4 ----
 21 files changed, 80 deletions(-)

diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index b6984311e00a..1d2df17b450f 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -183,10 +183,6 @@ static struct xattr_handler v9fs_xattr_security_handler = {
 const struct xattr_handler *v9fs_xattr_handlers[] = {
 	&v9fs_xattr_user_handler,
 	&v9fs_xattr_trusted_handler,
-#ifdef CONFIG_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 #ifdef CONFIG_9P_FS_SECURITY
 	&v9fs_xattr_security_handler,
 #endif
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 0ed4b119a7ca..a6abe528c5d8 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -444,10 +444,6 @@ static const struct xattr_handler btrfs_btrfs_xattr_handler = {
 
 const struct xattr_handler *btrfs_xattr_handlers[] = {
 	&btrfs_security_xattr_handler,
-#ifdef CONFIG_BTRFS_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&btrfs_trusted_xattr_handler,
 	&btrfs_user_xattr_handler,
 	&btrfs_btrfs_xattr_handler,
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index f31350cda960..22e22e8dc226 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1411,10 +1411,6 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
  * attributes are handled directly.
  */
 const struct xattr_handler *ceph_xattr_handlers[] = {
-#ifdef CONFIG_CEPH_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&ceph_other_xattr_handler,
 	NULL,
 };
diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 5f2fb2fd2e37..1b50814eadbb 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -487,9 +487,5 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
 	&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_cifs_ntsd_full_xattr_handler,
 	&smb3_ntsd_full_xattr_handler, /* alias for above since avoiding "cifs" */
-#ifdef CONFIG_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	NULL
 };
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index f3cd00fac9c3..5802b93b2cda 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1210,10 +1210,6 @@ static const struct xattr_handler ecryptfs_xattr_handler = {
 };
 
 const struct xattr_handler *ecryptfs_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&ecryptfs_xattr_handler,
 	NULL
 };
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index a62fb8a3318a..2c98a15a92ed 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -469,10 +469,6 @@ const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
 
 const struct xattr_handler *erofs_xattr_handlers[] = {
 	&erofs_xattr_user_handler,
-#ifdef CONFIG_EROFS_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&erofs_xattr_trusted_handler,
 #ifdef CONFIG_EROFS_FS_SECURITY
 	&erofs_xattr_security_handler,
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 641abfa4b718..262951ffe8d0 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -113,10 +113,6 @@ static const struct xattr_handler *ext2_xattr_handler_map[] = {
 const struct xattr_handler *ext2_xattr_handlers[] = {
 	&ext2_xattr_user_handler,
 	&ext2_xattr_trusted_handler,
-#ifdef CONFIG_EXT2_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 #ifdef CONFIG_EXT2_FS_SECURITY
 	&ext2_xattr_security_handler,
 #endif
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index a2f04a3808db..ba7f2557adb8 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -101,10 +101,6 @@ static const struct xattr_handler * const ext4_xattr_handler_map[] = {
 const struct xattr_handler *ext4_xattr_handlers[] = {
 	&ext4_xattr_user_handler,
 	&ext4_xattr_trusted_handler,
-#ifdef CONFIG_EXT4_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 #ifdef CONFIG_EXT4_FS_SECURITY
 	&ext4_xattr_security_handler,
 #endif
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index dc2e8637189e..ccb564e328af 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -204,10 +204,6 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
 
 const struct xattr_handler *f2fs_xattr_handlers[] = {
 	&f2fs_xattr_user_handler,
-#ifdef CONFIG_F2FS_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&f2fs_xattr_trusted_handler,
 #ifdef CONFIG_F2FS_FS_SECURITY
 	&f2fs_xattr_security_handler,
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 518c0677e12a..88c78dc526fa 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -1501,8 +1501,6 @@ const struct xattr_handler *gfs2_xattr_handlers_max[] = {
 	/* GFS2_FS_FORMAT_MIN */
 	&gfs2_xattr_user_handler,
 	&gfs2_xattr_security_handler,
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
 	NULL,
 };
 
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index da3e18503c65..0eaec4a0f3b1 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -919,10 +919,6 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
 	&jffs2_user_xattr_handler,
 #ifdef CONFIG_JFFS2_FS_SECURITY
 	&jffs2_security_xattr_handler,
-#endif
-#ifdef CONFIG_JFFS2_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
 #endif
 	&jffs2_trusted_xattr_handler,
 	NULL
diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
index f9273f6901c8..dfdc0c1f6e25 100644
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -986,10 +986,6 @@ static const struct xattr_handler jfs_trusted_xattr_handler = {
 };
 
 const struct xattr_handler *jfs_xattr_handlers[] = {
-#ifdef CONFIG_JFS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&jfs_os2_xattr_handler,
 	&jfs_user_xattr_handler,
 	&jfs_security_xattr_handler,
diff --git a/fs/nfs/nfs3_fs.h b/fs/nfs/nfs3_fs.h
index df9ca56db347..a6d1314dbe56 100644
--- a/fs/nfs/nfs3_fs.h
+++ b/fs/nfs/nfs3_fs.h
@@ -17,7 +17,6 @@ extern int nfs3_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry
 extern int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 		struct posix_acl *dfacl);
 extern ssize_t nfs3_listxattr(struct dentry *, char *, size_t);
-extern const struct xattr_handler *nfs3_xattr_handlers[];
 #else
 static inline int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 		struct posix_acl *dfacl)
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 74d11e3c4205..aeb158e3bd99 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -300,12 +300,6 @@ int nfs3_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 	goto out;
 }
 
-const struct xattr_handler *nfs3_xattr_handlers[] = {
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-	NULL,
-};
-
 static int
 nfs3_list_one_acl(struct inode *inode, int type, const char *name, void *data,
 		size_t size, ssize_t *result)
diff --git a/fs/nfs/nfs3super.c b/fs/nfs/nfs3super.c
index 7c5809431e61..8a9be9e47f76 100644
--- a/fs/nfs/nfs3super.c
+++ b/fs/nfs/nfs3super.c
@@ -14,9 +14,6 @@ struct nfs_subversion nfs_v3 = {
 	.rpc_vers = &nfs_version3,
 	.rpc_ops  = &nfs_v3_clientops,
 	.sops     = &nfs_sops,
-#ifdef CONFIG_NFS_V3_ACL
-	.xattr    = nfs3_xattr_handlers,
-#endif
 };
 
 static int __init init_nfs_v3(void)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 616df209feea..1eb9f3d9ba8c 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -1033,10 +1033,6 @@ static const struct xattr_handler ntfs_other_xattr_handler = {
 };
 
 const struct xattr_handler *ntfs_xattr_handlers[] = {
-#ifdef CONFIG_NTFS3_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&ntfs_other_xattr_handler,
 	NULL,
 };
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 95d0611c5fc7..482b2ef7ca54 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -89,8 +89,6 @@ static struct ocfs2_xattr_def_value_root def_xv = {
 
 const struct xattr_handler *ocfs2_xattr_handlers[] = {
 	&ocfs2_xattr_user_handler,
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
 	&ocfs2_xattr_trusted_handler,
 	&ocfs2_xattr_security_handler,
 	NULL
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 9a5b757fbd2f..3203abc89b9f 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -555,8 +555,6 @@ static const struct xattr_handler orangefs_xattr_default_handler = {
 };
 
 const struct xattr_handler *orangefs_xattr_handlers[] = {
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
 	&orangefs_xattr_default_handler,
 	NULL
 };
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 85b891152a2c..559d416e06a3 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1055,20 +1055,12 @@ static const struct xattr_handler ovl_other_xattr_handler = {
 };
 
 static const struct xattr_handler *ovl_trusted_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&ovl_own_trusted_xattr_handler,
 	&ovl_other_xattr_handler,
 	NULL
 };
 
 static const struct xattr_handler *ovl_user_xattr_handlers[] = {
-#ifdef CONFIG_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&ovl_own_user_xattr_handler,
 	&ovl_other_xattr_handler,
 	NULL
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 10aa1fd39d2b..379e1dc97225 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -179,10 +179,6 @@ const struct xattr_handler *xfs_xattr_handlers[] = {
 	&xfs_xattr_user_handler,
 	&xfs_xattr_trusted_handler,
 	&xfs_xattr_security_handler,
-#ifdef CONFIG_XFS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	NULL
 };
 
diff --git a/mm/shmem.c b/mm/shmem.c
index 0005ab2c29af..06b91b524dfc 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3331,10 +3331,6 @@ static const struct xattr_handler shmem_trusted_xattr_handler = {
 };
 
 static const struct xattr_handler *shmem_xattr_handlers[] = {
-#ifdef CONFIG_TMPFS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
-#endif
 	&shmem_security_xattr_handler,
 	&shmem_trusted_xattr_handler,
 	NULL

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 6/8] fs: simplify ->listxattr() implementation
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
  (?)
  (?)
@ 2023-01-30 16:42   ` Christian Brauner
  -1 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd

The ext{2,4}, erofs, f2fs, and jffs2 filesystems use the same logic to
check whether a given xattr can be listed. Simplify them and avoid
open-coding the same check by calling the helper we introduced earlier.

Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-erofs@lists.ozlabs.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
  - Rework this patch completey by keeping the legacy generic POSIX ACL
    handlers around so that array-based handler indexing still works.
    This means way less churn for filesystems.
---
 fs/erofs/xattr.c |  8 ++------
 fs/erofs/xattr.h | 14 +++++++++++---
 fs/ext2/xattr.c  | 17 ++++++++++-------
 fs/ext4/xattr.c  | 17 ++++++++++-------
 fs/f2fs/xattr.c  | 16 ++++++++++------
 fs/jffs2/xattr.c | 21 ++++++++++++---------
 6 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 2c98a15a92ed..4b73be57c9f4 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -492,13 +492,9 @@ static int xattr_entrylist(struct xattr_iter *_it,
 	unsigned int prefix_len;
 	const char *prefix;
 
-	const struct xattr_handler *h =
-		erofs_xattr_handler(entry->e_name_index);
-
-	if (!h || (h->list && !h->list(it->dentry)))
+	prefix = erofs_xattr_prefix(entry->e_name_index, it->dentry);
+	if (!prefix)
 		return 1;
-
-	prefix = xattr_prefix(h);
 	prefix_len = strlen(prefix);
 
 	if (!it->buffer) {
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 0a43c9ee9f8f..08658e414c33 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,8 +41,11 @@ extern const struct xattr_handler erofs_xattr_user_handler;
 extern const struct xattr_handler erofs_xattr_trusted_handler;
 extern const struct xattr_handler erofs_xattr_security_handler;
 
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+static inline const char *erofs_xattr_prefix(unsigned int idx,
+					     struct dentry *dentry)
 {
+	const struct xattr_handler *handler = NULL;
+
 	static const struct xattr_handler *xattr_handler_map[] = {
 		[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -57,8 +60,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
 #endif
 	};
 
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
+	if (idx && idx < ARRAY_SIZE(xattr_handler_map))
+		handler = xattr_handler_map[idx];
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 extern const struct xattr_handler *erofs_xattr_handlers[];
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 262951ffe8d0..958976f809f5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,14 +121,18 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
 
 #define EA_BLOCK_CACHE(inode)	(EXT2_SB(inode->i_sb)->s_ea_block_cache)
 
-static inline const struct xattr_handler *
-ext2_xattr_handler(int name_index)
+static inline const char *ext2_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map))
 		handler = ext2_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static bool
@@ -329,11 +333,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	/* list the attribute names */
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext2_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext2_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ba7f2557adb8..3fbeeb00fd78 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -169,14 +169,18 @@ static void ext4_xattr_block_csum_set(struct inode *inode,
 						bh->b_blocknr, BHDR(bh));
 }
 
-static inline const struct xattr_handler *
-ext4_xattr_handler(int name_index)
+static inline const char *ext4_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext4_xattr_handler_map))
 		handler = ext4_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static int
@@ -691,11 +695,10 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
 	size_t rest = buffer_size;
 
 	for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext4_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext4_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ccb564e328af..9de984645253 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -212,13 +212,18 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static inline const char *f2fs_xattr_prefix(int index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
 		handler = f2fs_xattr_handler_map[index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -569,12 +574,12 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 	last_base_addr = (void *)base_addr + XATTR_SIZE(inode);
 
 	list_for_each_xattr(entry, base_addr) {
-		const struct xattr_handler *handler =
-			f2fs_xattr_handler(entry->e_name_index);
 		const char *prefix;
 		size_t prefix_len;
 		size_t size;
 
+		prefix = f2fs_xattr_prefix(entry->e_name_index, dentry);
+
 		if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
 			(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
 			f2fs_err(F2FS_I_SB(inode), "inode (%lu) has corrupted xattr",
@@ -586,10 +591,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 			goto cleanup;
 		}
 
-		if (!handler || (handler->list && !handler->list(dentry)))
+		if (!prefix)
 			continue;
 
-		prefix = xattr_prefix(handler);
 		prefix_len = strlen(prefix);
 		size = prefix_len + entry->e_name_len + 1;
 		if (buffer) {
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 0eaec4a0f3b1..1189a70d2007 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -924,8 +924,9 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
 	NULL
 };
 
-static const struct xattr_handler *xprefix_to_handler(int xprefix) {
-	const struct xattr_handler *ret;
+static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
+{
+	const struct xattr_handler *ret = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
@@ -948,10 +949,13 @@ static const struct xattr_handler *xprefix_to_handler(int xprefix) {
 		ret = &jffs2_trusted_xattr_handler;
 		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (!xattr_handler_can_list(ret, dentry))
+		return NULL;
+
+	return xattr_prefix(ret);
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -962,7 +966,6 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct jffs2_inode_cache *ic = f->inocache;
 	struct jffs2_xattr_ref *ref, **pref;
 	struct jffs2_xattr_datum *xd;
-	const struct xattr_handler *xhandle;
 	const char *prefix;
 	ssize_t prefix_len, len, rc;
 	int retry = 0;
@@ -994,10 +997,10 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 					goto out;
 			}
 		}
-		xhandle = xprefix_to_handler(xd->xprefix);
-		if (!xhandle || (xhandle->list && !xhandle->list(dentry)))
+
+		prefix = jffs2_xattr_prefix(xd->xprefix, dentry);
+		if (!prefix)
 			continue;
-		prefix = xhandle->prefix ?: xhandle->name;
 		prefix_len = strlen(prefix);
 		rc = prefix_len + xd->name_len + 1;
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [f2fs-dev] [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:42   ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-mtd, Al Viro, linux-ext4, linux-erofs,
	Seth Forshee

The ext{2,4}, erofs, f2fs, and jffs2 filesystems use the same logic to
check whether a given xattr can be listed. Simplify them and avoid
open-coding the same check by calling the helper we introduced earlier.

Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-erofs@lists.ozlabs.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
  - Rework this patch completey by keeping the legacy generic POSIX ACL
    handlers around so that array-based handler indexing still works.
    This means way less churn for filesystems.
---
 fs/erofs/xattr.c |  8 ++------
 fs/erofs/xattr.h | 14 +++++++++++---
 fs/ext2/xattr.c  | 17 ++++++++++-------
 fs/ext4/xattr.c  | 17 ++++++++++-------
 fs/f2fs/xattr.c  | 16 ++++++++++------
 fs/jffs2/xattr.c | 21 ++++++++++++---------
 6 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 2c98a15a92ed..4b73be57c9f4 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -492,13 +492,9 @@ static int xattr_entrylist(struct xattr_iter *_it,
 	unsigned int prefix_len;
 	const char *prefix;
 
-	const struct xattr_handler *h =
-		erofs_xattr_handler(entry->e_name_index);
-
-	if (!h || (h->list && !h->list(it->dentry)))
+	prefix = erofs_xattr_prefix(entry->e_name_index, it->dentry);
+	if (!prefix)
 		return 1;
-
-	prefix = xattr_prefix(h);
 	prefix_len = strlen(prefix);
 
 	if (!it->buffer) {
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 0a43c9ee9f8f..08658e414c33 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,8 +41,11 @@ extern const struct xattr_handler erofs_xattr_user_handler;
 extern const struct xattr_handler erofs_xattr_trusted_handler;
 extern const struct xattr_handler erofs_xattr_security_handler;
 
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+static inline const char *erofs_xattr_prefix(unsigned int idx,
+					     struct dentry *dentry)
 {
+	const struct xattr_handler *handler = NULL;
+
 	static const struct xattr_handler *xattr_handler_map[] = {
 		[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -57,8 +60,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
 #endif
 	};
 
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
+	if (idx && idx < ARRAY_SIZE(xattr_handler_map))
+		handler = xattr_handler_map[idx];
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 extern const struct xattr_handler *erofs_xattr_handlers[];
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 262951ffe8d0..958976f809f5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,14 +121,18 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
 
 #define EA_BLOCK_CACHE(inode)	(EXT2_SB(inode->i_sb)->s_ea_block_cache)
 
-static inline const struct xattr_handler *
-ext2_xattr_handler(int name_index)
+static inline const char *ext2_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map))
 		handler = ext2_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static bool
@@ -329,11 +333,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	/* list the attribute names */
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext2_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext2_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ba7f2557adb8..3fbeeb00fd78 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -169,14 +169,18 @@ static void ext4_xattr_block_csum_set(struct inode *inode,
 						bh->b_blocknr, BHDR(bh));
 }
 
-static inline const struct xattr_handler *
-ext4_xattr_handler(int name_index)
+static inline const char *ext4_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext4_xattr_handler_map))
 		handler = ext4_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static int
@@ -691,11 +695,10 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
 	size_t rest = buffer_size;
 
 	for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext4_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext4_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ccb564e328af..9de984645253 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -212,13 +212,18 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static inline const char *f2fs_xattr_prefix(int index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
 		handler = f2fs_xattr_handler_map[index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -569,12 +574,12 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 	last_base_addr = (void *)base_addr + XATTR_SIZE(inode);
 
 	list_for_each_xattr(entry, base_addr) {
-		const struct xattr_handler *handler =
-			f2fs_xattr_handler(entry->e_name_index);
 		const char *prefix;
 		size_t prefix_len;
 		size_t size;
 
+		prefix = f2fs_xattr_prefix(entry->e_name_index, dentry);
+
 		if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
 			(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
 			f2fs_err(F2FS_I_SB(inode), "inode (%lu) has corrupted xattr",
@@ -586,10 +591,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 			goto cleanup;
 		}
 
-		if (!handler || (handler->list && !handler->list(dentry)))
+		if (!prefix)
 			continue;
 
-		prefix = xattr_prefix(handler);
 		prefix_len = strlen(prefix);
 		size = prefix_len + entry->e_name_len + 1;
 		if (buffer) {
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 0eaec4a0f3b1..1189a70d2007 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -924,8 +924,9 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
 	NULL
 };
 
-static const struct xattr_handler *xprefix_to_handler(int xprefix) {
-	const struct xattr_handler *ret;
+static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
+{
+	const struct xattr_handler *ret = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
@@ -948,10 +949,13 @@ static const struct xattr_handler *xprefix_to_handler(int xprefix) {
 		ret = &jffs2_trusted_xattr_handler;
 		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (!xattr_handler_can_list(ret, dentry))
+		return NULL;
+
+	return xattr_prefix(ret);
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -962,7 +966,6 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct jffs2_inode_cache *ic = f->inocache;
 	struct jffs2_xattr_ref *ref, **pref;
 	struct jffs2_xattr_datum *xd;
-	const struct xattr_handler *xhandle;
 	const char *prefix;
 	ssize_t prefix_len, len, rc;
 	int retry = 0;
@@ -994,10 +997,10 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 					goto out;
 			}
 		}
-		xhandle = xprefix_to_handler(xd->xprefix);
-		if (!xhandle || (xhandle->list && !xhandle->list(dentry)))
+
+		prefix = jffs2_xattr_prefix(xd->xprefix, dentry);
+		if (!prefix)
 			continue;
-		prefix = xhandle->prefix ?: xhandle->name;
 		prefix_len = strlen(prefix);
 		rc = prefix_len + xd->name_len + 1;
 

-- 
2.34.1



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

^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:42   ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-mtd, Al Viro, linux-ext4, linux-erofs,
	Seth Forshee

The ext{2,4}, erofs, f2fs, and jffs2 filesystems use the same logic to
check whether a given xattr can be listed. Simplify them and avoid
open-coding the same check by calling the helper we introduced earlier.

Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-erofs@lists.ozlabs.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
  - Rework this patch completey by keeping the legacy generic POSIX ACL
    handlers around so that array-based handler indexing still works.
    This means way less churn for filesystems.
---
 fs/erofs/xattr.c |  8 ++------
 fs/erofs/xattr.h | 14 +++++++++++---
 fs/ext2/xattr.c  | 17 ++++++++++-------
 fs/ext4/xattr.c  | 17 ++++++++++-------
 fs/f2fs/xattr.c  | 16 ++++++++++------
 fs/jffs2/xattr.c | 21 ++++++++++++---------
 6 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 2c98a15a92ed..4b73be57c9f4 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -492,13 +492,9 @@ static int xattr_entrylist(struct xattr_iter *_it,
 	unsigned int prefix_len;
 	const char *prefix;
 
-	const struct xattr_handler *h =
-		erofs_xattr_handler(entry->e_name_index);
-
-	if (!h || (h->list && !h->list(it->dentry)))
+	prefix = erofs_xattr_prefix(entry->e_name_index, it->dentry);
+	if (!prefix)
 		return 1;
-
-	prefix = xattr_prefix(h);
 	prefix_len = strlen(prefix);
 
 	if (!it->buffer) {
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 0a43c9ee9f8f..08658e414c33 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,8 +41,11 @@ extern const struct xattr_handler erofs_xattr_user_handler;
 extern const struct xattr_handler erofs_xattr_trusted_handler;
 extern const struct xattr_handler erofs_xattr_security_handler;
 
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+static inline const char *erofs_xattr_prefix(unsigned int idx,
+					     struct dentry *dentry)
 {
+	const struct xattr_handler *handler = NULL;
+
 	static const struct xattr_handler *xattr_handler_map[] = {
 		[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -57,8 +60,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
 #endif
 	};
 
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
+	if (idx && idx < ARRAY_SIZE(xattr_handler_map))
+		handler = xattr_handler_map[idx];
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 extern const struct xattr_handler *erofs_xattr_handlers[];
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 262951ffe8d0..958976f809f5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,14 +121,18 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
 
 #define EA_BLOCK_CACHE(inode)	(EXT2_SB(inode->i_sb)->s_ea_block_cache)
 
-static inline const struct xattr_handler *
-ext2_xattr_handler(int name_index)
+static inline const char *ext2_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map))
 		handler = ext2_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static bool
@@ -329,11 +333,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	/* list the attribute names */
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext2_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext2_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ba7f2557adb8..3fbeeb00fd78 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -169,14 +169,18 @@ static void ext4_xattr_block_csum_set(struct inode *inode,
 						bh->b_blocknr, BHDR(bh));
 }
 
-static inline const struct xattr_handler *
-ext4_xattr_handler(int name_index)
+static inline const char *ext4_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext4_xattr_handler_map))
 		handler = ext4_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static int
@@ -691,11 +695,10 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
 	size_t rest = buffer_size;
 
 	for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext4_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext4_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ccb564e328af..9de984645253 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -212,13 +212,18 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static inline const char *f2fs_xattr_prefix(int index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
 		handler = f2fs_xattr_handler_map[index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -569,12 +574,12 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 	last_base_addr = (void *)base_addr + XATTR_SIZE(inode);
 
 	list_for_each_xattr(entry, base_addr) {
-		const struct xattr_handler *handler =
-			f2fs_xattr_handler(entry->e_name_index);
 		const char *prefix;
 		size_t prefix_len;
 		size_t size;
 
+		prefix = f2fs_xattr_prefix(entry->e_name_index, dentry);
+
 		if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
 			(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
 			f2fs_err(F2FS_I_SB(inode), "inode (%lu) has corrupted xattr",
@@ -586,10 +591,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 			goto cleanup;
 		}
 
-		if (!handler || (handler->list && !handler->list(dentry)))
+		if (!prefix)
 			continue;
 
-		prefix = xattr_prefix(handler);
 		prefix_len = strlen(prefix);
 		size = prefix_len + entry->e_name_len + 1;
 		if (buffer) {
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 0eaec4a0f3b1..1189a70d2007 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -924,8 +924,9 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
 	NULL
 };
 
-static const struct xattr_handler *xprefix_to_handler(int xprefix) {
-	const struct xattr_handler *ret;
+static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
+{
+	const struct xattr_handler *ret = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
@@ -948,10 +949,13 @@ static const struct xattr_handler *xprefix_to_handler(int xprefix) {
 		ret = &jffs2_trusted_xattr_handler;
 		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (!xattr_handler_can_list(ret, dentry))
+		return NULL;
+
+	return xattr_prefix(ret);
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -962,7 +966,6 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct jffs2_inode_cache *ic = f->inocache;
 	struct jffs2_xattr_ref *ref, **pref;
 	struct jffs2_xattr_datum *xd;
-	const struct xattr_handler *xhandle;
 	const char *prefix;
 	ssize_t prefix_len, len, rc;
 	int retry = 0;
@@ -994,10 +997,10 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 					goto out;
 			}
 		}
-		xhandle = xprefix_to_handler(xd->xprefix);
-		if (!xhandle || (xhandle->list && !xhandle->list(dentry)))
+
+		prefix = jffs2_xattr_prefix(xd->xprefix, dentry);
+		if (!prefix)
 			continue;
-		prefix = xhandle->prefix ?: xhandle->name;
 		prefix_len = strlen(prefix);
 		rc = prefix_len + xd->name_len + 1;
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:42   ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft),
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd

The ext{2,4}, erofs, f2fs, and jffs2 filesystems use the same logic to
check whether a given xattr can be listed. Simplify them and avoid
open-coding the same check by calling the helper we introduced earlier.

Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-erofs@lists.ozlabs.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Christoph Hellwig <hch@lst.de>:
  - Rework this patch completey by keeping the legacy generic POSIX ACL
    handlers around so that array-based handler indexing still works.
    This means way less churn for filesystems.
---
 fs/erofs/xattr.c |  8 ++------
 fs/erofs/xattr.h | 14 +++++++++++---
 fs/ext2/xattr.c  | 17 ++++++++++-------
 fs/ext4/xattr.c  | 17 ++++++++++-------
 fs/f2fs/xattr.c  | 16 ++++++++++------
 fs/jffs2/xattr.c | 21 ++++++++++++---------
 6 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 2c98a15a92ed..4b73be57c9f4 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -492,13 +492,9 @@ static int xattr_entrylist(struct xattr_iter *_it,
 	unsigned int prefix_len;
 	const char *prefix;
 
-	const struct xattr_handler *h =
-		erofs_xattr_handler(entry->e_name_index);
-
-	if (!h || (h->list && !h->list(it->dentry)))
+	prefix = erofs_xattr_prefix(entry->e_name_index, it->dentry);
+	if (!prefix)
 		return 1;
-
-	prefix = xattr_prefix(h);
 	prefix_len = strlen(prefix);
 
 	if (!it->buffer) {
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 0a43c9ee9f8f..08658e414c33 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -41,8 +41,11 @@ extern const struct xattr_handler erofs_xattr_user_handler;
 extern const struct xattr_handler erofs_xattr_trusted_handler;
 extern const struct xattr_handler erofs_xattr_security_handler;
 
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+static inline const char *erofs_xattr_prefix(unsigned int idx,
+					     struct dentry *dentry)
 {
+	const struct xattr_handler *handler = NULL;
+
 	static const struct xattr_handler *xattr_handler_map[] = {
 		[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
@@ -57,8 +60,13 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
 #endif
 	};
 
-	return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
-		xattr_handler_map[idx] : NULL;
+	if (idx && idx < ARRAY_SIZE(xattr_handler_map))
+		handler = xattr_handler_map[idx];
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 extern const struct xattr_handler *erofs_xattr_handlers[];
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 262951ffe8d0..958976f809f5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,14 +121,18 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
 
 #define EA_BLOCK_CACHE(inode)	(EXT2_SB(inode->i_sb)->s_ea_block_cache)
 
-static inline const struct xattr_handler *
-ext2_xattr_handler(int name_index)
+static inline const char *ext2_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext2_xattr_handler_map))
 		handler = ext2_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static bool
@@ -329,11 +333,10 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	/* list the attribute names */
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext2_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext2_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ba7f2557adb8..3fbeeb00fd78 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -169,14 +169,18 @@ static void ext4_xattr_block_csum_set(struct inode *inode,
 						bh->b_blocknr, BHDR(bh));
 }
 
-static inline const struct xattr_handler *
-ext4_xattr_handler(int name_index)
+static inline const char *ext4_xattr_prefix(int name_index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (name_index > 0 && name_index < ARRAY_SIZE(ext4_xattr_handler_map))
 		handler = ext4_xattr_handler_map[name_index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static int
@@ -691,11 +695,10 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
 	size_t rest = buffer_size;
 
 	for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-		const struct xattr_handler *handler =
-			ext4_xattr_handler(entry->e_name_index);
+		const char *prefix;
 
-		if (handler && (!handler->list || handler->list(dentry))) {
-			const char *prefix = handler->prefix ?: handler->name;
+		prefix = ext4_xattr_prefix(entry->e_name_index, dentry);
+		if (prefix) {
 			size_t prefix_len = strlen(prefix);
 			size_t size = prefix_len + entry->e_name_len + 1;
 
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ccb564e328af..9de984645253 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -212,13 +212,18 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
 	NULL,
 };
 
-static inline const struct xattr_handler *f2fs_xattr_handler(int index)
+static inline const char *f2fs_xattr_prefix(int index,
+					    struct dentry *dentry)
 {
 	const struct xattr_handler *handler = NULL;
 
 	if (index > 0 && index < ARRAY_SIZE(f2fs_xattr_handler_map))
 		handler = f2fs_xattr_handler_map[index];
-	return handler;
+
+	if (!xattr_handler_can_list(handler, dentry))
+		return NULL;
+
+	return xattr_prefix(handler);
 }
 
 static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
@@ -569,12 +574,12 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 	last_base_addr = (void *)base_addr + XATTR_SIZE(inode);
 
 	list_for_each_xattr(entry, base_addr) {
-		const struct xattr_handler *handler =
-			f2fs_xattr_handler(entry->e_name_index);
 		const char *prefix;
 		size_t prefix_len;
 		size_t size;
 
+		prefix = f2fs_xattr_prefix(entry->e_name_index, dentry);
+
 		if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
 			(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
 			f2fs_err(F2FS_I_SB(inode), "inode (%lu) has corrupted xattr",
@@ -586,10 +591,9 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 			goto cleanup;
 		}
 
-		if (!handler || (handler->list && !handler->list(dentry)))
+		if (!prefix)
 			continue;
 
-		prefix = xattr_prefix(handler);
 		prefix_len = strlen(prefix);
 		size = prefix_len + entry->e_name_len + 1;
 		if (buffer) {
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 0eaec4a0f3b1..1189a70d2007 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -924,8 +924,9 @@ const struct xattr_handler *jffs2_xattr_handlers[] = {
 	NULL
 };
 
-static const struct xattr_handler *xprefix_to_handler(int xprefix) {
-	const struct xattr_handler *ret;
+static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
+{
+	const struct xattr_handler *ret = NULL;
 
 	switch (xprefix) {
 	case JFFS2_XPREFIX_USER:
@@ -948,10 +949,13 @@ static const struct xattr_handler *xprefix_to_handler(int xprefix) {
 		ret = &jffs2_trusted_xattr_handler;
 		break;
 	default:
-		ret = NULL;
-		break;
+		return NULL;
 	}
-	return ret;
+
+	if (!xattr_handler_can_list(ret, dentry))
+		return NULL;
+
+	return xattr_prefix(ret);
 }
 
 ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
@@ -962,7 +966,6 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct jffs2_inode_cache *ic = f->inocache;
 	struct jffs2_xattr_ref *ref, **pref;
 	struct jffs2_xattr_datum *xd;
-	const struct xattr_handler *xhandle;
 	const char *prefix;
 	ssize_t prefix_len, len, rc;
 	int retry = 0;
@@ -994,10 +997,10 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size)
 					goto out;
 			}
 		}
-		xhandle = xprefix_to_handler(xd->xprefix);
-		if (!xhandle || (xhandle->list && !xhandle->list(dentry)))
+
+		prefix = jffs2_xattr_prefix(xd->xprefix, dentry);
+		if (!prefix)
 			continue;
-		prefix = xhandle->prefix ?: xhandle->name;
 		prefix_len = strlen(prefix);
 		rc = prefix_len + xd->name_len + 1;
 

-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 7/8] reiserfs: rework ->listxattr() implementation
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (8 preceding siblings ...)
  (?)
@ 2023-01-30 16:42 ` Christian Brauner
  -1 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft), reiserfs-devel

Rework reiserfs so it doesn't have to rely on the dummy xattr handlers
in its s_xattr list anymore as this is completely unused for setting and
getting posix acls. Only leave them around for the sake of ->listxattr().

This is somewhat clumsy but reiserfs is marked deprecated and will be
removed in the future anway so I don't feel too bad about it.

Cc: reiserfs-devel@vger.kernel.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Rework patch to account for reiserfs quirks.
---
 fs/reiserfs/reiserfs.h |  2 +-
 fs/reiserfs/xattr.c    | 70 +++++++++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index 3aa928ec527a..14726fd353c4 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -1166,7 +1166,7 @@ static inline int bmap_would_wrap(unsigned bmap_nr)
 	return bmap_nr > ((1LL << 16) - 1);
 }
 
-extern const struct xattr_handler *reiserfs_xattr_handlers[];
+extern const struct xattr_handler **reiserfs_xattr_handlers;
 
 /*
  * this says about version of key of all items (but stat data) the
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 2c326b57d4bc..66574fcbe7a9 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -52,6 +52,7 @@
 #include <linux/quotaops.h>
 #include <linux/security.h>
 #include <linux/posix_acl_xattr.h>
+#include <linux/xattr.h>
 
 #define PRIVROOT_NAME ".reiserfs_priv"
 #define XAROOT_NAME   "xattrs"
@@ -770,23 +771,49 @@ reiserfs_xattr_get(struct inode *inode, const char *name, void *buffer,
 			(handler) != NULL;			\
 			(handler) = *(handlers)++)
 
+static const struct xattr_handler *reiserfs_xattr_handlers_max[] = {
+#ifdef CONFIG_REISERFS_FS_POSIX_ACL
+	&posix_acl_access_xattr_handler,
+	&posix_acl_default_xattr_handler,
+#endif
+#ifdef CONFIG_REISERFS_FS_XATTR
+	&reiserfs_xattr_user_handler,
+	&reiserfs_xattr_trusted_handler,
+#endif
+#ifdef CONFIG_REISERFS_FS_SECURITY
+	&reiserfs_xattr_security_handler,
+#endif
+	NULL
+};
+
+/* Actual operations that are exported to VFS-land */
+#ifdef CONFIG_REISERFS_FS_POSIX_ACL
+const struct xattr_handler **reiserfs_xattr_handlers =
+	reiserfs_xattr_handlers_max + 2;
+#else
+const struct xattr_handler **reiserfs_xattr_handlers =
+	reiserfs_xattr_handlers_max;
+#endif
+
 /* 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 char *name, struct dentry *dentry)
 {
-	const struct xattr_handler *xah;
-
-	if (!handlers)
-		return NULL;
+	const struct xattr_handler *xah = NULL;
+	const struct xattr_handler **handlers = reiserfs_xattr_handlers_max;
 
 	for_each_xattr_handler(handlers, xah) {
 		const char *prefix = xattr_prefix(xah);
-		if (strncmp(prefix, name, strlen(prefix)) == 0)
-			break;
+
+		if (strncmp(prefix, name, strlen(prefix)))
+			continue;
+
+		if (!xattr_handler_can_list(xah, dentry))
+			return false;
+
+		return true;
 	}
 
-	return xah;
+	return false;
 }
 
 struct listxattr_buf {
@@ -807,12 +834,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(name, b->dentry))
 			return true;
 		size = namelen + 1;
 		if (b->buf) {
@@ -902,22 +924,6 @@ void reiserfs_xattr_unregister_handlers(void) {}
 static int create_privroot(struct dentry *dentry) { return 0; }
 #endif
 
-/* Actual operations that are exported to VFS-land */
-const struct xattr_handler *reiserfs_xattr_handlers[] = {
-#ifdef CONFIG_REISERFS_FS_XATTR
-	&reiserfs_xattr_user_handler,
-	&reiserfs_xattr_trusted_handler,
-#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
-};
-
 static int xattr_mount_check(struct super_block *s)
 {
 	/*

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* [PATCH v2 8/8] fs: rename generic posix acl handlers
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
                   ` (9 preceding siblings ...)
  (?)
@ 2023-01-30 16:42 ` Christian Brauner
  2023-01-30 16:53   ` Christoph Hellwig
  -1 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 16:42 UTC (permalink / raw)
  To: linux-fsdevel, Christoph Hellwig
  Cc: Al Viro, Seth Forshee, Christian Brauner (Microsoft)

Reflect in their naming and document that they are kept around for
legacy reasons and shouldn't be used anymore by new code.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Changes in v2:
- Patch introduced.
---
 fs/erofs/xattr.h                |  6 ++----
 fs/ext2/xattr.c                 |  4 ++--
 fs/ext4/xattr.c                 |  4 ++--
 fs/f2fs/xattr.c                 |  4 ++--
 fs/jffs2/xattr.c                |  4 ++--
 fs/ocfs2/xattr.c                | 12 +++++-------
 fs/posix_acl.c                  | 24 ++++++++++++++++++------
 fs/reiserfs/xattr.c             |  4 ++--
 include/linux/posix_acl_xattr.h |  5 +++--
 9 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index 08658e414c33..97185cb649b6 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -49,10 +49,8 @@ static inline const char *erofs_xattr_prefix(unsigned int idx,
 	static const struct xattr_handler *xattr_handler_map[] = {
 		[EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
-		[EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
-			&posix_acl_access_xattr_handler,
-		[EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
-			&posix_acl_default_xattr_handler,
+		[EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+		[EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
 #endif
 		[EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
 #ifdef CONFIG_EROFS_FS_SECURITY
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 958976f809f5..b126af5f8b15 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -101,8 +101,8 @@ static void ext2_xattr_rehash(struct ext2_xattr_header *,
 static const struct xattr_handler *ext2_xattr_handler_map[] = {
 	[EXT2_XATTR_INDEX_USER]		     = &ext2_xattr_user_handler,
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
-	[EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  = &posix_acl_access_xattr_handler,
-	[EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
+	[EXT2_XATTR_INDEX_POSIX_ACL_ACCESS]  = &nop_posix_acl_access,
+	[EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
 #endif
 	[EXT2_XATTR_INDEX_TRUSTED]	     = &ext2_xattr_trusted_handler,
 #ifdef CONFIG_EXT2_FS_SECURITY
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 3fbeeb00fd78..edca79a62bd9 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -88,8 +88,8 @@ static void ext4_xattr_rehash(struct ext4_xattr_header *);
 static const struct xattr_handler * const ext4_xattr_handler_map[] = {
 	[EXT4_XATTR_INDEX_USER]		     = &ext4_xattr_user_handler,
 #ifdef CONFIG_EXT4_FS_POSIX_ACL
-	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &posix_acl_access_xattr_handler,
-	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
+	[EXT4_XATTR_INDEX_POSIX_ACL_ACCESS]  = &nop_posix_acl_access,
+	[EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
 #endif
 	[EXT4_XATTR_INDEX_TRUSTED]	     = &ext4_xattr_trusted_handler,
 #ifdef CONFIG_EXT4_FS_SECURITY
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 9de984645253..7eb9628478c8 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -192,8 +192,8 @@ const struct xattr_handler f2fs_xattr_security_handler = {
 static const struct xattr_handler *f2fs_xattr_handler_map[] = {
 	[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
 #ifdef CONFIG_F2FS_FS_POSIX_ACL
-	[F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &posix_acl_access_xattr_handler,
-	[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &posix_acl_default_xattr_handler,
+	[F2FS_XATTR_INDEX_POSIX_ACL_ACCESS] = &nop_posix_acl_access,
+	[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &nop_posix_acl_default,
 #endif
 	[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
 #ifdef CONFIG_F2FS_FS_SECURITY
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 1189a70d2007..aa4048a27f31 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -939,10 +939,10 @@ static const char *jffs2_xattr_prefix(int xprefix, struct dentry *dentry)
 #endif
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	case JFFS2_XPREFIX_ACL_ACCESS:
-		ret = &posix_acl_access_xattr_handler;
+		ret = &nop_posix_acl_access;
 		break;
 	case JFFS2_XPREFIX_ACL_DEFAULT:
-		ret = &posix_acl_default_xattr_handler;
+		ret = &nop_posix_acl_default;
 		break;
 #endif
 	case JFFS2_XPREFIX_TRUSTED:
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 482b2ef7ca54..ff85e418d7e3 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -95,13 +95,11 @@ const struct xattr_handler *ocfs2_xattr_handlers[] = {
 };
 
 static const struct xattr_handler *ocfs2_xattr_handler_map[OCFS2_XATTR_MAX] = {
-	[OCFS2_XATTR_INDEX_USER]	= &ocfs2_xattr_user_handler,
-	[OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]
-					= &posix_acl_access_xattr_handler,
-	[OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]
-					= &posix_acl_default_xattr_handler,
-	[OCFS2_XATTR_INDEX_TRUSTED]	= &ocfs2_xattr_trusted_handler,
-	[OCFS2_XATTR_INDEX_SECURITY]	= &ocfs2_xattr_security_handler,
+	[OCFS2_XATTR_INDEX_USER]		= &ocfs2_xattr_user_handler,
+	[OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS]	= &nop_posix_acl_access,
+	[OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT]	= &nop_posix_acl_default,
+	[OCFS2_XATTR_INDEX_TRUSTED]		= &ocfs2_xattr_trusted_handler,
+	[OCFS2_XATTR_INDEX_SECURITY]		= &ocfs2_xattr_security_handler,
 };
 
 struct ocfs2_xattr_info {
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index d5e9db0e4d66..e78b0cc56c6b 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -989,19 +989,31 @@ posix_acl_xattr_list(struct dentry *dentry)
 	return IS_POSIXACL(d_backing_inode(dentry));
 }
 
-const struct xattr_handler posix_acl_access_xattr_handler = {
+/*
+ * nop_posix_acl_access - legacy xattr handler for access POSIX ACLs
+ *
+ * This is the legacy POSIX ACL access xattr handler. It is used by some
+ * filesystems to implement their ->listxattr() inode operation. New code
+ * should never use them.
+ */
+const struct xattr_handler nop_posix_acl_access = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
-	.flags = ACL_TYPE_ACCESS,
 	.list = posix_acl_xattr_list,
 };
-EXPORT_SYMBOL_GPL(posix_acl_access_xattr_handler);
+EXPORT_SYMBOL_GPL(nop_posix_acl_access);
 
-const struct xattr_handler posix_acl_default_xattr_handler = {
+/*
+ * nop_posix_acl_default - legacy xattr handler for default POSIX ACLs
+ *
+ * This is the legacy POSIX ACL default xattr handler. It is used by some
+ * filesystems to implement their ->listxattr() inode operation. New code
+ * should never use them.
+ */
+const struct xattr_handler nop_posix_acl_default = {
 	.name = XATTR_NAME_POSIX_ACL_DEFAULT,
-	.flags = ACL_TYPE_DEFAULT,
 	.list = posix_acl_xattr_list,
 };
-EXPORT_SYMBOL_GPL(posix_acl_default_xattr_handler);
+EXPORT_SYMBOL_GPL(nop_posix_acl_default);
 
 int simple_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 		   struct posix_acl *acl, int type)
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 66574fcbe7a9..ff95d55c4c2c 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -773,8 +773,8 @@ reiserfs_xattr_get(struct inode *inode, const char *name, void *buffer,
 
 static const struct xattr_handler *reiserfs_xattr_handlers_max[] = {
 #ifdef CONFIG_REISERFS_FS_POSIX_ACL
-	&posix_acl_access_xattr_handler,
-	&posix_acl_default_xattr_handler,
+	&nop_posix_acl_access,
+	&nop_posix_acl_default,
 #endif
 #ifdef CONFIG_REISERFS_FS_XATTR
 	&reiserfs_xattr_user_handler,
diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
index 54cd7a14330d..e86f3b731da2 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -68,7 +68,8 @@ static inline int posix_acl_type(const char *name)
 	return -1;
 }
 
-extern const struct xattr_handler posix_acl_access_xattr_handler;
-extern const struct xattr_handler posix_acl_default_xattr_handler;
+/* These are legacy handlers. Don't use them for new code. */
+extern const struct xattr_handler nop_posix_acl_access;
+extern const struct xattr_handler nop_posix_acl_default;
 
 #endif	/* _POSIX_ACL_XATTR_H */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-30 16:41 ` [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls Christian Brauner
@ 2023-01-30 16:50   ` Christoph Hellwig
  2023-01-30 18:09     ` Christian Brauner
  0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:50 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee, Jeff Mahoney

On Mon, Jan 30, 2023 at 05:41:57PM +0100, Christian Brauner wrote:
> The POSIX ACL api doesn't use the xattr handler infrastructure anymore.
> If we keep relying on IOP_XATTR we will have to find a way to raise
> IOP_XATTR during inode_init_always() if a filesystem doesn't implement
> any xattrs other than POSIX ACLs. That's not done today but is in
> principle possible. A prior version introduced SB_I_XATTR to this end.
> Instead of this affecting all filesystems let those filesystems that
> explicitly disable xattrs for some inodes disable POSIX ACLs by raising
> IOP_NOACL.

I'm still a little confused about this, and also about
inode_xattr_disable.  More below:

> -	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
> -	    !(new->d_inode->i_opflags & IOP_XATTR))
> +	if (inode_xattr_disabled(old->d_inode) ||
> +	    inode_xattr_disabled(new->d_inode))

This code shouldn't care about ACLs because the copy up itself
should be all based around the ACL API, no?

> +	if (!(inode->i_opflags & IOP_NOACL))
>  		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
>  	else if (unlikely(is_bad_inode(inode)))
>  		error = -EIO;
> @@ -1205,7 +1205,7 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
>  	if (error)
>  		goto out_inode_unlock;
>  
> -	if (inode->i_opflags & IOP_XATTR)
> +	if (!(inode->i_opflags & IOP_NOACL))
>  		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);

And here the lack of get/set methods should be all we need unless
I'm missing something?

> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index c7d1fa526dea..2a7037b165f0 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -2089,7 +2089,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
>  	 */
>  	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
>  		inode->i_flags |= S_PRIVATE;
> -		inode->i_opflags &= ~IOP_XATTR;
> +		inode_xattr_disable(inode);

I'll need to hear from the reiserfs maintainers, but this also seems
like something that would be better done based on method presence.

> diff --git a/fs/xattr.c b/fs/xattr.c
> index adab9a70b536..89b6c122056d 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -468,7 +468,7 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
>  	error = security_inode_listxattr(dentry);
>  	if (error)
>  		return error;
> -	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
> +	if (inode->i_op->listxattr && !inode_xattr_disabled(inode)) {
>  		error = inode->i_op->listxattr(dentry, list, size);

So once listing ACLs is moved out of ->listxattr there should be no
need to check anything for ACLs here either.

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 2/8] xattr: simplify listxattr helpers
  2023-01-30 16:41 ` [PATCH v2 2/8] xattr: simplify listxattr helpers Christian Brauner
@ 2023-01-30 16:51   ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:51 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee

Looks good:

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 3/8] xattr: add listxattr helper
  2023-01-30 16:41 ` [PATCH v2 3/8] xattr: add listxattr helper Christian Brauner
@ 2023-01-30 16:51   ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:51 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee

Looks good:

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 6/8] fs: simplify ->listxattr() implementation
  2023-01-30 16:42   ` [f2fs-dev] " Christian Brauner
  (?)
  (?)
@ 2023-01-30 16:52     ` Christoph Hellwig
  -1 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:52 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee,
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd

Looks good:

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [f2fs-dev] [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:52     ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:52 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-f2fs-devel, linux-mtd, Seth Forshee, linux-fsdevel,
	linux-ext4, linux-erofs, Christoph Hellwig, Al Viro

Looks good:

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


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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:52     ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:52 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-f2fs-devel, linux-mtd, Seth Forshee, linux-fsdevel,
	linux-ext4, linux-erofs, Christoph Hellwig, Al Viro

Looks good:

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 6/8] fs: simplify ->listxattr() implementation
@ 2023-01-30 16:52     ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:52 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee,
	linux-f2fs-devel, linux-erofs, linux-ext4, linux-mtd

Looks good:

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

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 8/8] fs: rename generic posix acl handlers
  2023-01-30 16:42 ` [PATCH v2 8/8] fs: rename generic posix acl handlers Christian Brauner
@ 2023-01-30 16:53   ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-30 16:53 UTC (permalink / raw)
  To: Christian Brauner; +Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Seth Forshee

Looks good:

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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-30 16:50   ` Christoph Hellwig
@ 2023-01-30 18:09     ` Christian Brauner
  2023-01-31 11:36       ` Christian Brauner
  0 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-30 18:09 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, Al Viro, Seth Forshee, Jeff Mahoney

On Mon, Jan 30, 2023 at 05:50:53PM +0100, Christoph Hellwig wrote:
> On Mon, Jan 30, 2023 at 05:41:57PM +0100, Christian Brauner wrote:
> > The POSIX ACL api doesn't use the xattr handler infrastructure anymore.
> > If we keep relying on IOP_XATTR we will have to find a way to raise
> > IOP_XATTR during inode_init_always() if a filesystem doesn't implement
> > any xattrs other than POSIX ACLs. That's not done today but is in
> > principle possible. A prior version introduced SB_I_XATTR to this end.
> > Instead of this affecting all filesystems let those filesystems that
> > explicitly disable xattrs for some inodes disable POSIX ACLs by raising
> > IOP_NOACL.
> 
> I'm still a little confused about this, and also about
> inode_xattr_disable.  More below:
> 
> > -	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
> > -	    !(new->d_inode->i_opflags & IOP_XATTR))
> > +	if (inode_xattr_disabled(old->d_inode) ||
> > +	    inode_xattr_disabled(new->d_inode))
> 
> This code shouldn't care about ACLs because the copy up itself
> should be all based around the ACL API, no?

The loop copies up all xattrs. It retrieves all xattrs via
vfs_listxattr() then walks through all of them and copies them up. But
it's nothing that we couldn't work around if it buys as less headaches
overall.

> 
> > +	if (!(inode->i_opflags & IOP_NOACL))
> >  		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
> >  	else if (unlikely(is_bad_inode(inode)))
> >  		error = -EIO;
> > @@ -1205,7 +1205,7 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
> >  	if (error)
> >  		goto out_inode_unlock;
> >  
> > -	if (inode->i_opflags & IOP_XATTR)
> > +	if (!(inode->i_opflags & IOP_NOACL))
> >  		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
> 
> And here the lack of get/set methods should be all we need unless
> I'm missing something?

For setting acl that should work, yes.

> 
> > diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> > index c7d1fa526dea..2a7037b165f0 100644
> > --- a/fs/reiserfs/inode.c
> > +++ b/fs/reiserfs/inode.c
> > @@ -2089,7 +2089,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
> >  	 */
> >  	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
> >  		inode->i_flags |= S_PRIVATE;
> > -		inode->i_opflags &= ~IOP_XATTR;
> > +		inode_xattr_disable(inode);
> 
> I'll need to hear from the reiserfs maintainers, but this also seems
> like something that would be better done based on method presence.

I mean, since this is locked I would think we could just:

inode->i_op->{g,s}et_acl = NULL

and for btrfs it should work to as it uses simple_dir_inode_operations
which doesn't implement get/set posix acl methods.

> 
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index adab9a70b536..89b6c122056d 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -468,7 +468,7 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
> >  	error = security_inode_listxattr(dentry);
> >  	if (error)
> >  		return error;
> > -	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
> > +	if (inode->i_op->listxattr && !inode_xattr_disabled(inode)) {
> >  		error = inode->i_op->listxattr(dentry, list, size);
> 
> So once listing ACLs is moved out of ->listxattr there should be no
> need to check anything for ACLs here either.

I think so...

But that would mean we'd need to change the ->listxattr() inode
operation to not return POSIX ACLs anymore. Instead vfs_listxattr()
would issue two vfs_get_acl() calls to check whether POSIX ACLs are
supported and if so append them to the buffer. That seems doable as
vfs_listxattr() is rarely used in the fs/ and nowhere in security/
luckily. Wdyt?

The only thing potentially wrong with that is that's two more filesystem
calls which probably doesn't matter for listing xattrs as that isn't
really that fast anyway and the ->listxattr() api is broken beyond
repair for userspace anyway.

I would need to check tomorrow whether we run into any real issues with
this idea but it could work.

If we're lucky it might mean that we could get rid of the generic POSIX
ACL handler dependency even for ext2/ext4/erofs/f2fs/reiserfs/jffs2/ocfs2.

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-30 18:09     ` Christian Brauner
@ 2023-01-31 11:36       ` Christian Brauner
  2023-01-31 14:55         ` Christian Brauner
  0 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-31 11:36 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, Al Viro, Seth Forshee, Jeff Mahoney

On Mon, Jan 30, 2023 at 07:09:09PM +0100, Christian Brauner wrote:
> On Mon, Jan 30, 2023 at 05:50:53PM +0100, Christoph Hellwig wrote:
> > On Mon, Jan 30, 2023 at 05:41:57PM +0100, Christian Brauner wrote:
> > > The POSIX ACL api doesn't use the xattr handler infrastructure anymore.
> > > If we keep relying on IOP_XATTR we will have to find a way to raise
> > > IOP_XATTR during inode_init_always() if a filesystem doesn't implement
> > > any xattrs other than POSIX ACLs. That's not done today but is in
> > > principle possible. A prior version introduced SB_I_XATTR to this end.
> > > Instead of this affecting all filesystems let those filesystems that
> > > explicitly disable xattrs for some inodes disable POSIX ACLs by raising
> > > IOP_NOACL.
> > 
> > I'm still a little confused about this, and also about
> > inode_xattr_disable.  More below:
> > 
> > > -	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
> > > -	    !(new->d_inode->i_opflags & IOP_XATTR))
> > > +	if (inode_xattr_disabled(old->d_inode) ||
> > > +	    inode_xattr_disabled(new->d_inode))
> > 
> > This code shouldn't care about ACLs because the copy up itself
> > should be all based around the ACL API, no?
> 
> The loop copies up all xattrs. It retrieves all xattrs via
> vfs_listxattr() then walks through all of them and copies them up. But
> it's nothing that we couldn't work around if it buys as less headaches
> overall.
> 
> > 
> > > +	if (!(inode->i_opflags & IOP_NOACL))
> > >  		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
> > >  	else if (unlikely(is_bad_inode(inode)))
> > >  		error = -EIO;
> > > @@ -1205,7 +1205,7 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
> > >  	if (error)
> > >  		goto out_inode_unlock;
> > >  
> > > -	if (inode->i_opflags & IOP_XATTR)
> > > +	if (!(inode->i_opflags & IOP_NOACL))
> > >  		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
> > 
> > And here the lack of get/set methods should be all we need unless
> > I'm missing something?
> 
> For setting acl that should work, yes.
> 
> > 
> > > diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> > > index c7d1fa526dea..2a7037b165f0 100644
> > > --- a/fs/reiserfs/inode.c
> > > +++ b/fs/reiserfs/inode.c
> > > @@ -2089,7 +2089,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
> > >  	 */
> > >  	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
> > >  		inode->i_flags |= S_PRIVATE;
> > > -		inode->i_opflags &= ~IOP_XATTR;
> > > +		inode_xattr_disable(inode);
> > 
> > I'll need to hear from the reiserfs maintainers, but this also seems
> > like something that would be better done based on method presence.
> 
> I mean, since this is locked I would think we could just:
> 
> inode->i_op->{g,s}et_acl = NULL
> 
> and for btrfs it should work to as it uses simple_dir_inode_operations
> which doesn't implement get/set posix acl methods.
> 
> > 
> > > diff --git a/fs/xattr.c b/fs/xattr.c
> > > index adab9a70b536..89b6c122056d 100644
> > > --- a/fs/xattr.c
> > > +++ b/fs/xattr.c
> > > @@ -468,7 +468,7 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
> > >  	error = security_inode_listxattr(dentry);
> > >  	if (error)
> > >  		return error;
> > > -	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
> > > +	if (inode->i_op->listxattr && !inode_xattr_disabled(inode)) {
> > >  		error = inode->i_op->listxattr(dentry, list, size);
> > 
> > So once listing ACLs is moved out of ->listxattr there should be no
> > need to check anything for ACLs here either.
> 
> I think so...
> 
> But that would mean we'd need to change the ->listxattr() inode
> operation to not return POSIX ACLs anymore. Instead vfs_listxattr()
> would issue two vfs_get_acl() calls to check whether POSIX ACLs are

So I see a few issues with this:
* Network filesystems like 9p or cifs retrieve xattrs for ->listxattr()
  in a batch from the server and dump them into the provided buffer.
  If we want to stop listing POSIX ACLs in ->listxattr() that would mean
  we would need to filter them out of the buffer for such filesystems.
  From a cursory glance this might affect more than just 9p and cifs.
* The fuse_listxattr() implementation has different permission
  requirements than fuse_get_acl() which would mean we would potentially
  refuse to list POSIX ACLs where we would have before or vica versa.
* We risk losing returning a consistent snapshot of all xattr names for
  a given inode if we split ->listxattr() and POSIX ACLs apart.

So I'm not sure that we can do it this way.

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-31 11:36       ` Christian Brauner
@ 2023-01-31 14:55         ` Christian Brauner
  2023-01-31 15:18           ` Christoph Hellwig
  0 siblings, 1 reply; 31+ messages in thread
From: Christian Brauner @ 2023-01-31 14:55 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel, Al Viro, Seth Forshee, Jeff Mahoney

On Tue, Jan 31, 2023 at 12:36:47PM +0100, Christian Brauner wrote:
> On Mon, Jan 30, 2023 at 07:09:09PM +0100, Christian Brauner wrote:
> > On Mon, Jan 30, 2023 at 05:50:53PM +0100, Christoph Hellwig wrote:
> > > On Mon, Jan 30, 2023 at 05:41:57PM +0100, Christian Brauner wrote:
> > > > The POSIX ACL api doesn't use the xattr handler infrastructure anymore.
> > > > If we keep relying on IOP_XATTR we will have to find a way to raise
> > > > IOP_XATTR during inode_init_always() if a filesystem doesn't implement
> > > > any xattrs other than POSIX ACLs. That's not done today but is in
> > > > principle possible. A prior version introduced SB_I_XATTR to this end.
> > > > Instead of this affecting all filesystems let those filesystems that
> > > > explicitly disable xattrs for some inodes disable POSIX ACLs by raising
> > > > IOP_NOACL.
> > > 
> > > I'm still a little confused about this, and also about
> > > inode_xattr_disable.  More below:
> > > 
> > > > -	if (!(old->d_inode->i_opflags & IOP_XATTR) ||
> > > > -	    !(new->d_inode->i_opflags & IOP_XATTR))
> > > > +	if (inode_xattr_disabled(old->d_inode) ||
> > > > +	    inode_xattr_disabled(new->d_inode))
> > > 
> > > This code shouldn't care about ACLs because the copy up itself
> > > should be all based around the ACL API, no?
> > 
> > The loop copies up all xattrs. It retrieves all xattrs via
> > vfs_listxattr() then walks through all of them and copies them up. But
> > it's nothing that we couldn't work around if it buys as less headaches
> > overall.
> > 
> > > 
> > > > +	if (!(inode->i_opflags & IOP_NOACL))
> > > >  		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
> > > >  	else if (unlikely(is_bad_inode(inode)))
> > > >  		error = -EIO;
> > > > @@ -1205,7 +1205,7 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
> > > >  	if (error)
> > > >  		goto out_inode_unlock;
> > > >  
> > > > -	if (inode->i_opflags & IOP_XATTR)
> > > > +	if (!(inode->i_opflags & IOP_NOACL))
> > > >  		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
> > > 
> > > And here the lack of get/set methods should be all we need unless
> > > I'm missing something?
> > 
> > For setting acl that should work, yes.
> > 
> > > 
> > > > diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> > > > index c7d1fa526dea..2a7037b165f0 100644
> > > > --- a/fs/reiserfs/inode.c
> > > > +++ b/fs/reiserfs/inode.c
> > > > @@ -2089,7 +2089,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
> > > >  	 */
> > > >  	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
> > > >  		inode->i_flags |= S_PRIVATE;
> > > > -		inode->i_opflags &= ~IOP_XATTR;
> > > > +		inode_xattr_disable(inode);
> > > 
> > > I'll need to hear from the reiserfs maintainers, but this also seems
> > > like something that would be better done based on method presence.
> > 
> > I mean, since this is locked I would think we could just:
> > 
> > inode->i_op->{g,s}et_acl = NULL
> > 
> > and for btrfs it should work to as it uses simple_dir_inode_operations
> > which doesn't implement get/set posix acl methods.
> > 
> > > 
> > > > diff --git a/fs/xattr.c b/fs/xattr.c
> > > > index adab9a70b536..89b6c122056d 100644
> > > > --- a/fs/xattr.c
> > > > +++ b/fs/xattr.c
> > > > @@ -468,7 +468,7 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
> > > >  	error = security_inode_listxattr(dentry);
> > > >  	if (error)
> > > >  		return error;
> > > > -	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
> > > > +	if (inode->i_op->listxattr && !inode_xattr_disabled(inode)) {
> > > >  		error = inode->i_op->listxattr(dentry, list, size);
> > > 
> > > So once listing ACLs is moved out of ->listxattr there should be no
> > > need to check anything for ACLs here either.
> > 
> > I think so...
> > 
> > But that would mean we'd need to change the ->listxattr() inode
> > operation to not return POSIX ACLs anymore. Instead vfs_listxattr()
> > would issue two vfs_get_acl() calls to check whether POSIX ACLs are
> 
> So I see a few issues with this:
> * Network filesystems like 9p or cifs retrieve xattrs for ->listxattr()
>   in a batch from the server and dump them into the provided buffer.
>   If we want to stop listing POSIX ACLs in ->listxattr() that would mean
>   we would need to filter them out of the buffer for such filesystems.
>   From a cursory glance this might affect more than just 9p and cifs.
> * The fuse_listxattr() implementation has different permission
>   requirements than fuse_get_acl() which would mean we would potentially
>   refuse to list POSIX ACLs where we would have before or vica versa.
> * We risk losing returning a consistent snapshot of all xattr names for
>   a given inode if we split ->listxattr() and POSIX ACLs apart.
> 
> So I'm not sure that we can do it this way.

So I've experimented a bit. Really, the remaining issue to remove the
dependency of POSIX ACLs on IOP_XATTR is the check in vfs_listxattr()
for IOP_XATTR. Removing IOP_XATTR from vfs_listxattr() from is really
only a problem if there is any filesystems that removes IOP_XATTR
without also assigning a dedicated set of inode_operations that either
leaves ->listxattr() unimplement or makes it a NOP.

The only filesystems for which this is true is reiserfs. But I believe
we can fix this by forcing reiserfs to use a dedicated set of inode
operations with ->listxattr(), and the POSIX ACL inode operations
unimplemented.

So here's what I have which would allows us to proceed with the removal.
@Christoph, do you think that's doable or is there anything I'm still
missing?:

From 765c56cba40fb42e7e7a319cf3cbcc9d5abd7c11 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 31 Jan 2023 15:13:53 +0100
Subject: [PATCH 1/3] reiserfs: use simplify IOP_XATTR handling

Reiserfs is the only filesystem that removes IOP_XATTR without also
using a set of dedicated inode operations at the same time that nop all
xattr related inode operations. This means we need to have a IOP_XATTR
check in vfs_listxattr() instead of just being able to check for
->listxattr() being implemented.

Introduce a dedicated set of nop inode operations that are used when
IOP_XATTR is removed allowing us to remove that check from
vfs_listxattr().

Cc: reiserfs-devel@vger.kernel.org
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/reiserfs/inode.c    |  1 +
 fs/reiserfs/namei.c    | 17 +++++++++++++++++
 fs/reiserfs/reiserfs.h |  1 +
 fs/reiserfs/xattr.c    |  3 +++
 4 files changed, 22 insertions(+)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index c7d1fa526dea..e293eaaed185 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -2090,6 +2090,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
 		inode->i_flags |= S_PRIVATE;
 		inode->i_opflags &= ~IOP_XATTR;
+		inode->i_op = &reiserfs_privdir_inode_operations;
 	}
 
 	if (reiserfs_posixacl(inode->i_sb)) {
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 0b8aa99749f1..19aca1684fd1 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -384,6 +384,7 @@ static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry,
 		if (IS_PRIVATE(dir)) {
 			inode->i_flags |= S_PRIVATE;
 			inode->i_opflags &= ~IOP_XATTR;
+			inode->i_op = &reiserfs_privdir_inode_operations;
 		}
 	}
 	reiserfs_write_unlock(dir->i_sb);
@@ -1669,6 +1670,22 @@ const struct inode_operations reiserfs_dir_inode_operations = {
 	.fileattr_set = reiserfs_fileattr_set,
 };
 
+const struct inode_operations reiserfs_privdir_inode_operations = {
+	.create = reiserfs_create,
+	.lookup = reiserfs_lookup,
+	.link = reiserfs_link,
+	.unlink = reiserfs_unlink,
+	.symlink = reiserfs_symlink,
+	.mkdir = reiserfs_mkdir,
+	.rmdir = reiserfs_rmdir,
+	.mknod = reiserfs_mknod,
+	.rename = reiserfs_rename,
+	.setattr = reiserfs_setattr,
+	.permission = reiserfs_permission,
+	.fileattr_get = reiserfs_fileattr_get,
+	.fileattr_set = reiserfs_fileattr_set,
+};
+
 /*
  * symlink operations.. same as page_symlink_inode_operations, with xattr
  * stuff added
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index 14726fd353c4..9d3a9c0df43b 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -3160,6 +3160,7 @@ static inline int reiserfs_proc_info_global_done(void)
 
 /* dir.c */
 extern const struct inode_operations reiserfs_dir_inode_operations;
+extern const struct inode_operations reiserfs_privdir_inode_operations;
 extern const struct inode_operations reiserfs_symlink_inode_operations;
 extern const struct inode_operations reiserfs_special_inode_operations;
 extern const struct file_operations reiserfs_dir_operations;
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 1864a35853a9..01dc07fb60a4 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -912,6 +912,8 @@ static int create_privroot(struct dentry *dentry)
 
 	d_inode(dentry)->i_flags |= S_PRIVATE;
 	d_inode(dentry)->i_opflags &= ~IOP_XATTR;
+	d_inode(dentry)->i_op = &reiserfs_privdir_inode_operations;
+
 	reiserfs_info(dentry->d_sb, "Created %s - reserved for xattr "
 		      "storage.\n", PRIVROOT_NAME);
 
@@ -984,6 +986,7 @@ int reiserfs_lookup_privroot(struct super_block *s)
 		if (d_really_is_positive(dentry)) {
 			d_inode(dentry)->i_flags |= S_PRIVATE;
 			d_inode(dentry)->i_opflags &= ~IOP_XATTR;
+			d_inode(dentry)->i_op = &reiserfs_privdir_inode_operations;
 		}
 	} else
 		err = PTR_ERR(dentry);
-- 
2.34.1

From 3002147142b2558f9e642da38a567f7ce1fdd2e5 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 31 Jan 2023 15:17:11 +0100
Subject: [PATCH 2/3] acl: don't depend on IOP_XATTR

All codepaths that don't want to implement POSIX ACLs should simply not
implement the associated inode operations instead of relying on
IOP_XATTR. That's the case for all filesystems today.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
 fs/posix_acl.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 7a4d89897c37..881a7fd1cacb 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1132,12 +1132,10 @@ int vfs_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 	if (error)
 		goto out_inode_unlock;
 
-	if (inode->i_opflags & IOP_XATTR)
+	if (likely(!is_bad_inode(inode)))
 		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
-	else if (unlikely(is_bad_inode(inode)))
-		error = -EIO;
 	else
-		error = -EOPNOTSUPP;
+		error = -EIO;
 	if (!error) {
 		fsnotify_xattr(dentry);
 		evm_inode_post_set_acl(dentry, acl_name, kacl);
@@ -1242,12 +1240,10 @@ int vfs_remove_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
 	if (error)
 		goto out_inode_unlock;
 
-	if (inode->i_opflags & IOP_XATTR)
+	if (likely(!is_bad_inode(inode)))
 		error = set_posix_acl(mnt_userns, dentry, acl_type, NULL);
-	else if (unlikely(is_bad_inode(inode)))
-		error = -EIO;
 	else
-		error = -EOPNOTSUPP;
+		error = -EIO;
 	if (!error) {
 		fsnotify_xattr(dentry);
 		evm_inode_post_remove_acl(mnt_userns, dentry, acl_name);
-- 
2.34.1

From 303bcb6a2da513e7d904bc8f9915b992b33ab661 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 31 Jan 2023 15:25:10 +0100
Subject: [PATCH 3/4] xattr: don't rely on IOP_XATTR in vfs_listxattr()

Filesystems that explicitly turn of xattrs for a given inode all set
inode->i_op to a dedicated set of inode operations that doesn't
implement ->listxattr().  Removing this dependency will allow us to
decouple POSIX ACLs from IOP_XATTR and they can still be listed even if
no other xattr handlers are implemented.

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

diff --git a/fs/xattr.c b/fs/xattr.c
index 8743402a5e8b..aed1cacb97c6 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -466,7 +466,8 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
 	error = security_inode_listxattr(dentry);
 	if (error)
 		return error;
-	if (inode->i_op->listxattr && (inode->i_opflags & IOP_XATTR)) {
+
+	if (inode->i_op->listxattr) {
 		error = inode->i_op->listxattr(dentry, list, size);
 	} else {
 		error = security_inode_listsecurity(inode, list, size);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls
  2023-01-31 14:55         ` Christian Brauner
@ 2023-01-31 15:18           ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-01-31 15:18 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Christoph Hellwig, linux-fsdevel, Al Viro, Seth Forshee,
	Jeff Mahoney, reiserfs-devel

Sorry for not keeping up with your flow of ideas, so chiming in now:

> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index c7d1fa526dea..e293eaaed185 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -2090,6 +2090,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
>  	if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) {
>  		inode->i_flags |= S_PRIVATE;
>  		inode->i_opflags &= ~IOP_XATTR;
> +		inode->i_op = &reiserfs_privdir_inode_operations;

I wonder if there is any way to set this where reiserfs assigns
the other ops. 

> +const struct inode_operations reiserfs_privdir_inode_operations = {
> +	.create = reiserfs_create,
> +	.lookup = reiserfs_lookup,
> +	.link = reiserfs_link,
> +	.unlink = reiserfs_unlink,
> +	.symlink = reiserfs_symlink,
> +	.mkdir = reiserfs_mkdir,
> +	.rmdir = reiserfs_rmdir,
> +	.mknod = reiserfs_mknod,
> +	.rename = reiserfs_rename,
> +	.setattr = reiserfs_setattr,
> +	.permission = reiserfs_permission,
> +	.fileattr_get = reiserfs_fileattr_get,
> +	.fileattr_set = reiserfs_fileattr_set,
> +};

I suspect many other ops aren't need either, but I really need input
from people that known reiserfs better.

> +	if (likely(!is_bad_inode(inode)))
>  		error = set_posix_acl(mnt_userns, dentry, acl_type, kacl);
>  	else
> +		error = -EIO;

I wonder if the is_bad_inode check should simplify move into
get/set_posix_acl.  But otherwise this patch looks good.

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
  2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
  (?)
  (?)
@ 2023-04-26 23:07   ` patchwork-bot+f2fs
  -1 siblings, 0 replies; 31+ messages in thread
From: patchwork-bot+f2fs @ 2023-04-26 23:07 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, hch, reiserfs-devel, linux-f2fs-devel, linux-mtd,
	viro, linux-ext4, linux-erofs, sforshee

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Christian Brauner (Microsoft) <brauner@kernel.org>:

On Mon, 30 Jan 2023 17:41:56 +0100 you wrote:
> Hey everyone,
> 
> after we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handler registered at sb->s_xattr for two reasons.
> First, because a few filesystems rely 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.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v2,6/8] fs: simplify ->listxattr() implementation
    https://git.kernel.org/jaegeuk/f2fs/c/a5488f29835c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-04-26 23:07   ` patchwork-bot+f2fs
  0 siblings, 0 replies; 31+ messages in thread
From: patchwork-bot+f2fs @ 2023-04-26 23:07 UTC (permalink / raw)
  To: Christian Brauner
  Cc: reiserfs-devel, linux-f2fs-devel, linux-mtd, viro, linux-fsdevel,
	linux-ext4, linux-erofs, hch, sforshee

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Christian Brauner (Microsoft) <brauner@kernel.org>:

On Mon, 30 Jan 2023 17:41:56 +0100 you wrote:
> Hey everyone,
> 
> after we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handler registered at sb->s_xattr for two reasons.
> First, because a few filesystems rely 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.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v2,6/8] fs: simplify ->listxattr() implementation
    https://git.kernel.org/jaegeuk/f2fs/c/a5488f29835c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




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

^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-04-26 23:07   ` patchwork-bot+f2fs
  0 siblings, 0 replies; 31+ messages in thread
From: patchwork-bot+f2fs @ 2023-04-26 23:07 UTC (permalink / raw)
  To: Christian Brauner
  Cc: reiserfs-devel, linux-f2fs-devel, linux-mtd, viro, linux-fsdevel,
	linux-ext4, linux-erofs, hch, sforshee

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Christian Brauner (Microsoft) <brauner@kernel.org>:

On Mon, 30 Jan 2023 17:41:56 +0100 you wrote:
> Hey everyone,
> 
> after we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handler registered at sb->s_xattr for two reasons.
> First, because a few filesystems rely 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.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v2,6/8] fs: simplify ->listxattr() implementation
    https://git.kernel.org/jaegeuk/f2fs/c/a5488f29835c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 31+ messages in thread
* Re: [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers
@ 2023-04-26 23:07   ` patchwork-bot+f2fs
  0 siblings, 0 replies; 31+ messages in thread
From: patchwork-bot+f2fs @ 2023-04-26 23:07 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, hch, reiserfs-devel, linux-f2fs-devel, linux-mtd,
	viro, linux-ext4, linux-erofs, sforshee

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Christian Brauner (Microsoft) <brauner@kernel.org>:

On Mon, 30 Jan 2023 17:41:56 +0100 you wrote:
> Hey everyone,
> 
> after we finished the introduction of the new posix acl api last cycle
> we still left the generic POSIX ACL xattr handlers around in the
> filesystems xattr handler registered at sb->s_xattr for two reasons.
> First, because a few filesystems rely 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.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v2,6/8] fs: simplify ->listxattr() implementation
    https://git.kernel.org/jaegeuk/f2fs/c/a5488f29835c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

^ permalink raw reply	[flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-04-26 23:07 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 16:41 [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers Christian Brauner
2023-01-30 16:41 ` Christian Brauner
2023-01-30 16:41 ` Christian Brauner
2023-01-30 16:41 ` [f2fs-dev] " Christian Brauner
2023-01-30 16:41 ` [PATCH v2 1/8] fs: don't use IOP_XATTR for posix acls Christian Brauner
2023-01-30 16:50   ` Christoph Hellwig
2023-01-30 18:09     ` Christian Brauner
2023-01-31 11:36       ` Christian Brauner
2023-01-31 14:55         ` Christian Brauner
2023-01-31 15:18           ` Christoph Hellwig
2023-01-30 16:41 ` [PATCH v2 2/8] xattr: simplify listxattr helpers Christian Brauner
2023-01-30 16:51   ` Christoph Hellwig
2023-01-30 16:41 ` [PATCH v2 3/8] xattr: add listxattr helper Christian Brauner
2023-01-30 16:51   ` Christoph Hellwig
2023-01-30 16:42 ` [PATCH v2 4/8] xattr: remove unused argument Christian Brauner
2023-01-30 16:42 ` [PATCH v2 5/8] fs: drop unused posix acl handlers Christian Brauner
2023-01-30 16:42 ` [PATCH v2 6/8] fs: simplify ->listxattr() implementation Christian Brauner
2023-01-30 16:42   ` Christian Brauner
2023-01-30 16:42   ` Christian Brauner
2023-01-30 16:42   ` [f2fs-dev] " Christian Brauner
2023-01-30 16:52   ` Christoph Hellwig
2023-01-30 16:52     ` Christoph Hellwig
2023-01-30 16:52     ` Christoph Hellwig
2023-01-30 16:52     ` [f2fs-dev] " Christoph Hellwig
2023-01-30 16:42 ` [PATCH v2 7/8] reiserfs: rework " Christian Brauner
2023-01-30 16:42 ` [PATCH v2 8/8] fs: rename generic posix acl handlers Christian Brauner
2023-01-30 16:53   ` Christoph Hellwig
2023-04-26 23:07 ` [f2fs-dev] [PATCH v2 0/8] acl: remove generic posix acl handlers from all xattr handlers patchwork-bot+f2fs
2023-04-26 23:07   ` patchwork-bot+f2fs
2023-04-26 23:07   ` patchwork-bot+f2fs
2023-04-26 23:07   ` patchwork-bot+f2fs

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.