Re: [PATCH 10/29] selinux: implement set acl hook

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 23, 2022 at 10:35 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> On Fri, Sep 23, 2022 at 10:26:35AM -0400, Paul Moore wrote:
> > On Fri, Sep 23, 2022 at 3:57 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > > On Fri, Sep 23, 2022 at 08:47:07AM +0200, Christoph Hellwig wrote:
> > > > On Thu, Sep 22, 2022 at 01:16:57PM -0400, Paul Moore wrote:
> > > > > properly review the changes, but one thing immediately jumped out at
> > > > > me when looking at this: why is the LSM hook
> > > > > "security_inode_set_acl()" when we are passing a dentry instead of an
> > > > > inode?  We don't have a lot of them, but there are
> > > > > `security_dentry_*()` LSM hooks in the existing kernel code.
> > > >
> > > > I'm no LSM expert, but isn't the inode vs dentry for if it is
> > > > related to an inode operation or dentry operation, not about that
> > > > the first argument is?
> > >
> > > Indeed. For example ...
> >
> > If the goal is for this LSM hook to operate on an inode and not a
> > dentry, let's pass it an inode instead.  This should help prevent
>
> I would be ok with that but EVM requires a dentry being passed and as
> evm is called from security_inode_set_acl() exactly like it is from
> security_inode_setxattr() and similar the hook has to take a dentry.

If a dentry is truly needed by EVM (a quick look indicates that it may
just be for the VFS getxattr API, but I haven't traced the full code
path), then I'm having a hard time reconciling that this isn't a
dentry operation.  Yes, I get that the ACLs belong to the inode and
not the dentry, but then why do we need the dentry?  It seems like the
interfaces are broken slightly, or at least a little odd ... <shrug>

> And I want to minimize - ideally get rid of at some point - separate
> calls to security_*() and evm_*() or ima_() in the vfs. So the evm hook
> should please stay in there.

For the record, I want to get rid of the IMA and EVM specific hooks in
the kernel.  They were a necessity back when there could only be one
LSM active at a given time, but with that no longer the case I see
little reason why IMA/EVM/etc. remain separate; it makes the code
worse and complicates a lot of things both at the LSM layer as well as
the rest of the kernel.  I've mentioned this to a few people,
including Mimi, and it came up during at talk at LPC this year.

-- 
paul-moore.com



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux