[PATCH v2 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY

[KP Singh]

On Tue, Jun 20, 2023 at 10:59 PM Kees Cook <keescook at chromium.org> wrote:
>
> On Fri, Jun 16, 2023 at 02:04:41AM +0200, KP Singh wrote:
> > [...]
> > @@ -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