diff mbox series

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

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

Checks

Context Check Description
netdev/tree_selection success Not a local patch, async

Commit Message

KP Singh June 29, 2024, 8:43 a.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>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/args.h      |   6 +-
 include/linux/lsm_count.h | 128 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/lsm_count.h

Comments

Rasmus Villemoes July 3, 2024, 9:44 a.m. UTC | #1 
KP Singh <kpsingh@kernel.org> writes:

> 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.

[snip]

> diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
> new file mode 100644
> index 000000000000..73c7cc81349b
> --- /dev/null
> +++ b/include/linux/lsm_count.h
> @@ -0,0 +1,128 @@
> +/* 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
> +
[snip]
> +
> +#if IS_ENABLED(CONFIG_EVM)
> +#define EVM_ENABLED 1,
> +#else
> +#define EVM_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	\
> +		SAFESETID_ENABLED	\
> +		BPF_LSM_ENABLED		\
> +		LANDLOCK_ENABLED	\
> +		IMA_ENABLED		\
> +		EVM_ENABLED)
> +
> +#else
> +
> +#define MAX_LSM_COUNT 0
> +
> +#endif /* CONFIG_SECURITY */
> +
> +#endif  /* __LINUX_LSM_COUNT_H */

OK, so I can tell from the other patches that this isn't just about
getting MAX_LSM_COUNT to be a compile-time constant, it really has to be
a single preprocessor token representing the right decimal value. That
information could have been in some comment or the commit log. So

#define MAX_LSM_COUNT (IS_ENABLED(CONFIG_SECURITY) + IS_ENABLED(CONFIG_SECURITY_SELINUX) + ...)

doesn't work immediately. But this does provide not just a compile-time
constant, but a preprocessor constant, so:

Instead of all this trickery with defining temporary, never used again,
macros expanding to something with trailing comma or not, what about
this simpler (at least in terms of LOC, but IMO also readability)
approach:

/*
 * The sum of the IS_ENABLED() values provides the right value, but we
 * need MAX_LSM_COUNT to be a single preprocessor token representing
 * that value, because it will be passed to the UNROLL macro which
 * does token concatenation.
 */

#define __MAX_LSM_COUNT (\
  IS_ENABLED(CONFIG_SECURITY) /* capabilities */ + \
  IS_ENABLED(CONFIG_SECURITY_SELINUX) + \
  ... \
  IS_ENABLED(CONFIG_EVM) \
  )
#if   __MAX_LSM_COUNT == 0
#define MAX_LSM_COUNT 0
#elif __MAX_LSM_COUNT == 1
#define MAX_LSM_COUNT 1
#elif
...
#elif __MAX_LSM_COUNT == 15
#define MAX_LSM_COUNT 15
#else
#error "Too many LSMs, add an #elif case"
#endif

Rasmus
KP Singh July 3, 2024, 1:12 p.m. UTC | #2 
> On 3 Jul 2024, at 11:44, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
> KP Singh <kpsingh@kernel.org> writes:
> 
>> 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.
> 
> [snip]
> 
>> diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
>> new file mode 100644
>> index 000000000000..73c7cc81349b
>> --- /dev/null
>> +++ b/include/linux/lsm_count.h
>> @@ -0,0 +1,128 @@
>> +/* 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
>> +
> [snip]
>> +
>> +#if IS_ENABLED(CONFIG_EVM)
>> +#define EVM_ENABLED 1,
>> +#else
>> +#define EVM_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 \
>> + SAFESETID_ENABLED \
>> + BPF_LSM_ENABLED \
>> + LANDLOCK_ENABLED \
>> + IMA_ENABLED \
>> + EVM_ENABLED)
>> +
>> +#else
>> +
>> +#define MAX_LSM_COUNT 0
>> +
>> +#endif /* CONFIG_SECURITY */
>> +
>> +#endif  /* __LINUX_LSM_COUNT_H */
> 
> OK, so I can tell from the other patches that this isn't just about
> getting MAX_LSM_COUNT to be a compile-time constant, it really has to be
> a single preprocessor token representing the right decimal value. That
> information could have been in some comment or the commit log. So
> 
> #define MAX_LSM_COUNT (IS_ENABLED(CONFIG_SECURITY) + IS_ENABLED(CONFIG_SECURITY_SELINUX) + ...)
> 
> doesn't work immediately. But this does provide not just a compile-time
> constant, but a preprocessor constant, so:
> 
> Instead of all this trickery with defining temporary, never used again,
> macros expanding to something with trailing comma or not, what about
> this simpler (at least in terms of LOC, but IMO also readability)
> approach:
> 
> /*
> * The sum of the IS_ENABLED() values provides the right value, but we
> * need MAX_LSM_COUNT to be a single preprocessor token representing
> * that value, because it will be passed to the UNROLL macro which
> * does token concatenation.
> */
> 

I actually prefer the version we have now from a readability perspective, it makes it more explicit (the check about the CONFIG_* being enabled and counting them). let's keep this as an incremental change that you can propose :) once the patches are merged.

- KP


> #define __MAX_LSM_COUNT (\
>  IS_ENABLED(CONFIG_SECURITY) /* capabilities */ + \
>  IS_ENABLED(CONFIG_SECURITY_SELINUX) + \
>  ... \
>  IS_ENABLED(CONFIG_EVM) \
>  )
> #if   __MAX_LSM_COUNT == 0
> #define MAX_LSM_COUNT 0
> #elif __MAX_LSM_COUNT == 1
> #define MAX_LSM_COUNT 1
> #elif
> ...
> #elif __MAX_LSM_COUNT == 15
> #define MAX_LSM_COUNT 15
> #else
> #error "Too many LSMs, add an #elif case"
> #endif
> 
> Rasmus
Paul Moore July 3, 2024, 2:54 p.m. UTC | #3 
On Wed, Jul 3, 2024 at 9:12 AM KP Singh <kpsingh@kernel.org> wrote:
> > On 3 Jul 2024, at 11:44, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> > KP Singh <kpsingh@kernel.org> writes:
> >
> >> 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.

...

> > Instead of all this trickery with defining temporary, never used again,
> > macros expanding to something with trailing comma or not, what about
> > this simpler (at least in terms of LOC, but IMO also readability)
> > approach:

...

> I actually prefer the version we have now from a readability perspective, it makes it more explicit (the check about the CONFIG_* being enabled and counting them). let's keep this as an incremental change that you can propose :) once the patches are merged.

I prefer the original approach by KP as well, let's leave it as-is.
IMO, it's far from the worst of the macro shenanigans in this patchset
(or existing LSM code for that matter).
diff mbox series

Patch

diff --git a/include/linux/args.h b/include/linux/args.h
index 8ff60a54eb7d..2e8e65d975c7 100644
--- a/include/linux/args.h
+++ b/include/linux/args.h
@@ -17,9 +17,9 @@ 
  * that as _n.
  */
 
-/* This counts to 12. Any more, it will return 13th argument. */
-#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
-#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+/* This counts to 15. Any more, it will return 16th argument. */
+#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _n, X...) _n
+#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
 
 /* Concatenate two parameters, but allow them to be expanded beforehand. */
 #define __CONCAT(a, b) a ## b
diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
new file mode 100644
index 000000000000..73c7cc81349b
--- /dev/null
+++ b/include/linux/lsm_count.h
@@ -0,0 +1,128 @@ 
+/* 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_SECURITY_SAFESETID)
+#define SAFESETID_ENABLED 1,
+#else
+#define SAFESETID_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
+
+#if IS_ENABLED(CONFIG_IMA)
+#define IMA_ENABLED 1,
+#else
+#define IMA_ENABLED
+#endif
+
+#if IS_ENABLED(CONFIG_EVM)
+#define EVM_ENABLED 1,
+#else
+#define EVM_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	\
+		SAFESETID_ENABLED	\
+		BPF_LSM_ENABLED		\
+		LANDLOCK_ENABLED	\
+		IMA_ENABLED		\
+		EVM_ENABLED)
+
+#else
+
+#define MAX_LSM_COUNT 0
+
+#endif /* CONFIG_SECURITY */
+
+#endif  /* __LINUX_LSM_COUNT_H */