On 2/6/2023 5:04 AM, KP Singh wrote: > On Fri, Jan 20, 2023 at 5:36 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote: >> On Fri, Jan 20, 2023 at 01:08:17AM +0100, KP Singh wrote: >>> The indirect calls are not really needed as one knows the addresses of > [...] > >>> +/* >>> + * Define static calls and static keys for each LSM hook. >>> + */ >>> + >>> +#define DEFINE_LSM_STATIC_CALL(NUM, NAME, RET, ...) \ >>> + DEFINE_STATIC_CALL_NULL(LSM_STATIC_CALL(NAME, NUM), \ >>> + *((RET(*)(__VA_ARGS__))NULL)); \ >>> + DEFINE_STATIC_KEY_FALSE(SECURITY_HOOK_ENABLED_KEY(NAME, NUM)); >> Hm, another place where we would benefit from having separated logic for >> "is it built?" and "is it enabled by default?" and we could use >> DEFINE_STATIC_KEY_MAYBE(). But, since we don't, I think we need to use >> DEFINE_STATIC_KEY_TRUE() here or else won't all the calls be >> out-of-line? (i.e. the default compiled state will be NOPs?) If we're >> trying to optimize for having LSMs, I think we should default to inline >> calls. (The machine code in the commit log seems to indicate that they >> are out of line -- it uses jumps.) >> > I should have added it in the commit description, actually we are > optimizing for "hot paths are less likely to have LSM hooks enabled" > (eg. socket_sendmsg). How did you come to that conclusion? Where is there a correlation between "hot path" and "less likely to be enabled"? > But I do see that there are LSMs that have these > enabled. Maybe we can put this behind a config option, possibly > depending on CONFIG_EXPERT? Help me, as the maintainer of one of those LSMs, understand why that would be a good idea.
