On 5/12/2025 6:25 AM, Andrey Albershteyn wrote:
> Introduce new hooks for setting and getting filesystem extended
> attributes on inode (FS_IOC_FSGETXATTR).
>
> Cc: seli...@vger.kernel.org
> Cc: Paul Moore <p...@paul-moore.com>
>
> Signed-off-by: Andrey Albershteyn <aalbe...@kernel.org>
> ---
> fs/file_attr.c | 19 ++++++++++++++++---
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 16 ++++++++++++++++
> security/security.c | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/fs/file_attr.c b/fs/file_attr.c
> index 2910b7047721..be62d97cc444 100644
> --- a/fs/file_attr.c
> +++ b/fs/file_attr.c
> @@ -76,10 +76,15 @@ EXPORT_SYMBOL(fileattr_fill_flags);
> int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> {
> struct inode *inode = d_inode(dentry);
> + int error;
>
> if (!inode->i_op->fileattr_get)
> return -ENOIOCTLCMD;
>
> + error = security_inode_file_getattr(dentry, fa);
> + if (error)
> + return error;
> +
If you're changing VFS behavior to depend on LSMs supporting the new
hooks I'm concerned about the impact it will have on the LSMs that you
haven't supplied hooks for. Have you tested these changes with anything
besides SELinux?
> return inode->i_op->fileattr_get(dentry, fa);
> }
> EXPORT_SYMBOL(vfs_fileattr_get);
> @@ -242,12 +247,20 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct
> dentry *dentry,
> } else {
> fa->flags |= old_ma.flags & ~FS_COMMON_FL;
> }
> +
> err = fileattr_set_prepare(inode, &old_ma, fa);
> - if (!err)
> - err = inode->i_op->fileattr_set(idmap, dentry, fa);
> + if (err)
> + goto out;
> + err = security_inode_file_setattr(dentry, fa);
> + if (err)
> + goto out;
> + err = inode->i_op->fileattr_set(idmap, dentry, fa);
> + if (err)
> + goto out;
> }
> +
> +out:
> inode_unlock(inode);
> -
> return err;
> }
> EXPORT_SYMBOL(vfs_fileattr_set);
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index bf3bbac4e02a..9600a4350e79 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -157,6 +157,8 @@ LSM_HOOK(int, 0, inode_removexattr, struct mnt_idmap
> *idmap,
> struct dentry *dentry, const char *name)
> LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
> const char *name)
> +LSM_HOOK(int, 0, inode_file_setattr, struct dentry *dentry, struct fileattr
> *fa)
> +LSM_HOOK(int, 0, inode_file_getattr, struct dentry *dentry, struct fileattr
> *fa)
> LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
> struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
> LSM_HOOK(void, LSM_RET_VOID, inode_post_set_acl, struct dentry *dentry,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index cc9b54d95d22..d2da2f654345 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -451,6 +451,10 @@ int security_inode_listxattr(struct dentry *dentry);
> int security_inode_removexattr(struct mnt_idmap *idmap,
> struct dentry *dentry, const char *name);
> void security_inode_post_removexattr(struct dentry *dentry, const char
> *name);
> +int security_inode_file_setattr(struct dentry *dentry,
> + struct fileattr *fa);
> +int security_inode_file_getattr(struct dentry *dentry,
> + struct fileattr *fa);
> int security_inode_need_killpriv(struct dentry *dentry);
> int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
> int security_inode_getsecurity(struct mnt_idmap *idmap,
> @@ -1053,6 +1057,18 @@ static inline void
> security_inode_post_removexattr(struct dentry *dentry,
> const char *name)
> { }
>
> +static inline int security_inode_file_setattr(struct dentry *dentry,
> + struct fileattr *fa)
> +{
> + return 0;
> +}
> +
> +static inline int security_inode_file_getattr(struct dentry *dentry,
> + struct fileattr *fa)
> +{
> + return 0;
> +}
> +
> static inline int security_inode_need_killpriv(struct dentry *dentry)
> {
> return cap_inode_need_killpriv(dentry);
> diff --git a/security/security.c b/security/security.c
> index fb57e8fddd91..09c891e6027d 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2622,6 +2622,36 @@ void security_inode_post_removexattr(struct dentry
> *dentry, const char *name)
> call_void_hook(inode_post_removexattr, dentry, name);
> }
>
> +/**
> + * security_inode_file_setattr() - check if setting fsxattr is allowed
> + * @dentry: file to set filesystem extended attributes on
> + * @fa: extended attributes to set on the inode
> + *
> + * Called when file_setattr() syscall or FS_IOC_FSSETXATTR ioctl() is called
> on
> + * inode
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_file_setattr(struct dentry *dentry, struct fileattr *fa)
> +{
> + return call_int_hook(inode_file_setattr, dentry, fa);
> +}
> +
> +/**
> + * security_inode_file_getattr() - check if retrieving fsxattr is allowed
> + * @dentry: file to retrieve filesystem extended attributes from
> + * @fa: extended attributes to get
> + *
> + * Called when file_getattr() syscall or FS_IOC_FSGETXATTR ioctl() is called
> on
> + * inode
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_file_getattr(struct dentry *dentry, struct fileattr *fa)
> +{
> + return call_int_hook(inode_file_getattr, dentry, fa);
> +}
> +
> /**
> * security_inode_need_killpriv() - Check if security_inode_killpriv()
> required
> * @dentry: associated dentry
>
