| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f700845d-f332-4336-a441-08f98cd7f075@schaufler-ca.com>
Date: Mon, 12 May 2025 08:43:32 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Andrey Albershteyn <aalbersh@...hat.com>,
Richard Henderson <richard.henderson@...aro.org>,
Matt Turner <mattst88@...il.com>, Russell King <linux@...linux.org.uk>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>, Michal Simek <monstr@...str.eu>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
Helge Deller <deller@....de>, Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Naveen N Rao <naveen@...nel.org>, Heiko Carstens <hca@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>, Alexander Gordeev
<agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>, Rich Felker <dalias@...c.org>,
John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
"David S. Miller" <davem@...emloft.net>,
Andreas Larsson <andreas@...sler.com>, Andy Lutomirski <luto@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
Chris Zankel <chris@...kel.net>, Max Filippov <jcmvbkbc@...il.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Mickaël Salaün <mic@...ikod.net>,
Günther Noack <gnoack@...gle.com>,
Arnd Bergmann <arnd@...db.de>, Pali Rohár
<pali@...nel.org>, Paul Moore <paul@...l-moore.com>,
James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
Stephen Smalley <stephen.smalley.work@...il.com>,
Ondrej Mosnacek <omosnace@...hat.com>, Tyler Hicks <code@...icks.com>,
Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>
Cc: linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-m68k@...ts.linux-m68k.org,
linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-api@...r.kernel.org, linux-arch@...r.kernel.org,
selinux@...r.kernel.org, ecryptfs@...r.kernel.org,
linux-unionfs@...r.kernel.org, linux-xfs@...r.kernel.org,
Andrey Albershteyn <aalbersh@...nel.org>,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v5 2/7] lsm: introduce new hooks for setting/getting inode
fsxattr
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: selinux@...r.kernel.org
> Cc: Paul Moore <paul@...l-moore.com>
>
> Signed-off-by: Andrey Albershteyn <aalbersh@...nel.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
>
Powered by blists - more mailing lists