On 09/24/2018 05:18 PM, Kees Cook wrote:
> As a prerequisite to adjusting LSM selection logic in the future, this
> moves the selection logic up out of the individual major LSMs, making
> their init functions only run when actually enabled. This considers all
> LSMs enabled by default unless they specified an external "enable"
> variable.
>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
Reviewed-by: John Johansen <john.johansen@xxxxxxxxxxxxx>
> ---
> include/linux/lsm_hooks.h | 1 -
> security/apparmor/lsm.c | 6 ---
> security/security.c | 84 ++++++++++++++++++++++++--------------
> security/selinux/hooks.c | 10 -----
> security/smack/smack_lsm.c | 3 --
> security/tomoyo/tomoyo.c | 2 -
> 6 files changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 2a41e8e6f6e5..95798f212dbf 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2091,7 +2091,6 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
> #define __lsm_ro_after_init __ro_after_init
> #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
>
> -extern int __init security_module_enable(const char *module);
> extern void __init capability_add_hooks(void);
> #ifdef CONFIG_SECURITY_YAMA
> extern void __init yama_add_hooks(void);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index d03133a267f2..5399c2f03536 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1542,12 +1542,6 @@ static int __init apparmor_init(void)
> {
> int error;
>
> - if (!apparmor_enabled || !security_module_enable("apparmor")) {
> - aa_info_message("AppArmor disabled by boot time parameter");
> - apparmor_enabled = false;
> - return 0;
> - }
> -
> aa_secids_init();
>
> error = aa_setup_dfa_engine();
> diff --git a/security/security.c b/security/security.c
> index a886a978214a..056b36cf6245 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,33 +52,78 @@ static bool debug __initdata;
> pr_info(__VA_ARGS__); \
> } while (0)
>
> +static bool __init is_enabled(struct lsm_info *lsm)
> +{
> + if (!lsm->enabled || *lsm->enabled)
> + return true;
> +
> + return false;
> +}
> +
> +/* Mark an LSM's enabled flag, if it exists. */
> +static void __init set_enabled(struct lsm_info *lsm, bool enabled)
> +{
> + if (lsm->enabled)
> + *lsm->enabled = enabled;
> +}
> +
> +/* Is an LSM allowed to be initialized? */
> +static bool __init lsm_allowed(struct lsm_info *lsm)
> +{
> + /* Skip if the LSM is disabled. */
> + if (!is_enabled(lsm))
> + return false;
> +
> + /* Skip major-specific checks if not a major LSM. */
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
> + return true;
> +
> + /* Disabled if this LSM isn't the chosen one. */
> + if (strcmp(lsm->name, chosen_lsm) != 0)
> + return false;
> +
> + return true;
> +}
> +
> +/* Check if LSM should be enabled. Mark any that are disabled. */
> +static void __init maybe_initialize_lsm(struct lsm_info *lsm)
> +{
> + int enabled = lsm_allowed(lsm);
> +
> + /* Record enablement. */
> + set_enabled(lsm, enabled);
> +
> + /* If selected, initialize the LSM. */
> + if (enabled) {
> + int ret;
> +
> + init_debug("initializing %s\n", lsm->name);
> + ret = lsm->init();
> + WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + }
> +}
> +
> static void __init ordered_lsm_init(void)
> {
> struct lsm_info *lsm;
> - int ret;
>
> for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
> continue;
>
> - init_debug("initializing %s\n", lsm->name);
> - ret = lsm->init();
> - WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + maybe_initialize_lsm(lsm);
> }
> }
>
> static void __init major_lsm_init(void)
> {
> struct lsm_info *lsm;
> - int ret;
>
> for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
> continue;
>
> - init_debug("initializing %s\n", lsm->name);
> - ret = lsm->init();
> - WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + maybe_initialize_lsm(lsm);
> }
> }
>
> @@ -168,29 +213,6 @@ static int lsm_append(char *new, char **result)
> return 0;
> }
>
> -/**
> - * security_module_enable - Load given security module on boot ?
> - * @module: the name of the module
> - *
> - * Each LSM must pass this method before registering its own operations
> - * to avoid security registration races. This method may also be used
> - * to check if your LSM is currently loaded during kernel initialization.
> - *
> - * Returns:
> - *
> - * true if:
> - *
> - * - The passed LSM is the one chosen by user at boot time,
> - * - or the passed LSM is configured as the default and the user did not
> - * choose an alternate LSM at boot time.
> - *
> - * Otherwise, return false.
> - */
> -int __init security_module_enable(const char *module)
> -{
> - return !strcmp(module, chosen_lsm);
> -}
> -
> /**
> * security_add_hooks - Add a modules hooks to the hook lists.
> * @hooks: the hooks to add
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3f999ed98cfd..409a9252aeb6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7133,16 +7133,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>
> static __init int selinux_init(void)
> {
> - if (!security_module_enable("selinux")) {
> - selinux_enabled = 0;
> - return 0;
> - }
> -
> - if (!selinux_enabled) {
> - pr_info("SELinux: Disabled at boot.\n");
> - return 0;
> - }
> -
> pr_info("SELinux: Initializing.\n");
>
> memset(&selinux_state, 0, sizeof(selinux_state));
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 4aef844fc0e2..e79fad43a8e3 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4834,9 +4834,6 @@ static __init int smack_init(void)
> struct cred *cred;
> struct task_smack *tsp;
>
> - if (!security_module_enable("smack"))
> - return 0;
> -
> smack_inode_cache = KMEM_CACHE(inode_smack, 0);
> if (!smack_inode_cache)
> return -ENOMEM;
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index 528b6244a648..39bb994ebe09 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -540,8 +540,6 @@ static int __init tomoyo_init(void)
> {
> struct cred *cred = (struct cred *) current_cred();
>
> - if (!security_module_enable("tomoyo"))
> - return 0;
> /* register ourselves with the security framework */
> security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
> printk(KERN_INFO "TOMOYO Linux initialized\n");
>
