Re: [PATCH] lsm: make SECURITY_PATH always enabled

[Song Liu <songliubraving@meta.com>]

Hi Paul, 

Thanks for your comments!

> On Apr 22, 2025, at 2:13 PM, Paul Moore <paul@paul-moore.com> wrote:

[...]

>> 
>> However, I don't think existing CONFIG_SECURITY_* are doing the right
>> things. Among all the configs, CONFIG_SECURITY_PATH is the most awkward
>> to me. Say , if we have CONFIG_SECURITY_PATH, shouldn't we also have
>> CONFIG_SECURITY_INODE? IOW, something like:
>> 
>> #ifdef CONFIG_SECURITY_INODE
>> int security_inode_rmdir(struct inode *dir, struct dentry *dentry);
>> #endif
>> 
>> #ifdef CONFIG_SECURITY_PATH
>> int security_path_rmdir(struct inode *dir, struct dentry *dentry);
>> #endif
> 
> Without putting much thought into what would fall under
> CONFIG_SECURITY_INODE, I think it would be interesting to see what
> hooks one might be able to make conditional on such a Kconfig knob.
> Using security_inode_permission() as a simple test, it looks like only
> SELinux and Smack provide implementations, spot checks on a few other
> security_*inode*() hooks shows similar, or even more limited, results.
> 
> You would need to spend some time to determine what LSM hooks are used
> by which LSMs and adjust their Kconfigs appropriately for the new
> CONFIG_SECURITY_INODE knob, but if you do that then I think that would
> be okay.

Well, I was hoping to simplify the CONFIGs by removing one. So I am 
not sure whether adding a new CONFIG is the right thing to do. 

> 
>> OR, maybe we should just remove security_inode_rmdir(), and users of
>> security_inode_rmdir() can just use security_path_rmdir() instead?
> 
> Those two LSM hooks are called from slightly different places in the
> codepath which has an impact on their environment.  For example, the
> inode variant doesn't have to deal with directory inodes that don't
> have a defined rmdir op, whereas the path variant does; the inode
> variant doesn't have to worry about S_KERNEL_FILE files, the inode
> variant has a refcount'd and locked dentry, etc.  Moving an existing
> LSM, especially complex ones, from one LSM hook to another, is a
> delicate operation and might not be worth it for such a small return.

Given there is pushback when a new LSM hook is added, I assume 
removing a hook (or merge two hooks into one) may be a good move. 
Well, it is totally possible that I underestimated the complexity of 
the work. 

Thanks,
Song