Re: [PATCH security-next v3 14/29] LSM: Plumb visibility into optional "enabled" state

On Mon, Oct 1, 2018 at 2:47 PM, James Morris [off-list ref] wrote:

On Mon, 24 Sep 2018, Kees Cook wrote:

quoted

In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.

Signed-off-by: Kees Cook <redacted>
---
 include/linux/lsm_hooks.h | 1 +
 security/apparmor/lsm.c   | 5 +++--
 security/selinux/hooks.c  | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5056f7374b3d..2a41e8e6f6e5 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
 struct lsm_info {
      const char *name;       /* Populated automatically. */
      unsigned long flags;    /* Optional: flags describing LSM */
+     int *enabled;           /* Optional: NULL means enabled. */

This seems potentially confusing.

Perhaps initialize 'enabled' to a default int pointer, like:

        static int lsm_default_enabled = 1;

Then,

        DEFINE_LSM(foobar)
        flags = LSM_FLAG_LEGACY_MAJOR,
        .enabled = &lsm_default_enabled,
        .init = foobar_init,
        END_LSM;

The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).

However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...

-Kees

-- 
Kees Cook
Pixel Security