linux-kernel - Re: [PATCH v5 2/7] lsm: introduce new hooks for setting/getting inode fsxattr

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb737e58-51ab-4918-b5ba-2c18bf1ad601@schaufler-ca.com>
Date: Wed, 14 May 2025 11:21:46 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Andrey Albershteyn <aalbersh@...hat.com>
Cc: 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>,
 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/14/2025 4:02 AM, Andrey Albershteyn wrote:
> On 2025-05-12 08:43:32, Casey Schaufler wrote:
>> 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?
> Sorry, this thread is incomplete, I've resent full patchset again.
> If you have any further comments please comment in that thread [1]
>
> I haven't tested with anything except SELinux, but I suppose if
> module won't register any hooks, then security_inode_file_*() will
> return 0. Reverting SELinux implementation of the hooks doesn't
> cause any errors.
>
> I'm not that familiar with LSMs/selinux and its codebase, if you can
> recommend what need to be tested while adding new hooks, I will try
> to do that for next revision.

At a minimum the Smack testsuite:
	https://github.com/smack-team/smack-testsuite.git
And the audit suite:
	https://github.com/linux-audit/audit-testsuite.git

AppArmor has a suite as well, but I'm not sure where is resides.

My primary concern is that you're making changes that remove existing
hook calls and add new hook calls without verifying that the protections
provided by the old calls are always also provided by the new ones.

>
> [1]: https://lore.kernel.org/linux-fsdevel/CAOQ4uxgOAxg7N1OUJfb1KMp7oWOfN=KV9Lzz6ZrX0=XRGOQrEQ@mail.gmail.com/T/#t
>