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: 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?
> 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=xrgoq...@mail.gmail.com/T/#t
>
