[PATCH v5 02/30] fs: pass dentry to set acl method

[Mickaël Salaün]

Hi Christian,

We are working on updating the security_inode_*attr LSM hooks to use 
path instead of inode [1]. Indeed, this is required for path-based LSMs 
such as Landlock, AppArmor and Tomoyo to make sense of attr/xattr 
accesses. Could you please update this new ACL API to use struct path 
instead of struct dentry?

[1] 
https://lore.kernel.org/all/1373bbe5-16b1-bf0e-5f92-14c31cb94897@huawei.com/


On 18/10/2022 13:56, Christian Brauner wrote:
> The current way of setting and getting posix acls through the generic
> xattr interface is error prone and type unsafe. The vfs needs to
> interpret and fixup posix acls before storing or reporting it to
> userspace. Various hacks exist to make this work. The code is hard to
> understand and difficult to maintain in it's current form. Instead of
> making this work by hacking posix acls through xattr handlers we are
> building a dedicated posix acl api around the get and set inode
> operations. This removes a lot of hackiness and makes the codepaths
> easier to maintain. A lot of background can be found in [1].
> 
> Since some filesystem rely on the dentry being available to them when
> setting posix acls (e.g., 9p and cifs) they cannot rely on set acl inode
> operation. But since ->set_acl() is required in order to use the generic
> posix acl xattr handlers filesystems that do not implement this inode
> operation cannot use the handler and need to implement their own
> dedicated posix acl handlers.
> 
> Update the ->set_acl() inode method to take a dentry argument. This
> allows all filesystems to rely on ->set_acl().
> 
> As far as I can tell all codepaths can be switched to rely on the dentry
> instead of just the inode. Note that the original motivation for passing
> the dentry separate from the inode instead of just the dentry in the
> xattr handlers was because of security modules that call
> security_d_instantiate(). This hook is called during
> d_instantiate_new(), d_add(), __d_instantiate_anon(), and
> d_splice_alias() to initialize the inode's security context and possibly
> to set security.* xattrs. Since this only affects security.* xattrs this
> is completely irrelevant for posix acls.
> 
> Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org [1]
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> Signed-off-by: Christian Brauner (Microsoft) <brauner at kernel.org>
> ---
> 
> Notes:
>      /* v2 */
>      Christoph Hellwig <hch at lst.de>:
>      - Split orangefs into a preparatory patch.
>      
>      /* v3 */
>      unchanged
>      
>      /* v4 */
>      unchanged
>      
>      /* v5 */
>      unchanged
> 
>   Documentation/filesystems/vfs.rst |  2 +-
>   fs/bad_inode.c                    |  2 +-
>   fs/btrfs/acl.c                    |  3 ++-
>   fs/btrfs/ctree.h                  |  2 +-
>   fs/btrfs/inode.c                  |  2 +-
>   fs/ceph/acl.c                     |  3 ++-
>   fs/ceph/inode.c                   |  2 +-
>   fs/ceph/super.h                   |  2 +-
>   fs/ext2/acl.c                     |  3 ++-
>   fs/ext2/acl.h                     |  2 +-
>   fs/ext2/inode.c                   |  2 +-
>   fs/ext4/acl.c                     |  3 ++-
>   fs/ext4/acl.h                     |  2 +-
>   fs/ext4/inode.c                   |  2 +-
>   fs/f2fs/acl.c                     |  4 +++-
>   fs/f2fs/acl.h                     |  2 +-
>   fs/f2fs/file.c                    |  2 +-
>   fs/fuse/acl.c                     |  3 ++-
>   fs/fuse/fuse_i.h                  |  2 +-
>   fs/gfs2/acl.c                     |  3 ++-
>   fs/gfs2/acl.h                     |  2 +-
>   fs/gfs2/inode.c                   |  2 +-
>   fs/jffs2/acl.c                    |  3 ++-
>   fs/jffs2/acl.h                    |  2 +-
>   fs/jffs2/fs.c                     |  2 +-
>   fs/jfs/acl.c                      |  3 ++-
>   fs/jfs/file.c                     |  2 +-
>   fs/jfs/jfs_acl.h                  |  2 +-
>   fs/ksmbd/smb2pdu.c                |  4 ++--
>   fs/ksmbd/smbacl.c                 |  4 ++--
>   fs/ksmbd/vfs.c                    | 15 ++++++++-------
>   fs/ksmbd/vfs.h                    |  4 ++--
>   fs/nfs/nfs3_fs.h                  |  2 +-
>   fs/nfs/nfs3acl.c                  |  3 ++-
>   fs/nfsd/nfs2acl.c                 |  4 ++--
>   fs/nfsd/nfs3acl.c                 |  4 ++--
>   fs/nfsd/vfs.c                     |  4 ++--
>   fs/ntfs3/file.c                   |  2 +-
>   fs/ntfs3/ntfs_fs.h                |  4 ++--
>   fs/ntfs3/xattr.c                  |  9 +++++----
>   fs/ocfs2/acl.c                    |  3 ++-
>   fs/ocfs2/acl.h                    |  2 +-
>   fs/orangefs/acl.c                 |  5 +++--
>   fs/orangefs/inode.c               |  7 ++++---
>   fs/orangefs/orangefs-kernel.h     |  4 ++--
>   fs/posix_acl.c                    | 18 +++++++++++-------
>   fs/reiserfs/acl.h                 |  6 +++---
>   fs/reiserfs/inode.c               |  2 +-
>   fs/reiserfs/xattr_acl.c           |  9 ++++++---
>   fs/xfs/xfs_acl.c                  |  3 ++-
>   fs/xfs/xfs_acl.h                  |  2 +-
>   fs/xfs/xfs_iops.c                 | 10 ++++++----
>   include/linux/fs.h                |  2 +-
>   include/linux/posix_acl.h         | 12 ++++++------
>   mm/shmem.c                        |  2 +-
>   55 files changed, 119 insertions(+), 93 deletions(-)