On Mar 21, 2025 Andrey Albershteyn <aalbe...@redhat.com> 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/ioctl.c | 7 ++++++-
> include/linux/lsm_hook_defs.h | 4 ++++
> include/linux/security.h | 16 ++++++++++++++++
> security/security.c | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+), 1 deletion(-)
Thanks Andrey, one small change below, but otherwise this looks pretty
good. If you feel like trying to work up the SELinux implementation but
need some assitance please let me know, I'll be happy to help :)
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index
> 638a36be31c14afc66a7fd6eb237d9545e8ad997..4434c97bc5dff5a3e8635e28745cd99404ff353e
> 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -525,10 +525,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_getfsxattr(inode, fa);
> + if (error)
> + return error;
> +
> return inode->i_op->fileattr_get(dentry, fa);
> }
> EXPORT_SYMBOL(vfs_fileattr_get);
> @@ -692,7 +697,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct
> dentry *dentry,
> fa->flags |= old_ma.flags & ~FS_COMMON_FL;
> }
> err = fileattr_set_prepare(inode, &old_ma, fa);
> - if (!err)
> + if (!err && !security_inode_setfsxattr(inode, fa))
> err = inode->i_op->fileattr_set(idmap, dentry, fa);
> }
> inode_unlock(inode);
I don't believe we want to hide or otherwise drop the LSM return code as
that could lead to odd behavior, e.g. returning 0/success despite not
having executed the fileattr_set operation.
--
paul-moore.com
