Re: [PATCH v1] LSM: Enable multiple calls to security_add_hooks() for the same LSM

[MickaÃl SalaÃn]

On 08/05/2017 22:07, Casey Schaufler wrote:
> On 5/8/2017 12:24 PM, MickaÃl SalaÃn wrote:
>> On 01/05/2017 01:28, James Morris wrote:
>>> On Sat, 29 Apr 2017, MickaÃl SalaÃn wrote:
>>>
>>>> Check if the registering LSM already registered hooks just before. This
>>>> enable to split hook declarations into multiple files without
>>>> registering multiple time the same LSM name, starting from commit
>>>> d69dece5f5b6 ("LSM: Add /sys/kernel/security/lsm").
>>> Please include a detailed rationale for these patches.  The above tells us 
>>> very little about why they are needed.
>> Right, what do you think about that?
>>
>> The commit d69dece5f5b6 ("LSM: Add /sys/kernel/security/lsm") extend
>> security_add_hooks() with a new parameter to register the LSM name,
>> which may be useful to make the list of currently loaded LSM available
>> to userspace. However, there is no clean way for an LSM no split its
>> hook declarations into multiple files, which may reduce the mess with
>> all the included files (needed for LSM hook argument types) and make the
>> source code easier to review and maintain.
>>
>> This change allows an LSM to register multiple times its hook while
>> keeping a consistent list of LSM names as described in
>> Documentation/security/LSM.txt . The list reflects the order in which
>> checks are made. This patch only check for the last registered LSM,
>> which should be the only case. If an LSM register multiple times its
>> hooks, interleaved with other LSM registrations, which should not
>> happen, its name will still appear in the same order that the hooks are
>> called, hence multiple times.
>>
>>
>> Casey, Tetsuo, are you OK with this approach or do you want me to handle
>> the case with interleaved hook registration, i.e. no duplicate name nor
>> following the current Documentation/security/LSM.txt?
>> What about the API with the NULL name (which is much simpler)?
> 
> Initially I thought that the module name should
> never appear more than once, however I could see
> a module that would bracket another or another set,
> so
> 
> 	capability,spiffy,selinux
> 
> might be different behaviorally than
> 
> 	capability,spiffy,selinux,spiffy
> 
> and userspace might care. I still don't see any value
> in
> 
> 	capability,selinux,spiffy,spiffy
> 
> Passing a NULL name could lead to ambiguity if more
> than one module did that, so I can't say I approve.

OK, so this patch seems good.

Attachment: signature.asc
Description: OpenPGP digital signature