sorry, I forgot to add the link to preview discussion: https://lore.kernel.org/all/20220827111215.131442-1-xiujianfeng@xxxxxxxxxx/ On 2023/5/5 16:11, Xiu Jianfeng wrote: > Hi, > > I am working on adding xattr/attr support for landlock [1], so we can > control fs accesses such as chmod, chown, uptimes, setxattr, etc.. inside > landlock sandbox. the LSM hooks as following are invoved: > 1.inode_setattr > 2.inode_setxattr > 3.inode_removexattr > 4.inode_set_acl > 5.inode_remove_acl > which are controlled by LANDLOCK_ACCESS_FS_WRITE_METADATA. > > and > 1.inode_getattr > 2.inode_get_acl > 3.inode_getxattr > 4.inode_listxattr > which are controlled by LANDLOCK_ACCESS_FS_READ_METADATA > > Some of these hooks only take struct dentry as a argument, However, for > path-based LSMs such Landlock, Apparmor and Tomoyo, struct path instead > of struct dentry required to make sense of attr/xattr accesses. So we > need to refactor these hooks to take a struct path argument. > > This patchset only refators inode_setattr hook as part of whole work. > > Also, I have a problem about file_dentry() in __file_remove_privs() of the > first patch, before changes in commit c1892c37769cf ("vfs: fix deadlock in > file_remove_privs() on overlayfs"), it gets dentry and inode as belows: > > struct dentry *dentry = file->f_path.dentry; > struct inode *inode = d_inode(dentry); > > That would be clear to change it to pass &file->f_path to > __remove_privs()->notify_change()->inode_setattr(). > After that commit, it has been changed to: > > struct dentry *dentry = file_dentry(file); > struct inode *inode = file_inode(file); > > If I understand correctly, the dentry from file_dentry() maybe the upper > or the lower, it can be different from file->f_path.dentry. It can't just > go back to use &file->f_path otherwise the bug will come back for > overlayfs. So for such scenario, how to get a path from file if the file > maybe or not from overlayfs, and which kind of overlayfs path is ok for > Landlock? > > Xiu Jianfeng (2): > fs: Change notify_change() to take struct path argument > lsm: Change inode_setattr hook to take struct path argument > > drivers/base/devtmpfs.c | 5 +++-- > fs/attr.c | 7 ++++--- > fs/cachefiles/interface.c | 4 ++-- > fs/coredump.c | 2 +- > fs/ecryptfs/inode.c | 18 +++++++++--------- > fs/fat/file.c | 2 +- > fs/inode.c | 8 +++++--- > fs/ksmbd/smb2pdu.c | 6 +++--- > fs/ksmbd/smbacl.c | 2 +- > fs/namei.c | 2 +- > fs/nfsd/vfs.c | 12 ++++++++---- > fs/open.c | 19 ++++++++++--------- > fs/overlayfs/overlayfs.h | 4 +++- > fs/utimes.c | 2 +- > include/linux/fs.h | 4 ++-- > include/linux/lsm_hook_defs.h | 2 +- > include/linux/security.h | 4 ++-- > security/security.c | 10 +++++----- > security/selinux/hooks.c | 3 ++- > security/smack/smack_lsm.c | 5 +++-- > 20 files changed, 67 insertions(+), 54 deletions(-) >
