On 2025-05-14 11:21:46, Casey Schaufler wrote:
> 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.
Well, I thought about something more specific, I know about these
testsuites
>
> 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.
I'm only adding new hooks, ioctls weren't calling any hooks.
--
- Andrey
