> On 3 Jul 2024, at 11:44, Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> wrote: > > KP Singh <kpsingh@xxxxxxxxxx> 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
