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.
signature.asc
Description: OpenPGP digital signature