diff mbox series

[v4,2/5] security: Count the LSMs enabled at compile time

Message ID 20230922145505.4044003-3-kpsingh@kernel.org (mailing list archive)
State Superseded
Delegated to: Paul Moore
Headers show
Series Reduce overhead of LSMs with static calls | expand

Commit Message

KP Singh Sept. 22, 2023, 2:55 p.m. UTC
These macros are a clever trick to determine a count of the number of
LSMs that are enabled in the config to ascertain the maximum number of
static calls that need to be configured per LSM hook.

Without this one would need to generate static calls for the total
number of LSMs in the kernel (even if they are not compiled) times the
number of LSM hooks which ends up being quite wasteful.

Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/lsm_count.h | 107 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 include/linux/lsm_count.h

Comments

Kees Cook Sept. 22, 2023, 3:50 p.m. UTC | #1
On Fri, Sep 22, 2023 at 04:55:02PM +0200, KP Singh wrote:
> These macros are a clever trick to determine a count of the number of
> LSMs that are enabled in the config to ascertain the maximum number of
> static calls that need to be configured per LSM hook.
> 
> Without this one would need to generate static calls for the total
> number of LSMs in the kernel (even if they are not compiled) times the
> number of LSM hooks which ends up being quite wasteful.
> 
> Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Acked-by: Song Liu <song@kernel.org>
> Signed-off-by: KP Singh <kpsingh@kernel.org>

Thought below, but regardless of result:

Reviewed-by: Kees Cook <keescook@chromium.org>


> ---
>  include/linux/lsm_count.h | 107 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
>  create mode 100644 include/linux/lsm_count.h
> 
> diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
> new file mode 100644
> index 000000000000..4d6dac6efb75
> --- /dev/null
> +++ b/include/linux/lsm_count.h
> @@ -0,0 +1,107 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2023 Google LLC.
> + */
> +
> +#ifndef __LINUX_LSM_COUNT_H
> +#define __LINUX_LSM_COUNT_H
> +
> +#include <linux/args.h>
> +
> +#ifdef CONFIG_SECURITY
> +
> +/*
> + * Macros to count the number of LSMs enabled in the kernel at compile time.
> + */
> +
> +/*
> + * Capabilities is enabled when CONFIG_SECURITY is enabled.
> + */
> +#if IS_ENABLED(CONFIG_SECURITY)
> +#define CAPABILITIES_ENABLED 1,
> +#else
> +#define CAPABILITIES_ENABLED
> +#endif

We're in an #ifdef CONFIG_SECURITY, so CAPABILITIES_ENABLED will always
be set. As such, we could leave off the trailing comma and list it
_last_ in the macro, and then ...

> +/*
> + *  There is a trailing comma that we need to be accounted for. This is done by
> + *  using a skipped argument in __COUNT_LSMS
> + */
> +#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args)
> +#define COUNT_LSMS(args...) __COUNT_LSMS(args)

This wouldn't be needed...

> +
> +#define MAX_LSM_COUNT			\
> +	COUNT_LSMS(			\
> +		CAPABILITIES_ENABLED	\
> +		SELINUX_ENABLED		\
> +		SMACK_ENABLED		\
> +		APPARMOR_ENABLED	\
> +		TOMOYO_ENABLED		\
> +		YAMA_ENABLED		\
> +		LOADPIN_ENABLED		\
> +		LOCKDOWN_ENABLED	\
> +		BPF_LSM_ENABLED		\
> +		LANDLOCK_ENABLED)


	COUNT_ARGS(			\
		SELINUX_ENABLED		\
		SMACK_ENABLED		\
		...
		CAPABILITIES_ENABLED)

-Kees
KP Singh Sept. 22, 2023, 4:07 p.m. UTC | #2
On Fri, Sep 22, 2023 at 5:50 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Sep 22, 2023 at 04:55:02PM +0200, KP Singh wrote:
> > These macros are a clever trick to determine a count of the number of
> > LSMs that are enabled in the config to ascertain the maximum number of
> > static calls that need to be configured per LSM hook.
> >
> > Without this one would need to generate static calls for the total
> > number of LSMs in the kernel (even if they are not compiled) times the
> > number of LSM hooks which ends up being quite wasteful.
> >
> > Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
> > Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> > Acked-by: Song Liu <song@kernel.org>
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
>
> Thought below, but regardless of result:
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
>
> > ---
> >  include/linux/lsm_count.h | 107 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 107 insertions(+)
> >  create mode 100644 include/linux/lsm_count.h
> >
> > diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
> > new file mode 100644
> > index 000000000000..4d6dac6efb75
> > --- /dev/null
> > +++ b/include/linux/lsm_count.h
> > @@ -0,0 +1,107 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +/*
> > + * Copyright (C) 2023 Google LLC.
> > + */
> > +
> > +#ifndef __LINUX_LSM_COUNT_H
> > +#define __LINUX_LSM_COUNT_H
> > +
> > +#include <linux/args.h>
> > +
> > +#ifdef CONFIG_SECURITY
> > +
> > +/*
> > + * Macros to count the number of LSMs enabled in the kernel at compile time.
> > + */
> > +
> > +/*
> > + * Capabilities is enabled when CONFIG_SECURITY is enabled.
> > + */
> > +#if IS_ENABLED(CONFIG_SECURITY)
> > +#define CAPABILITIES_ENABLED 1,
> > +#else
> > +#define CAPABILITIES_ENABLED
> > +#endif
>
> We're in an #ifdef CONFIG_SECURITY, so CAPABILITIES_ENABLED will always
> be set. As such, we could leave off the trailing comma and list it
> _last_ in the macro, and then ...
>
> > +/*
> > + *  There is a trailing comma that we need to be accounted for. This is done by
> > + *  using a skipped argument in __COUNT_LSMS
> > + */
> > +#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args)
> > +#define COUNT_LSMS(args...) __COUNT_LSMS(args)
>
> This wouldn't be needed...

Slight preference for explicitly having the capabilities listed than
implicitly over counting. But no strong opinion, fine with either
approches.

>
> > +
> > +#define MAX_LSM_COUNT                        \
> > +     COUNT_LSMS(                     \
> > +             CAPABILITIES_ENABLED    \
> > +             SELINUX_ENABLED         \
> > +             SMACK_ENABLED           \
> > +             APPARMOR_ENABLED        \
> > +             TOMOYO_ENABLED          \
> > +             YAMA_ENABLED            \
> > +             LOADPIN_ENABLED         \
> > +             LOCKDOWN_ENABLED        \
> > +             BPF_LSM_ENABLED         \
> > +             LANDLOCK_ENABLED)
>
>
>         COUNT_ARGS(                     \
>                 SELINUX_ENABLED         \
>                 SMACK_ENABLED           \
>                 ...
>                 CAPABILITIES_ENABLED)
>
> -Kees
>
> --
> Kees Cook
KP Singh Sept. 27, 2023, 10:37 p.m. UTC | #3
On Fri, Sep 22, 2023 at 6:07 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Fri, Sep 22, 2023 at 5:50 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Sep 22, 2023 at 04:55:02PM +0200, KP Singh wrote:
> > > These macros are a clever trick to determine a count of the number of
> > > LSMs that are enabled in the config to ascertain the maximum number of
> > > static calls that need to be configured per LSM hook.
> > >
> > > Without this one would need to generate static calls for the total
> > > number of LSMs in the kernel (even if they are not compiled) times the
> > > number of LSM hooks which ends up being quite wasteful.
> > >
> > > Suggested-by: Kui-Feng Lee <sinquersw@gmail.com>
> > > Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> > > Acked-by: Song Liu <song@kernel.org>
> > > Signed-off-by: KP Singh <kpsingh@kernel.org>
> >
> > Thought below, but regardless of result:
> >
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> >
> >
> > > ---
> > >  include/linux/lsm_count.h | 107 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 107 insertions(+)
> > >  create mode 100644 include/linux/lsm_count.h
> > >
> > > diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
> > > new file mode 100644
> > > index 000000000000..4d6dac6efb75
> > > --- /dev/null
> > > +++ b/include/linux/lsm_count.h
> > > @@ -0,0 +1,107 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +/*
> > > + * Copyright (C) 2023 Google LLC.
> > > + */
> > > +
> > > +#ifndef __LINUX_LSM_COUNT_H
> > > +#define __LINUX_LSM_COUNT_H
> > > +
> > > +#include <linux/args.h>
> > > +
> > > +#ifdef CONFIG_SECURITY
> > > +
> > > +/*
> > > + * Macros to count the number of LSMs enabled in the kernel at compile time.
> > > + */
> > > +
> > > +/*
> > > + * Capabilities is enabled when CONFIG_SECURITY is enabled.
> > > + */
> > > +#if IS_ENABLED(CONFIG_SECURITY)
> > > +#define CAPABILITIES_ENABLED 1,
> > > +#else
> > > +#define CAPABILITIES_ENABLED
> > > +#endif
> >
> > We're in an #ifdef CONFIG_SECURITY, so CAPABILITIES_ENABLED will always
> > be set. As such, we could leave off the trailing comma and list it
> > _last_ in the macro, and then ...
> >
> > > +/*
> > > + *  There is a trailing comma that we need to be accounted for. This is done by
> > > + *  using a skipped argument in __COUNT_LSMS
> > > + */
> > > +#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args)
> > > +#define COUNT_LSMS(args...) __COUNT_LSMS(args)
> >
> > This wouldn't be needed...
>
> Slight preference for explicitly having the capabilities listed than
> implicitly over counting. But no strong opinion, fine with either
> approches.

Actually it's not just a preference but really required. When the
CAPABILITIES is absent and all other LSMs are disabled it leads to
COUNT_ARGS() which evaluates to 0

This also happens here:

https://lore.kernel.org/bpf/202309271206.d7fb60f9-oliver.sang@intel.com/

and to fix this we need:

-#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args)
+#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args...)

And I checked the edge cases with a simple c file

int test(void) {
   int count = MAX_LSM_COUNT;
}

and make security/count.i:

for just CONFIG_SECURITY enabled:

int test(void) {
  int count = 1;
}

with another LSM:

int test(void) {
  int count = 2;
}


- KP
>
> >
> > > +
> > > +#define MAX_LSM_COUNT                        \
> > > +     COUNT_LSMS(                     \
> > > +             CAPABILITIES_ENABLED    \
> > > +             SELINUX_ENABLED         \
> > > +             SMACK_ENABLED           \
> > > +             APPARMOR_ENABLED        \
> > > +             TOMOYO_ENABLED          \
> > > +             YAMA_ENABLED            \
> > > +             LOADPIN_ENABLED         \
> > > +             LOCKDOWN_ENABLED        \
> > > +             BPF_LSM_ENABLED         \
> > > +             LANDLOCK_ENABLED)
> >
> >
> >         COUNT_ARGS(                     \
> >                 SELINUX_ENABLED         \
> >                 SMACK_ENABLED           \
> >                 ...
> >                 CAPABILITIES_ENABLED)
> >
> > -Kees
> >
> > --
> > Kees Cook
diff mbox series

Patch

diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
new file mode 100644
index 000000000000..4d6dac6efb75
--- /dev/null
+++ b/include/linux/lsm_count.h
@@ -0,0 +1,107 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2023 Google LLC.
+ */
+
+#ifndef __LINUX_LSM_COUNT_H
+#define __LINUX_LSM_COUNT_H
+
+#include <linux/args.h>
+
+#ifdef CONFIG_SECURITY
+
+/*
+ * Macros to count the number of LSMs enabled in the kernel at compile time.
+ */
+
+/*
+ * Capabilities is enabled when CONFIG_SECURITY is enabled.
+ */
+#if IS_ENABLED(CONFIG_SECURITY)
+#define CAPABILITIES_ENABLED 1,
+#else
+#define CAPABILITIES_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_SELINUX)
+#define SELINUX_ENABLED 1,
+#else
+#define SELINUX_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_SMACK)
+#define SMACK_ENABLED 1,
+#else
+#define SMACK_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_APPARMOR)
+#define APPARMOR_ENABLED 1,
+#else
+#define APPARMOR_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_TOMOYO)
+#define TOMOYO_ENABLED 1,
+#else
+#define TOMOYO_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_YAMA)
+#define YAMA_ENABLED 1,
+#else
+#define YAMA_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_LOADPIN)
+#define LOADPIN_ENABLED 1,
+#else
+#define LOADPIN_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM)
+#define LOCKDOWN_ENABLED 1,
+#else
+#define LOCKDOWN_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_BPF_LSM)
+#define BPF_LSM_ENABLED 1,
+#else
+#define BPF_LSM_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_SECURITY_LANDLOCK)
+#define LANDLOCK_ENABLED 1,
+#else
+#define LANDLOCK_ENABLED
+#endif
+
+/*
+ *  There is a trailing comma that we need to be accounted for. This is done by
+ *  using a skipped argument in __COUNT_LSMS
+ */
+#define __COUNT_LSMS(skipped_arg, args...) COUNT_ARGS(args)
+#define COUNT_LSMS(args...) __COUNT_LSMS(args)
+
+#define MAX_LSM_COUNT			\
+	COUNT_LSMS(			\
+		CAPABILITIES_ENABLED	\
+		SELINUX_ENABLED		\
+		SMACK_ENABLED		\
+		APPARMOR_ENABLED	\
+		TOMOYO_ENABLED		\
+		YAMA_ENABLED		\
+		LOADPIN_ENABLED		\
+		LOCKDOWN_ENABLED	\
+		BPF_LSM_ENABLED		\
+		LANDLOCK_ENABLED)
+
+#else
+
+#define MAX_LSM_COUNT 0
+
+#endif /* CONFIG_SECURITY */
+
+#endif  /* __LINUX_LSM_COUNT_H */