[PATCH RESEND bpf-next 3/4] security: Replace indirect LSM hook calls with static calls

[Casey Schaufler]

On 2/6/2023 5:04 AM, KP Singh wrote:
> On Fri, Jan 20, 2023 at 5:36 AM Kees Cook <keescook at chromium.org> 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.