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 <keesc...@chromium.org>

Reviewed-by: John Johansen <john.johan...@canonical.com>


> ---
>  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");
> 

Reply via email to