On Tue, Jun 20, 2023 at 10:59 PM Kees Cook <keescook@xxxxxxxxxxxx> 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
