On Thu, Sep 21, 2023 at 3:21 PM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 2023/09/19 6:24, 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.
>
> As a LKM-based LSM user, indirect function calls using a linked list have
> an advantage which this series kills. There always is a situation where a
> LSM cannot be built into vmlinux (and hence has to be loaded as a LKM-based
> LSM) due to distributor's support policy. Therefore, honestly speaking,
> I don't want LSM infrastructure to define the maximum number of "slots" or
> "static calls"...
>
> >
> > Without this one would need to generate static calls for (number of
> > possible LSMs * number of LSM hooks) which ends up being quite wasteful
> > especially when some LSMs are not compiled into the kernel.
>
> I can't interpret "number of possible LSMs * number of LSM hooks" part.
> Is this tokenized as "number of possible LSMs" (an integer) * (multipled by)
> "number of LSM hooks" (an integer) ? But the next patch includes
>
The tokenization is in the name of the static call slots. you cannot
have __SCT__lsm_static_call_bprm_check_security_1+1+1 it's not a valid
name. You may want to build security/security.i to see what's going on
(and actually try disabling some of the DEFINE_STATIC_CALL macros to
reduce further expansion of macros.
> struct lsm_static_calls_table {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> struct lsm_static_call NAME[MAX_LSM_COUNT];
Each LSM that is compiled in the kernel can theoretically register a
callback, so we add MAX_LSM_COUNT slots. Now the word "possible"
because one may compile the LSM but not choose to enable it with the
lsm= parameter.
> #include <linux/lsm_hook_defs.h>
> #undef LSM_HOOK
> } __randomize_layout;
>
> which seems to me that lsm_static_calls_table will get "number of possible
> LSMs" static calls for each LSM hook defined in linux/lsm_hook_defs.h .
> How did this patch help reducing static calls? What does "possible LSMs" mean?
If the kernel is compiled only with CONFIG_BPF_LSM, CONFIG_SELINUX and
CONFIG_SECURITY (for capabilities) and not any other LSM, then one
does not need to generate 12 slots for all each LSM hook when there
are only 3 LSMs compiled in (capabilities being implicitly behind
CONFIG_SECURITY).
> Should "number of possible LSMs" be replaced with "number of built-in LSMs" ?
Sure. I think "compiled LSMs" is a better word here.
>
> > Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx
>
> Trailing ">" is missing.
Fixed.
>
> > +/*
> > + * Macros to count the number of LSMs enabled in the kernel at compile time.
> > + */
> > +#define MAX_LSM_COUNT \
> > + ___COUNT_COMMAS( \
> > + CAPABILITIES_ENABLED \
> > + SELINUX_ENABLED \
> > + SMACK_ENABLED \
> > + APPARMOR_ENABLED \
> > + TOMOYO_ENABLED \
> > + YAMA_ENABLED \
> > + LOADPIN_ENABLED \
> > + LOCKDOWN_ENABLED \
> > + BPF_LSM_ENABLED \
> > + LANDLOCK_ENABLED)
>
> Since IS_ENABLED(CONFIG_FOO) is evaluated to either 1 or 0, why can't you directly
> do like IS_ENABLED(CONFIG_FOO) + IS_ENABLED(CONFIG_BAR) + IS_ENABLED(CONFIG_BUZ) ?
> If you can't do direct "+", can't you still do indirect "+" like something below?
>
> #if IS_ENABLED(CONFIG_FOO)
> #define FOO_ENABLED 1
> #else
> #define FOO_ENABLED 0
> #endif
>
