On 2/6/2023 5:04 AM, KP Singh wrote:

On Fri, Jan 20, 2023 at 5:36 AM Kees Cook [off-list ref] wrote:

quoted

On Fri, Jan 20, 2023 at 01:08:17AM +0100, KP Singh wrote:

quoted

The indirect calls are not really needed as one knows the addresses of

[...]

quoted

quoted

+/*
+ * 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.