On 7/22/2016 11:21 AM, Paul Moore wrote:
> On Thu, Jul 14, 2016 at 6:56 PM, Dan Jurgens <danielj@xxxxxxxxxxxx> wrote:
>> v2:
>> - new patch that has the generic notification, replaces selinux and
>> IB/core patches related to the ib_flush callback. Yuval Shaia and Paul
>> Moore
>> ---
>> drivers/infiniband/core/device.c | 46 ++++++++++++++++++++++++++++++
>> include/linux/security.h | 10 ++++++
>> security/security.c | 58 ++++++++++++++++++++++++++++++++++++++
>> security/selinux/hooks.c | 5 ++-
>> security/selinux/selinuxfs.c | 2 +
>> 5 files changed, 119 insertions(+), 2 deletions(-)
>>
> Thanks for making this more generic. I've got some comments below,
> but in the course of reviewing this I realized that the kernel has an
> existing, general purpose notifier mechanism (see
> include/linux/notifier.h); while I've seen the notifier code in the
> network stack, I never realized it was created as a general purpose
> mechanism. My apologies, I should have noticed this earlier and
> mentioned it.
>
> As far as how that impacts this patch; it seems like we should be good
> citizens and use the general mechanism, I don't see any obvious
> reasons why it wouldn't work. That said, I do feel bad for not
> realizing this earlier. If it isn't too annoying, would you mind
> updating this patch *again*?
Sure, I'll can do that for v3.
>> +
>> +static LIST_HEAD(lsm_notifier_list);
>> +static DEFINE_SPINLOCK(lsm_notifier_lock);
>> +static u32 next_callback_id;
> I'd rather see the callback ID named something that makes it obvious
> it is tied to the lsm_notifier_* bits, perhaps lsm_notifier_nextid?
I think this will go away with the switch to the generic notification, but if it stays I'll make the change.
>> /* Boot-time LSM user choice */
>> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>> CONFIG_DEFAULT_SECURITY;
>> @@ -98,6 +110,52 @@ int __init security_module_enable(const char *module)
>> return !strcmp(module, chosen_lsm);
>> }
>>
>> +void security_lsm_notify(enum lsm_event event, void *data)
>> +{
>> + struct lsm_notifier_entry *entry;
>> +
>> + rcu_read_lock();
>> + list_for_each_entry_rcu(entry, &lsm_notifier_list, list)
>> + entry->func(event, entry->ctx, data);
>> + rcu_read_unlock();
>> +}
>> +EXPORT_SYMBOL(security_lsm_notify);
>> +
>> +int security_register_lsm_notifier(lsm_notifier func, void *ctx, u32 *id)
>> +{
>> + struct lsm_notifier_entry *entry;
>> +
>> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> + if (!entry)
>> + return -ENOMEM;
>> +
>> + entry->func = func;
>> + entry->ctx = ctx;
> Do we need ctx? We do need to ability to pass a blob to the callback
> which will vary based on the event, but I'm not sure the ctx here has
> much value.
You'll see how I used ctx in "[PATCH v2 4/9] IB/core: Enforce security on management datagrams". Many mad agents can be created and each will register for a callback. I used the context to store a pointer to the specific mad agent.
>> + spin_lock(&lsm_notifier_lock);
>> + entry->callback_id = next_callback_id++;
>> + *id = entry->callback_id;
>> + list_add_rcu(&entry->list, &lsm_notifier_list);
>> + spin_unlock(&lsm_notifier_lock);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(security_register_lsm_notifier);
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
