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

[Rasmus Villemoes <rasmus.villemoes@prevas.dk>]

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