diff mbox series

[security-next,v5,12/30] LSM: Provide separate ordered initialization

Message ID 20181011001846.30964-13-keescook@chromium.org (mailing list archive)
State New, archived
Headers show
Series LSM: Explict ordering | expand

Commit Message

Kees Cook Oct. 11, 2018, 12:18 a.m. UTC 
This provides a place for ordered LSMs to be initialized, separate from
the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
ordered_lsm_init(), but it will change drastically in later patches.

What is not obvious in the patch is that this change moves the integrity
LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
list, there is no reordering yet created.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
---
 security/security.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Mimi Zohar Nov. 2, 2018, 6:13 p.m. UTC | #1 
Hi Kees,

On Wed, 2018-10-10 at 17:18 -0700, Kees Cook wrote:
> This provides a place for ordered LSMs to be initialized, separate from
> the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
> ordered_lsm_init(), but it will change drastically in later patches.
> 
> What is not obvious in the patch is that this change moves the integrity
> LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
> with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
> list, there is no reordering yet created.

I'm so sorry for not reviewing these patches earlier.  Both IMA and
EVM are dependent on "integrity", but "integrity", at least by itself,
should not be considered an LSM.

I don't recall why "integrity" is on the security_initcall, while both
IMA and EVM are on the late_initcall().

Mimi

> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
>  security/security.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/security/security.c b/security/security.c
> index 2055af907eba..ebbbb672ced5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,12 +52,30 @@ static __initdata bool debug;
>  			pr_info(__VA_ARGS__);			\
>  	} while (0)
> 
> +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);
> +	}
> +}
> +
>  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);
> @@ -87,6 +105,9 @@ int __init security_init(void)
>  	yama_add_hooks();
>  	loadpin_add_hooks();
> 
> +	/* Load LSMs in specified order. */
> +	ordered_lsm_init();
> +
>  	/*
>  	 * Load all the remaining security modules.
>  	 */
Kees Cook Nov. 2, 2018, 8:49 p.m. UTC | #2 
On Fri, Nov 2, 2018 at 11:13 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> I don't recall why "integrity" is on the security_initcall, while both
> IMA and EVM are on the late_initcall().

It's because integrity needs to have a VFS buffer allocated extremely
early, so it used the security init to do it. While it's not an LSM,
it does use this part of LSM infrastructure. I didn't see an obvious
alternative at the time, but now that I think about it, maybe just a
simple postcore_initcall() would work?

-Kees
Mimi Zohar Nov. 5, 2018, 2:13 p.m. UTC | #3 
On Fri, 2018-11-02 at 13:49 -0700, Kees Cook wrote:
> On Fri, Nov 2, 2018 at 11:13 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > I don't recall why "integrity" is on the security_initcall, while both
> > IMA and EVM are on the late_initcall().
> 
> It's because integrity needs to have a VFS buffer allocated extremely
> early, so it used the security init to do it. While it's not an LSM,
> it does use this part of LSM infrastructure. I didn't see an obvious
> alternative at the time, but now that I think about it, maybe just a
> simple postcore_initcall() would work?

I was questioning why the "security_initcall", which is called after
the late_initcall.  Moving it to the postcore_initcall, before the
late_initcall, sounds right.

Mimi
diff mbox series

Patch

diff --git a/security/security.c b/security/security.c
index 2055af907eba..ebbbb672ced5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -52,12 +52,30 @@  static __initdata bool debug;
 			pr_info(__VA_ARGS__);			\
 	} while (0)
 
+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);
+	}
+}
+
 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);
@@ -87,6 +105,9 @@  int __init security_init(void)
 	yama_add_hooks();
 	loadpin_add_hooks();
 
+	/* Load LSMs in specified order. */
+	ordered_lsm_init();
+
 	/*
 	 * Load all the remaining security modules.
 	 */