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"...
>
Yeah, LSMs are not meant to be used from a kernel module. The data
structure is actually __ro_after_init. So, I am not even sure how you
are using it in kernel modules (unless you are patching this out).
And, if you are really patching stuff to get your out of tree LSMs to
work, then you might as well add your "custom" LSM config here or just
override this count.
The performance benefits here outweigh the need for a completely
unsupported use case.
> >
> > 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
>
> struct lsm_static_calls_table {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> struct lsm_static_call NAME[MAX_LSM_COUNT];
> #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?
> Should "number of possible LSMs" be replaced with "number of built-in LSMs" ?
>
> > Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx
>
> Trailing ">" is missing.
>
> > +/*
> > + * 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) ?
You cannot do this because this is not evaluated in the preprocessor
and is used to generate the variable names. If you have a working
snippet of code, please share.
> 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
How is this an indirect addition? I am not following. The end goal is
that when the preprocessor runs MAX_LSM_COUNT is a constant number and
not an expression like (1 + 1 + 1) if you have ideas please share the
actual code.
- KP
>
