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

[KP Singh]

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