Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1626831

Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On 19/04/2017 01:40, Kees Cook wrote:
> On Tue, Apr 18, 2017 at 4:16 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 4/18/2017 3:44 PM, Mickaël Salaün wrote:
>>> On 19/04/2017 00:17, Kees Cook wrote:
>>>> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>>> +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().

I prefer to have one file per hook "family" (e.g. filesystem, network,
ptrace…). This reduce the mess with all the included files (needed for
LSM hook argument types) and make the files easier to read, understand
and maintain.

>>
>>>
>>> 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.

Right, but now, with the new lsm_append(), module names are concatenated
("%s,%s") in the lsm_names variable. It would be nice to not pollute
this string with multiple time the same module name.

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem Kees Cook <keescook@chromium.org> - 2017-04-19 00:20 +0200
  Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to  filesystem Mickaël Salaün <mic@digikod.net> - 2017-04-19 00:50 +0200
    Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to  filesystem Casey Schaufler <casey@schaufler-ca.com> - 2017-04-19 01:20 +0200
      Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem Kees Cook <keescook@chromium.org> - 2017-04-19 01:50 +0200
        Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to  filesystem Mickaël Salaün <mic@digikod.net> - 2017-04-20 00:10 +0200
          Re: [kernel-hardening] Re: [PATCH net-next v6 04/11] landlock: Add  LSM hooks related to filesystem Casey Schaufler <casey@schaufler-ca.com> - 2017-04-20 02:00 +0200
          Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem Kees Cook <keescook@chromium.org> - 2017-04-20 03:50 +0200
    Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem Kees Cook <keescook@chromium.org> - 2017-04-19 01:40 +0200

csiph-web