[RFC PATCH 0/29] Rework the LSM initialization
Casey Schaufler
casey at schaufler-ca.com
Thu Apr 10 14:13:11 UTC 2025
On 4/9/2025 11:49 AM, Paul Moore wrote:
> This is one of those patchsets that started out small and then quickly
> expanded to what you see here. I will warn you that some of the
> individual patches are a bit ugly to look at, but I believe the end
> result is much cleaner than what we have now, fixes some odd/undesirable
> behavior on boot, and enables some new functionality.
>
> The most obvious changes are the extraction of the LSM notifier and
> initialization code out of security/security.c and into their own files,
> security/lsm_notifier.c and security/lsm_init.c. While not strictly
> necessary, I think we can all agree that security/security.c has grown
> to be a bit of a mess, and these are two bits of functionality which
> can be extracted out into their own files without too much fuss. I
> personally find this to be a nice quality-of-life improvement, and while
> I'm open to keeping everything in security.c, the argument for doing so
> is going to need to be *very* persuasive.
It's something I've considered doing as part of the stacking work,
but that I have eschewed in the spirit of churn reduction. I've no
problem with it.
> The other significant change is moving all of the LSM initcalls into the
> LSM framework. While I've always pushed to keep the LSM framework as
> minimal as possible, there are some things that we really can't defer to
> the individual LSMs and with the LSM framework responsible for enabling
> or disabling the individual LSMs at boot, I believe management and
> execution of the LSM initcalls needs to be handled in the framework as
> well. Not only does this move ensure that we aren't running initcalls
> for LSMs which are disabled, it also provides us with a convenient spot
> to signal when all of the LSMs have been actived (see the LSM_STARTED_ALL
> patch towards the end of the patchset). This is not a feature we
> currently need, but I'm aware of some future work that does require this
> so I thought it would be good to think about it now while doing this
> work.
>
> Related to the LSM_STARTED_ALL patch, the final patch in this series
> adds support for LSMs to indicate if they provide lsm_prop values for
> subjects and/or objects. Casey needs this functionality for his recent
> audit changes, and I personally find the counting approach presented
> here to be ... less ugly I guess?
The flags approach works for me. I was going to propose adding a call
audit_lsm_secctx() that LSMs would call to identify that a secctx was
being supported, but I had considered the flag approach as well. As for
ugly, I can't say one way or the other.
> This patchset is marked as a RFC for a number of reasons: additional
> testing is required, the commit descriptions could benefit from some
> extra attention, and I still have hopes that some of the individual
> patches could be cleaned up a bit (I still like the end result, but how
> we get there could be improved). I would really appreciate if the
> individual LSM maintainers could give this a quick look, especially
> the individual LSM patches that move the initcalls into the LSM
> framework as some of those are non-trivial.
General comments:
Adjacent patches with no more commit message than "cleanup" should
be combined, as that message is telling me "these aren't the changes
you're looking for".
And about that. I believe that missing or uninformative commit messages
are on your list of things that displease you. You will need to improve
them to get them past yourself. :)
There's a lot of churn here due to unnecessary name changes. I can't
say they're unjustified, but the patch set is bigger than it needs to
be, and more disruptive.
I haven't tested it, but I don't see any substantial problems so far.
> Mimi and Roberto, the
> IMA/EVM work here was particularly "fun"; from what I've seen thus far
> it appears to work correctly, but I have no idea if that code is good
> or bad from you perspective. It's perfectly okay if you want to
> reject the approach taken in IMA/EVM, but we do need to move the
> initcalls up to the LSM framework, so please suggest some code that
> would allow us to do that for IMA/EVM.
>
> --
> Paul Moore (29):
> lsm: split the notifier code out into lsm_notifier.c
> lsm: split the init code out into lsm_init.c
> lsm: simplify prepare_lsm() and rename to lsm_prep_single()
> lsm: simplify ordered_lsm_init() and rename to lsm_init_ordered()
> lsm: replace the name field with a pointer to the lsm_id struct
> lsm: cleanup and normalize the LSM order symbols naming
> lsm: rework lsm_active_cnt and lsm_idlist[]
> lsm: get rid of the lsm_names list and do some cleanup
> lsm: cleanup and normalize the LSM enabled functions
> lsm: cleanup the LSM blob size code
> lsm: cleanup initialize_lsm() and rename to lsm_init_single()
> lsm: cleanup the LSM ordered parsing
> lsm: fold lsm_init_ordered() into security_init()
> lsm: add missing function header comment blocks in lsm_init.c
> lsm: cleanup the debug and console output in lsm_init.c
> lsm: output available LSMs when debugging
> lsm: introduce an initcall mechanism into the LSM framework
> loadpin: move initcalls to the LSM framework
> ipe: move initcalls to the LSM framework
> smack: move initcalls to the LSM framework
> tomoyo: move initcalls to the LSM framework
> safesetid: move initcalls to the LSM framework
> apparmor: move initcalls to the LSM framework
> lockdown: move initcalls to the LSM framework
> ima,evm: move initcalls to the LSM framework
> selinux: move initcalls to the LSM framework
> lsm: consolidate all of the LSM framework initcalls
> lsm: add a LSM_STARTED_ALL notification event
> lsm: add support for counting lsm_prop support among LSMs
>
> include/linux/lsm_hooks.h | 73 -
> include/linux/security.h | 3
> security/Makefile | 2
> security/apparmor/apparmorfs.c | 4
> security/apparmor/crypto.c | 4
> security/apparmor/include/apparmorfs.h | 2
> security/apparmor/include/crypto.h | 1
> security/apparmor/lsm.c | 12
> security/bpf/hooks.c | 3
> security/commoncap.c | 3
> security/inode.c | 29
> security/integrity/Makefile | 2
> security/integrity/evm/evm_main.c | 10
> security/integrity/iint.c | 4
> security/integrity/ima/ima_main.c | 10
> security/integrity/ima/ima_mok.c | 4
> security/integrity/initcalls.c | 97 +
> security/integrity/initcalls.h | 23
> security/integrity/platform_certs/load_ipl_s390.c | 4
> security/integrity/platform_certs/load_powerpc.c | 4
> security/integrity/platform_certs/load_uefi.c | 4
> security/integrity/platform_certs/machine_keyring.c | 4
> security/integrity/platform_certs/platform_keyring.c | 14
> security/ipe/fs.c | 4
> security/ipe/ipe.c | 4
> security/ipe/ipe.h | 2
> security/landlock/setup.c | 3
> security/loadpin/loadpin.c | 16
> security/lockdown/lockdown.c | 6
> security/lsm.h | 46
> security/lsm_init.c | 566 ++++++++++
> security/lsm_notifier.c | 31
> security/lsm_syscalls.c | 8
> security/min_addr.c | 5
> security/safesetid/lsm.c | 4
> security/safesetid/lsm.h | 2
> security/safesetid/securityfs.c | 3
> security/security.c | 620 -----------
> security/selinux/Makefile | 2
> security/selinux/hooks.c | 12
> security/selinux/ibpkey.c | 5
> security/selinux/include/audit.h | 5
> security/selinux/include/initcalls.h | 19
> security/selinux/initcalls.c | 50
> security/selinux/netif.c | 5
> security/selinux/netlink.c | 5
> security/selinux/netnode.c | 5
> security/selinux/netport.c | 5
> security/selinux/selinuxfs.c | 5
> security/selinux/ss/services.c | 26
> security/smack/smack.h | 6
> security/smack/smack_lsm.c | 19
> security/smack/smack_netfilter.c | 4
> security/smack/smackfs.c | 4
> security/tomoyo/common.h | 2
> security/tomoyo/securityfs_if.c | 4
> security/tomoyo/tomoyo.c | 4
> security/yama/yama_lsm.c | 3
> 58 files changed, 1102 insertions(+), 724 deletions(-)
>
More information about the Linux-security-module-archive
mailing list