Re: [PATCH v2 3/9] selinux lsm IB/core: Implement LSM notification system
On Thu, Jul 14, 2016 at 6:56 PM, Dan Jurgenswrote: > From: Daniel Jurgens > > Add a generic notificaiton mechanism in the LSM. Interested consumers > can register a callback with the LSM and security modules can produce > events. > > Because access to Infiniband QPs are enforced in the setup phase of a > connection security should be enforced again if the policy changes. > Register infiniband devices for policy change notification and check all > QPs on that device when the notification is received. > > Add a call to the notification mechanism is from SELinux when the AVC > cache changes. > > Signed-off-by: Daniel Jurgens > > --- > 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*? > diff --git a/include/linux/security.h b/include/linux/security.h > index 33e23c4..bf53911 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -69,6 +69,16 @@ struct audit_krule; > struct user_namespace; > struct timezone; > > +enum lsm_event { > + LSM_POLICY_CHANGE, > +}; > + > +typedef void (*lsm_notifier)(enum lsm_event event, void *ctx, void *data); > + > +void security_lsm_notify(enum lsm_event event, void *data); > +int security_register_lsm_notifier(lsm_notifier func, void *ctx, u32 *id); > +void security_unregister_lsm_notifier(u32 id); > + > /* These functions are in security/commoncap.c */ > extern int cap_capable(const struct cred *cred, struct user_namespace *ns, >int cap, int audit); > diff --git a/security/security.c b/security/security.c > index 234982d..1263c1d 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -33,6 +33,18 @@ > /* Maximum number of letters for an LSM name string */ > #define SECURITY_NAME_MAX 10 > > +struct lsm_notifier_entry { > + u32 callback_id; > + lsm_notifierfunc; > + void*ctx; > + struct list_headlist; > + struct rcu_head rcu; > +}; > + > +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? > /* 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, _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. > + spin_lock(_notifier_lock); > + entry->callback_id = next_callback_id++; > + *id = entry->callback_id; > + list_add_rcu(>list, _notifier_list); > + spin_unlock(_notifier_lock); > + > + return 0; > +} > +EXPORT_SYMBOL(security_register_lsm_notifier); -- paul moore www.paul-moore.com ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To
Re: [PATCH v2 3/9] selinux lsm IB/core: Implement LSM notification system
Hi, [auto build test ERROR on next-20160713] [cannot apply to pcmoore-selinux/next rdma/master v4.7-rc7 v4.7-rc6 v4.7-rc5 v4.7-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Dan-Jurgens/SELinux-support-for-Infiniband-RDMA/20160715-122805 config: ia64-defconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): >> ERROR: "security_register_lsm_notifier" [drivers/infiniband/core/ib_core.ko] >> undefined! >> ERROR: "security_unregister_lsm_notifier" >> [drivers/infiniband/core/ib_core.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH v2 3/9] selinux lsm IB/core: Implement LSM notification system
From: Daniel JurgensAdd a generic notificaiton mechanism in the LSM. Interested consumers can register a callback with the LSM and security modules can produce events. Because access to Infiniband QPs are enforced in the setup phase of a connection security should be enforced again if the policy changes. Register infiniband devices for policy change notification and check all QPs on that device when the notification is received. Add a call to the notification mechanism is from SELinux when the AVC cache changes. Signed-off-by: Daniel Jurgens --- 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(-) diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 5b42e83..3219a7a 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -82,6 +83,10 @@ static LIST_HEAD(client_list); static DEFINE_MUTEX(device_mutex); static DECLARE_RWSEM(lists_rwsem); +static void ib_policy_change_task(struct work_struct *work); +static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task); + +static u32 lsm_callback_id; static int ib_device_check_mandatory(struct ib_device *device) { @@ -344,6 +349,37 @@ static int setup_port_pkey_list(struct ib_device *device) return 0; } +static void ib_policy_change_task(struct work_struct *work) +{ + struct ib_device *dev; + + down_read(_rwsem); + list_for_each_entry(dev, _list, core_list) { + int i; + + for (i = rdma_start_port(dev); i <= rdma_end_port(dev); i++) { + u64 sp; + int ret = ib_get_cached_subnet_prefix(dev, + i, + ); + + WARN_ONCE(ret, + "ib_get_cached_subnet_prefix err: %d, this should never happen here\n", + ret); + ib_security_cache_change(dev, i, sp); + } + } + up_read(_rwsem); +} + +static void ib_security_change(enum lsm_event event, void *ctx, void *lsm_data) +{ + if (event != LSM_POLICY_CHANGE) + return; + + schedule_work(_policy_change_work); +} + /** * ib_register_device - Register an IB device with IB core * @device:Device to register @@ -1075,10 +,19 @@ static int __init ib_core_init(void) goto err_sa; } + ret = security_register_lsm_notifier(_security_change, NULL, +_callback_id); + if (ret) { + pr_warn("Couldn't register LSM notifier. ret %d\n", ret); + goto err_ibnl_clients; + } + ib_cache_setup(); return 0; +err_ibnl_clients: + ib_remove_ibnl_clients(); err_sa: ib_sa_cleanup(); err_mad: @@ -1098,6 +1143,7 @@ err: static void __exit ib_core_cleanup(void) { + security_unregister_lsm_notifier(lsm_callback_id); ib_cache_cleanup(); ib_remove_ibnl_clients(); ib_sa_cleanup(); diff --git a/include/linux/security.h b/include/linux/security.h index 33e23c4..bf53911 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -69,6 +69,16 @@ struct audit_krule; struct user_namespace; struct timezone; +enum lsm_event { + LSM_POLICY_CHANGE, +}; + +typedef void (*lsm_notifier)(enum lsm_event event, void *ctx, void *data); + +void security_lsm_notify(enum lsm_event event, void *data); +int security_register_lsm_notifier(lsm_notifier func, void *ctx, u32 *id); +void security_unregister_lsm_notifier(u32 id); + /* These functions are in security/commoncap.c */ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, int cap, int audit); diff --git a/security/security.c b/security/security.c index 234982d..1263c1d 100644 --- a/security/security.c +++ b/security/security.c @@ -33,6 +33,18 @@ /* Maximum number of letters for an LSM name string */ #define SECURITY_NAME_MAX 10 +struct lsm_notifier_entry { + u32 callback_id; + lsm_notifierfunc; + void*ctx; + struct list_headlist; + struct rcu_head rcu; +}; + +static LIST_HEAD(lsm_notifier_list); +static DEFINE_SPINLOCK(lsm_notifier_lock); +static u32