On Tue, Jun 20, 2023 at 10:59 PM Kees Cook [off-list ref] wrote:

On Fri, Jun 16, 2023 at 02:04:41AM +0200, KP Singh wrote:

quoted

[...]

@@ -110,6 +110,9 @@ static __initdata struct lsm_info *exclusive;
 #undef LSM_HOOK
 #undef DEFINE_LSM_STATIC_CALL

+#define security_hook_active(n, h) \
+     static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY, &SECURITY_HOOK_ACTIVE_KEY(h, n))
+
 /*
  * Initialise a table of static calls for each LSM hook.
  * DEFINE_STATIC_CALL_NULL invocation above generates a key (STATIC_CALL_KEY)

@@ -816,7 +819,7 @@ static int lsm_superblock_alloc(struct super_block *sb)
  */
 #define __CALL_STATIC_VOID(NUM, HOOK, ...)                                \
 do {                                                                      \
-     if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {    \
+     if (security_hook_active(NUM, HOOK)) {                               \
              static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);        \
      }                                                                    \
 } while (0);

@@ -828,7 +831,7 @@ do {                                                                           \

 #define __CALL_STATIC_INT(NUM, R, HOOK, LABEL, ...)                       \
 do {                                                                      \
-     if (static_branch_unlikely(&SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM))) {  \
+     if (security_hook_active(NUM, HOOK)) {    \
              R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);    \
              if (R != 0)                                                  \
                      goto LABEL;                                          \

I actually think I'd prefer there be no macro wrapping
static_branch_maybe(), just for reading it more easily. i.e. people
reading this code are going to expect the static_branch/static_call code
patterns, and seeing "security_hook_active" only slows them down in
understanding it. I don't think it's _that_ ugly to have it all typed
out. e.g.:

Done and agreed, especially given that this is behind a macro anyways.

        if (static_branch_maybe(CONFIG_SECURITY_HOOK_LIKELY,                 \
                                &SECURITY_HOOK_ACTIVE_KEY(HOOK, NUM)) {      \
                R = static_call(LSM_STATIC_CALL(HOOK, NUM))(__VA_ARGS__);    \
                if (R != 0)                                                  \
                        goto LABEL;                                          \



--
Kees Cook