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
Headers show
Series Reduce overhead of LSMs with static calls | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-30 success Logs for veristat
netdev/tree_selection success Not a local patch, async
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on s390x with gcc

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 */