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
