Messages in this thread | | | From | Kees Cook <> | Date | Sat, 31 Mar 2018 07:38:54 -0700 | Subject | Re: [PATCH v3 1/1] security: Add mechanism to safely (un)load LSMs after boot time |
| |
On Fri, Mar 30, 2018 at 11:16 PM, Sargun Dhillon <sargun@sargun.me> wrote: > On Fri, Mar 30, 2018 at 2:39 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >>> static struct security_hook_list null_hooks[SECURITY_HOOK_COUNT]; >>> -#define HAS_FUNC(SHL, FUNC) (SHL->hook.FUNC) >>> +DEFINE_STATIC_SRCU(security_hook_srcu); >>> + >>> +static inline bool is_null_hook(struct security_hook_list *shl) >>> +{ >>> + union { >>> + void *cb_ptr; >>> + union security_list_options slo; >>> + } hook_options; >>> + >>> + hook_options.slo = shl->hook; >>> + return !hook_options.cb_ptr; >>> +} >> >> I like the HAS_FUNC() approach better. > > Just curious, why? I personally prefer small static inline functions > over macros, if possible.
Generally speaking, small static inline functions are better since they provide type-checking. In this case, though, it looks like you're just doing a cast, but with a union. Why isn't this just:
return !!((uintptr_t)shl->hook)
?
Though the security_list_options union exists for callback type checking, so really, having HAS_FUNC() with the explicit function you're interested in creates a bit of self-documenting code (even if it always resolves to the above test).
-Kees
-- Kees Cook Pixel Security
| |