On Tue, Apr 18, 2017 at 4:16 PM, Casey Schaufler [off-list ref] wrote:

On 4/18/2017 3:44 PM, Mickaël Salaün wrote:

quoted

On 19/04/2017 00:17, Kees Cook wrote:

quoted

On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün [off-list ref] wrote:

quoted

+void __init landlock_add_hooks(void)
+{
+       pr_info("landlock: Version %u", LANDLOCK_VERSION);
+       landlock_add_hooks_fs();
+       security_add_hooks(NULL, 0, "landlock");
+       bpf_register_prog_type(&bpf_landlock_type);

I'm confused by the separation of hook registration here. The call to
security_add_hooks is with count=0 is especially weird. Why isn't this
just a single call with security_add_hooks(landlock_hooks,
ARRAY_SIZE(landlock_hooks), "landlock")?

Yes, this is ugly with the new security_add_hooks() with three arguments
but I wanted to split the hooks definition in multiple files.

Why? I'll buy a good argument, but there are dangers in
allowing multiple calls to security_add_hooks().

quoted

The current security_add_hooks() use lsm_append(lsm, &lsm_names) which
is not exported. Unfortunately, calling multiple security_add_hooks()
with the same LSM name would register multiple names for the same LSM…
Is it OK if I modify this function to not add duplicated entries?

It may seem absurd, but it's conceivable that a module might
have two hooks it wants called. My example is a module that
counts the number of times SELinux denies a process access to
things (which needs to be called before and after SELinux in
order to detect denials) and takes "appropriate action" if
too many denials occur. It would be weird, wonky and hackish,
but that never stopped anybody before.

If ends up being sane and clear, I'm fine with allowing multiple calls.

-Kees

-- 
Kees Cook
Pixel Security