| Message ID | 1466711578-64398-5-git-send-email-danielj@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jun 23, 2016 at 10:52:50PM +0300, Dan Jurgens wrote: > From: Daniel Jurgens <danielj@mellanox.com> > > Implement and attach hooks to allocate and free Infiniband QP and MAD > agent security structures. > > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > Reviewed-by: Eli Cohen <eli@mellanox.com> > --- > include/rdma/ib_mad.h | 1 + > include/rdma/ib_verbs.h | 1 + > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++++++++++++ > security/selinux/include/objsec.h | 5 ++++ > 4 files changed, 60 insertions(+) > > diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h > index c8a773f..a1ed025 100644 > --- a/include/rdma/ib_mad.h > +++ b/include/rdma/ib_mad.h > @@ -537,6 +537,7 @@ struct ib_mad_agent { > u32 flags; > u8 port_num; > u8 rmpp_version; > + void *m_security; > }; > > /** > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 3f6780b..e522acb 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1454,6 +1454,7 @@ struct ib_qp { > void *qp_context; > u32 qp_num; > enum ib_qp_type qp_type; > + struct ib_qp_security *qp_sec; > }; > > struct ib_mr { > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 6a8841d..4f13ea4 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -17,6 +17,7 @@ > * Paul Moore <paul@paul-moore.com> > * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. > * Yuichi Nakamura <ynakam@hitachisoft.jp> > + * Copyright (C) 2016 Mellanox Technologies > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2, > @@ -83,6 +84,8 @@ > #include <linux/export.h> > #include <linux/msg.h> > #include <linux/shm.h> > +#include <rdma/ib_verbs.h> > +#include <rdma/ib_mad.h> > > #include "avc.h" > #include "objsec.h" > @@ -6015,6 +6018,47 @@ static void selinux_unregister_ib_flush_callback(void) > mutex_unlock(&ib_flush_mutex); > } > > +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec) > +{ > + struct ib_security_struct *sec; > + > + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); Kindly reminder to make sure GFP_ATOMIC is needed. > + if (!sec) > + return -ENOMEM; > + sec->sid = current_sid(); > + > + qp_sec->q_security = sec; > + return 0; > +} > + > +static void selinux_ib_qp_free_security(struct ib_qp_security *qp_sec) > +{ > + struct ib_security_struct *sec = qp_sec->q_security; > + > + qp_sec->q_security = NULL; > + kfree(sec); > +} > + > +static int selinux_ib_mad_agent_alloc_security(struct ib_mad_agent *mad_agent) > +{ > + struct ib_security_struct *sec; > + > + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); > + if (!sec) > + return -ENOMEM; > + sec->sid = current_sid(); > + > + mad_agent->m_security = sec; > + return 0; > +} > + > +static void selinux_ib_mad_agent_free_security(struct ib_mad_agent *mad_agent) > +{ > + struct ib_security_struct *sec = mad_agent->m_security; > + > + mad_agent->m_security = NULL; > + kfree(sec); > +} > #endif > > static struct security_hook_list selinux_hooks[] = { > @@ -6198,11 +6242,20 @@ static struct security_hook_list selinux_hooks[] = { > LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue), > LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach), > LSM_HOOK_INIT(tun_dev_open, selinux_tun_dev_open), > + > #ifdef CONFIG_SECURITY_INFINIBAND > LSM_HOOK_INIT(register_ib_flush_callback, > selinux_register_ib_flush_callback), > LSM_HOOK_INIT(unregister_ib_flush_callback, > selinux_unregister_ib_flush_callback), > + LSM_HOOK_INIT(ib_qp_alloc_security, > + selinux_ib_qp_alloc_security), > + LSM_HOOK_INIT(ib_qp_free_security, > + selinux_ib_qp_free_security), > + LSM_HOOK_INIT(ib_mad_agent_alloc_security, > + selinux_ib_mad_agent_alloc_security), > + LSM_HOOK_INIT(ib_mad_agent_free_security, > + selinux_ib_mad_agent_free_security), > #endif > > #ifdef CONFIG_SECURITY_NETWORK_XFRM > diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h > index c21e135..8e7db43 100644 > --- a/security/selinux/include/objsec.h > +++ b/security/selinux/include/objsec.h > @@ -10,6 +10,7 @@ > * > * Copyright (C) 2001,2002 Networks Associates Technology, Inc. > * Copyright (C) 2003 Red Hat, Inc., James Morris <jmorris@redhat.com> > + * Copyright (C) 2016 Mellanox Technologies > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2, > @@ -128,6 +129,10 @@ struct key_security_struct { > u32 sid; /* SID of key */ > }; > > +struct ib_security_struct { > + u32 sid; /* SID of the queue pair or MAD agent */ > +}; > + > extern unsigned int selinux_checkreqprot; > > #endif /* _SELINUX_OBJSEC_H_ */ > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <danielj@mellanox.com> wrote: > From: Daniel Jurgens <danielj@mellanox.com> > > Implement and attach hooks to allocate and free Infiniband QP and MAD > agent security structures. > > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > Reviewed-by: Eli Cohen <eli@mellanox.com> > --- > include/rdma/ib_mad.h | 1 + > include/rdma/ib_verbs.h | 1 + > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++++++++++++ > security/selinux/include/objsec.h | 5 ++++ > 4 files changed, 60 insertions(+) > > diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h > index c8a773f..a1ed025 100644 > --- a/include/rdma/ib_mad.h > +++ b/include/rdma/ib_mad.h > @@ -537,6 +537,7 @@ struct ib_mad_agent { > u32 flags; > u8 port_num; > u8 rmpp_version; > + void *m_security; General convention is to just call the LSM blobs "security" unless there is already a field with that name. > }; > > /** > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 3f6780b..e522acb 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1454,6 +1454,7 @@ struct ib_qp { > void *qp_context; > u32 qp_num; > enum ib_qp_type qp_type; > + struct ib_qp_security *qp_sec; See my earlier question/comment about just using a void pointer here. > }; > > struct ib_mr { > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 6a8841d..4f13ea4 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -17,6 +17,7 @@ > * Paul Moore <paul@paul-moore.com> > * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. > * Yuichi Nakamura <ynakam@hitachisoft.jp> > + * Copyright (C) 2016 Mellanox Technologies > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2, > @@ -83,6 +84,8 @@ > #include <linux/export.h> > #include <linux/msg.h> > #include <linux/shm.h> > +#include <rdma/ib_verbs.h> > +#include <rdma/ib_mad.h> > > #include "avc.h" > #include "objsec.h" > @@ -6015,6 +6018,47 @@ static void selinux_unregister_ib_flush_callback(void) > mutex_unlock(&ib_flush_mutex); > } > > +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec) > +{ > + struct ib_security_struct *sec; > + > + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); > + if (!sec) > + return -ENOMEM; > + sec->sid = current_sid(); > + > + qp_sec->q_security = sec; > + return 0; > +} If you get rid of the ip_qp_security struct, you can just return the blob instead of an int (NULL on error). Same with the MAD allocator below. Also, and this may be more important for the MAD allocator below (I'm still pretty IB-ignorant), can you forsee the need/desire to have the QP/MAD label different from the process which creates them? How often will other SELinux domains need to interact with these objects? > +static void selinux_ib_qp_free_security(struct ib_qp_security *qp_sec) > +{ > + struct ib_security_struct *sec = qp_sec->q_security; > + > + qp_sec->q_security = NULL; > + kfree(sec); > +} > + > +static int selinux_ib_mad_agent_alloc_security(struct ib_mad_agent *mad_agent) > +{ > + struct ib_security_struct *sec; > + > + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); > + if (!sec) > + return -ENOMEM; > + sec->sid = current_sid(); > + > + mad_agent->m_security = sec; > + return 0; > +} > + > +static void selinux_ib_mad_agent_free_security(struct ib_mad_agent *mad_agent) > +{ > + struct ib_security_struct *sec = mad_agent->m_security; > + > + mad_agent->m_security = NULL; > + kfree(sec); > +} > #endif
On 6/30/2016 1:42 PM, Paul Moore wrote: > On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <danielj@mellanox.com> wrote: >> From: Daniel Jurgens <danielj@mellanox.com> >> >> Implement and attach hooks to allocate and free Infiniband QP and MAD >> agent security structures. >> >> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >> Reviewed-by: Eli Cohen <eli@mellanox.com> >> --- >> include/rdma/ib_mad.h | 1 + >> include/rdma/ib_verbs.h | 1 + >> security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++++++++++++ >> security/selinux/include/objsec.h | 5 ++++ >> 4 files changed, 60 insertions(+) >> >> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h >> index c8a773f..a1ed025 100644 >> --- a/include/rdma/ib_mad.h >> +++ b/include/rdma/ib_mad.h >> @@ -537,6 +537,7 @@ struct ib_mad_agent { >> u32 flags; >> u8 port_num; >> u8 rmpp_version; >> + void *m_security; > General convention is to just call the LSM blobs "security" unless > there is already a field with that name. Not that it really matters all that much, but an unadorned "security" makes it unnecessarily difficult to match "p->security" to the data involved when you're looking at keys, creds and ipc. I like having the prefix. I think the other fields in the structure should have it, too, but as I'm not an acknowledged authority on good style I hesitate to suggest it in general. > >> }; >> >> /** >> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >> index 3f6780b..e522acb 100644 >> --- a/include/rdma/ib_verbs.h >> +++ b/include/rdma/ib_verbs.h >> @@ -1454,6 +1454,7 @@ struct ib_qp { >> void *qp_context; >> u32 qp_num; >> enum ib_qp_type qp_type; >> + struct ib_qp_security *qp_sec; > See my earlier question/comment about just using a void pointer here. I think that this is in response to my comments to the effect that I would like to see the LSM infrastructure using the inode like (inode->i_security) to the xfrm (void *) approach. I haven't been looking at the IB patches too carefully to date. It's possible I have not been clear. > >> }; >> >> struct ib_mr { >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 6a8841d..4f13ea4 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -17,6 +17,7 @@ >> * Paul Moore <paul@paul-moore.com> >> * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. >> * Yuichi Nakamura <ynakam@hitachisoft.jp> >> + * Copyright (C) 2016 Mellanox Technologies >> * >> * This program is free software; you can redistribute it and/or modify >> * it under the terms of the GNU General Public License version 2, >> @@ -83,6 +84,8 @@ >> #include <linux/export.h> >> #include <linux/msg.h> >> #include <linux/shm.h> >> +#include <rdma/ib_verbs.h> >> +#include <rdma/ib_mad.h> >> >> #include "avc.h" >> #include "objsec.h" >> @@ -6015,6 +6018,47 @@ static void selinux_unregister_ib_flush_callback(void) >> mutex_unlock(&ib_flush_mutex); >> } >> >> +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec) >> +{ >> + struct ib_security_struct *sec; >> + >> + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); >> + if (!sec) >> + return -ENOMEM; >> + sec->sid = current_sid(); >> + >> + qp_sec->q_security = sec; >> + return 0; >> +} > If you get rid of the ip_qp_security struct, you can just return the > blob instead of an int (NULL on error). Same with the MAD allocator > below. > > Also, and this may be more important for the MAD allocator below (I'm > still pretty IB-ignorant), can you forsee the need/desire to have the > QP/MAD label different from the process which creates them? How often > will other SELinux domains need to interact with these objects? > >> +static void selinux_ib_qp_free_security(struct ib_qp_security *qp_sec) >> +{ >> + struct ib_security_struct *sec = qp_sec->q_security; >> + >> + qp_sec->q_security = NULL; >> + kfree(sec); >> +} >> + >> +static int selinux_ib_mad_agent_alloc_security(struct ib_mad_agent *mad_agent) >> +{ >> + struct ib_security_struct *sec; >> + >> + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); >> + if (!sec) >> + return -ENOMEM; >> + sec->sid = current_sid(); >> + >> + mad_agent->m_security = sec; >> + return 0; >> +} >> + >> +static void selinux_ib_mad_agent_free_security(struct ib_mad_agent *mad_agent) >> +{ >> + struct ib_security_struct *sec = mad_agent->m_security; >> + >> + mad_agent->m_security = NULL; >> + kfree(sec); >> +} >> #endif -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/30/2016 4:06 PM, Casey Schaufler wrote: > On 6/30/2016 1:42 PM, Paul Moore wrote: >> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <danielj@mellanox.com> wrote: >>> From: Daniel Jurgens <danielj@mellanox.com> >>> >>> Implement and attach hooks to allocate and free Infiniband QP and MAD >>> agent security structures. >>> >>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >>> Reviewed-by: Eli Cohen <eli@mellanox.com> >>> --- >>> include/rdma/ib_mad.h | 1 + >>> include/rdma/ib_verbs.h | 1 + >>> security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++++++++++++ >>> security/selinux/include/objsec.h | 5 ++++ >>> 4 files changed, 60 insertions(+) >>> >>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h >>> index c8a773f..a1ed025 100644 >>> --- a/include/rdma/ib_mad.h >>> +++ b/include/rdma/ib_mad.h >>> @@ -537,6 +537,7 @@ struct ib_mad_agent { >>> u32 flags; >>> u8 port_num; >>> u8 rmpp_version; >>> + void *m_security; >> General convention is to just call the LSM blobs "security" unless >> there is already a field with that name. > Not that it really matters all that much, but an unadorned "security" > makes it unnecessarily difficult to match "p->security" to the data > involved when you're looking at keys, creds and ipc. I like having > the prefix. I think the other fields in the structure should have it, > too, but as I'm not an acknowledged authority on good style I hesitate > to suggest it in general. Now that you mention it I think this was part of your comment about not using void*. >>> }; >>> >>> /** >>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>> index 3f6780b..e522acb 100644 >>> --- a/include/rdma/ib_verbs.h >>> +++ b/include/rdma/ib_verbs.h >>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>> void *qp_context; >>> u32 qp_num; >>> enum ib_qp_type qp_type; >>> + struct ib_qp_security *qp_sec; >> See my earlier question/comment about just using a void pointer here. > I think that this is in response to my comments to the > effect that I would like to see the LSM infrastructure > using the inode like (inode->i_security) to the xfrm > (void *) approach. I haven't been looking at the IB patches > too carefully to date. It's possible I have not been clear. My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >>> }; >>> >>> struct ib_mr { >>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >>> index 6a8841d..4f13ea4 100644 >>> --- a/security/selinux/hooks.c >>> +++ b/security/selinux/hooks.c >>> @@ -17,6 +17,7 @@ >>> * Paul Moore <paul@paul-moore.com> >>> * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. >>> * Yuichi Nakamura <ynakam@hitachisoft.jp> >>> + * Copyright (C) 2016 Mellanox Technologies >>> * >>> * This program is free software; you can redistribute it and/or modify >>> * it under the terms of the GNU General Public License version 2, >>> @@ -83,6 +84,8 @@ >>> #include <linux/export.h> >>> #include <linux/msg.h> >>> #include <linux/shm.h> >>> +#include <rdma/ib_verbs.h> >>> +#include <rdma/ib_mad.h> >>> >>> #include "avc.h" >>> #include "objsec.h" >>> @@ -6015,6 +6018,47 @@ static void selinux_unregister_ib_flush_callback(void) >>> mutex_unlock(&ib_flush_mutex); >>> } >>> >>> +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec) >>> +{ >>> + struct ib_security_struct *sec; >>> + >>> + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); >>> + if (!sec) >>> + return -ENOMEM; >>> + sec->sid = current_sid(); >>> + >>> + qp_sec->q_security = sec; >>> + return 0; >>> +} >> If you get rid of the ip_qp_security struct, you can just return the >> blob instead of an int (NULL on error). Same with the MAD allocator >> below. >> >> Also, and this may be more important for the MAD allocator below (I'm >> still pretty IB-ignorant), can you forsee the need/desire to have the >> QP/MAD label different from the process which creates them? How often >> will other SELinux domains need to interact with these objects? >> >>> +static void selinux_ib_qp_free_security(struct ib_qp_security *qp_sec) >>> +{ >>> + struct ib_security_struct *sec = qp_sec->q_security; >>> + >>> + qp_sec->q_security = NULL; >>> + kfree(sec); >>> +} >>> + >>> +static int selinux_ib_mad_agent_alloc_security(struct ib_mad_agent *mad_agent) >>> +{ >>> + struct ib_security_struct *sec; >>> + >>> + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); >>> + if (!sec) >>> + return -ENOMEM; >>> + sec->sid = current_sid(); >>> + >>> + mad_agent->m_security = sec; >>> + return 0; >>> +} >>> + >>> +static void selinux_ib_mad_agent_free_security(struct ib_mad_agent *mad_agent) >>> +{ >>> + struct ib_security_struct *sec = mad_agent->m_security; >>> + >>> + mad_agent->m_security = NULL; >>> + kfree(sec); >>> +} >>> #endif > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: > On 6/30/2016 4:06 PM, Casey Schaufler wrote: >> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>> }; >>>> >>>> /** >>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>> index 3f6780b..e522acb 100644 >>>> --- a/include/rdma/ib_verbs.h >>>> +++ b/include/rdma/ib_verbs.h >>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>> void *qp_context; >>>> u32 qp_num; >>>> enum ib_qp_type qp_type; >>>> + struct ib_qp_security *qp_sec; >>> See my earlier question/comment about just using a void pointer here. >> >> I think that this is in response to my comments to the >> effect that I would like to see the LSM infrastructure >> using the inode like (inode->i_security) to the xfrm >> (void *) approach. I haven't been looking at the IB patches >> too carefully to date. It's possible I have not been clear. > > My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. All of what you describe above can still happen with a void pointer; in some ways it is even easier with a void pointer.
On 7/1/2016 1:54 PM, Paul Moore wrote: > On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>> }; >>>>> >>>>> /** >>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>> index 3f6780b..e522acb 100644 >>>>> --- a/include/rdma/ib_verbs.h >>>>> +++ b/include/rdma/ib_verbs.h >>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>> void *qp_context; >>>>> u32 qp_num; >>>>> enum ib_qp_type qp_type; >>>>> + struct ib_qp_security *qp_sec; >>>> See my earlier question/comment about just using a void pointer here. >>> I think that this is in response to my comments to the >>> effect that I would like to see the LSM infrastructure >>> using the inode like (inode->i_security) to the xfrm >>> (void *) approach. I haven't been looking at the IB patches >>> too carefully to date. It's possible I have not been clear. >> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. > All of what you describe above can still happen with a void pointer; > in some ways it is even easier with a void pointer. If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <danielj@mellanox.com> wrote: > On 7/1/2016 1:54 PM, Paul Moore wrote: >> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>>> }; >>>>>> >>>>>> /** >>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>>> index 3f6780b..e522acb 100644 >>>>>> --- a/include/rdma/ib_verbs.h >>>>>> +++ b/include/rdma/ib_verbs.h >>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>>> void *qp_context; >>>>>> u32 qp_num; >>>>>> enum ib_qp_type qp_type; >>>>>> + struct ib_qp_security *qp_sec; >>>>> See my earlier question/comment about just using a void pointer here. >>>> I think that this is in response to my comments to the >>>> effect that I would like to see the LSM infrastructure >>>> using the inode like (inode->i_security) to the xfrm >>>> (void *) approach. I haven't been looking at the IB patches >>>> too carefully to date. It's possible I have not been clear. >>> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >> All of what you describe above can still happen with a void pointer; >> in some ways it is even easier with a void pointer. > > If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? You worry about that in the LSM framework and hide the details behind the void pointer. For example, you create an array/list of LSM specific blobs and just stash a pointer to the head of the data in the void pointer.
On 7/1/2016 12:17 PM, Paul Moore wrote: > On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >> On 7/1/2016 1:54 PM, Paul Moore wrote: >>> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>>>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>>>> }; >>>>>>> >>>>>>> /** >>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>>>> index 3f6780b..e522acb 100644 >>>>>>> --- a/include/rdma/ib_verbs.h >>>>>>> +++ b/include/rdma/ib_verbs.h >>>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>>>> void *qp_context; >>>>>>> u32 qp_num; >>>>>>> enum ib_qp_type qp_type; >>>>>>> + struct ib_qp_security *qp_sec; >>>>>> See my earlier question/comment about just using a void pointer here. >>>>> I think that this is in response to my comments to the >>>>> effect that I would like to see the LSM infrastructure >>>>> using the inode like (inode->i_security) to the xfrm >>>>> (void *) approach. I haven't been looking at the IB patches >>>>> too carefully to date. It's possible I have not been clear. >>>> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >>> All of what you describe above can still happen with a void pointer; >>> in some ways it is even easier with a void pointer. >> If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? > You worry about that in the LSM framework and hide the details behind > the void pointer. For example, you create an array/list of LSM > specific blobs and just stash a pointer to the head of the data in the > void pointer. Don't worry about it at this point. Patches pending. If I have to change modules to accommodate the infrastructure I'm not afraid to do so. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/1/2016 3:13 PM, Casey Schaufler wrote: > On 7/1/2016 12:17 PM, Paul Moore wrote: >> On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>> On 7/1/2016 1:54 PM, Paul Moore wrote: >>>> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>>> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>>>>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>>>>> }; >>>>>>>> >>>>>>>> /** >>>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>>>>> index 3f6780b..e522acb 100644 >>>>>>>> --- a/include/rdma/ib_verbs.h >>>>>>>> +++ b/include/rdma/ib_verbs.h >>>>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>>>>> void *qp_context; >>>>>>>> u32 qp_num; >>>>>>>> enum ib_qp_type qp_type; >>>>>>>> + struct ib_qp_security *qp_sec; >>>>>>> See my earlier question/comment about just using a void pointer here. >>>>>> I think that this is in response to my comments to the >>>>>> effect that I would like to see the LSM infrastructure >>>>>> using the inode like (inode->i_security) to the xfrm >>>>>> (void *) approach. I haven't been looking at the IB patches >>>>>> too carefully to date. It's possible I have not been clear. >>>>> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >>>> All of what you describe above can still happen with a void pointer; >>>> in some ways it is even easier with a void pointer. >>> If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? >> You worry about that in the LSM framework and hide the details behind >> the void pointer. For example, you create an array/list of LSM >> specific blobs and just stash a pointer to the head of the data in the >> void pointer. > Don't worry about it at this point. Patches pending. > If I have to change modules to accommodate the > infrastructure I'm not afraid to do so. So I should revert to void* blobs? I just want to be clear before making the change. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/1/2016 1:46 PM, Daniel Jurgens wrote: > On 7/1/2016 3:13 PM, Casey Schaufler wrote: >> On 7/1/2016 12:17 PM, Paul Moore wrote: >>> On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>> On 7/1/2016 1:54 PM, Paul Moore wrote: >>>>> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>>>> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>>>>>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>>>>>> }; >>>>>>>>> >>>>>>>>> /** >>>>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>>>>>> index 3f6780b..e522acb 100644 >>>>>>>>> --- a/include/rdma/ib_verbs.h >>>>>>>>> +++ b/include/rdma/ib_verbs.h >>>>>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>>>>>> void *qp_context; >>>>>>>>> u32 qp_num; >>>>>>>>> enum ib_qp_type qp_type; >>>>>>>>> + struct ib_qp_security *qp_sec; >>>>>>>> See my earlier question/comment about just using a void pointer here. >>>>>>> I think that this is in response to my comments to the >>>>>>> effect that I would like to see the LSM infrastructure >>>>>>> using the inode like (inode->i_security) to the xfrm >>>>>>> (void *) approach. I haven't been looking at the IB patches >>>>>>> too carefully to date. It's possible I have not been clear. >>>>>> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >>>>> All of what you describe above can still happen with a void pointer; >>>>> in some ways it is even easier with a void pointer. >>>> If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? >>> You worry about that in the LSM framework and hide the details behind >>> the void pointer. For example, you create an array/list of LSM >>> specific blobs and just stash a pointer to the head of the data in the >>> void pointer. >> Don't worry about it at this point. Patches pending. >> If I have to change modules to accommodate the >> infrastructure I'm not afraid to do so. > So I should revert to void* blobs? I just want to be clear before making the change. > Whichever you're really comfortable with. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 1, 2016 at 4:46 PM, Daniel Jurgens <danielj@mellanox.com> wrote: > On 7/1/2016 3:13 PM, Casey Schaufler wrote: >> On 7/1/2016 12:17 PM, Paul Moore wrote: >>> On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>> On 7/1/2016 1:54 PM, Paul Moore wrote: >>>>> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <danielj@mellanox.com> wrote: >>>>>> On 6/30/2016 4:06 PM, Casey Schaufler wrote: >>>>>>> On 6/30/2016 1:42 PM, Paul Moore wrote: >>>>>>>>> }; >>>>>>>>> >>>>>>>>> /** >>>>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>>>>>>> index 3f6780b..e522acb 100644 >>>>>>>>> --- a/include/rdma/ib_verbs.h >>>>>>>>> +++ b/include/rdma/ib_verbs.h >>>>>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp { >>>>>>>>> void *qp_context; >>>>>>>>> u32 qp_num; >>>>>>>>> enum ib_qp_type qp_type; >>>>>>>>> + struct ib_qp_security *qp_sec; >>>>>>>> See my earlier question/comment about just using a void pointer here. >>>>>>> I think that this is in response to my comments to the >>>>>>> effect that I would like to see the LSM infrastructure >>>>>>> using the inode like (inode->i_security) to the xfrm >>>>>>> (void *) approach. I haven't been looking at the IB patches >>>>>>> too carefully to date. It's possible I have not been clear. >>>>>> My understanding at the time was that by using something other than a void * different security modules could maintain their own opaque blobs with in and keep the same prototype for the hook. It's possible I misunderstood you, but it made sense to me. I don't know of any plans for other security modules to support Infiniband, but this leaves the door open. >>>>> All of what you describe above can still happen with a void pointer; >>>>> in some ways it is even easier with a void pointer. >>>> If multiple security modules register an alloc_security hook for example, how would you coordinate between them to allocate the memory? >>> You worry about that in the LSM framework and hide the details behind >>> the void pointer. For example, you create an array/list of LSM >>> specific blobs and just stash a pointer to the head of the data in the >>> void pointer. >> Don't worry about it at this point. Patches pending. >> If I have to change modules to accommodate the >> infrastructure I'm not afraid to do so. > > So I should revert to void* blobs? I just want to be clear before making the change. Yes.
diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h index c8a773f..a1ed025 100644 --- a/include/rdma/ib_mad.h +++ b/include/rdma/ib_mad.h @@ -537,6 +537,7 @@ struct ib_mad_agent { u32 flags; u8 port_num; u8 rmpp_version; + void *m_security; }; /** diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 3f6780b..e522acb 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1454,6 +1454,7 @@ struct ib_qp { void *qp_context; u32 qp_num; enum ib_qp_type qp_type; + struct ib_qp_security *qp_sec; }; struct ib_mr { diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6a8841d..4f13ea4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -17,6 +17,7 @@ * Paul Moore <paul@paul-moore.com> * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd. * Yuichi Nakamura <ynakam@hitachisoft.jp> + * Copyright (C) 2016 Mellanox Technologies * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2, @@ -83,6 +84,8 @@ #include <linux/export.h> #include <linux/msg.h> #include <linux/shm.h> +#include <rdma/ib_verbs.h> +#include <rdma/ib_mad.h> #include "avc.h" #include "objsec.h" @@ -6015,6 +6018,47 @@ static void selinux_unregister_ib_flush_callback(void) mutex_unlock(&ib_flush_mutex); } +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec) +{ + struct ib_security_struct *sec; + + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); + if (!sec) + return -ENOMEM; + sec->sid = current_sid(); + + qp_sec->q_security = sec; + return 0; +} + +static void selinux_ib_qp_free_security(struct ib_qp_security *qp_sec) +{ + struct ib_security_struct *sec = qp_sec->q_security; + + qp_sec->q_security = NULL; + kfree(sec); +} + +static int selinux_ib_mad_agent_alloc_security(struct ib_mad_agent *mad_agent) +{ + struct ib_security_struct *sec; + + sec = kzalloc(sizeof(*sec), GFP_ATOMIC); + if (!sec) + return -ENOMEM; + sec->sid = current_sid(); + + mad_agent->m_security = sec; + return 0; +} + +static void selinux_ib_mad_agent_free_security(struct ib_mad_agent *mad_agent) +{ + struct ib_security_struct *sec = mad_agent->m_security; + + mad_agent->m_security = NULL; + kfree(sec); +} #endif static struct security_hook_list selinux_hooks[] = { @@ -6198,11 +6242,20 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue), LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach), LSM_HOOK_INIT(tun_dev_open, selinux_tun_dev_open), + #ifdef CONFIG_SECURITY_INFINIBAND LSM_HOOK_INIT(register_ib_flush_callback, selinux_register_ib_flush_callback), LSM_HOOK_INIT(unregister_ib_flush_callback, selinux_unregister_ib_flush_callback), + LSM_HOOK_INIT(ib_qp_alloc_security, + selinux_ib_qp_alloc_security), + LSM_HOOK_INIT(ib_qp_free_security, + selinux_ib_qp_free_security), + LSM_HOOK_INIT(ib_mad_agent_alloc_security, + selinux_ib_mad_agent_alloc_security), + LSM_HOOK_INIT(ib_mad_agent_free_security, + selinux_ib_mad_agent_free_security), #endif #ifdef CONFIG_SECURITY_NETWORK_XFRM diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index c21e135..8e7db43 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -10,6 +10,7 @@ * * Copyright (C) 2001,2002 Networks Associates Technology, Inc. * Copyright (C) 2003 Red Hat, Inc., James Morris <jmorris@redhat.com> + * Copyright (C) 2016 Mellanox Technologies * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2, @@ -128,6 +129,10 @@ struct key_security_struct { u32 sid; /* SID of key */ }; +struct ib_security_struct { + u32 sid; /* SID of the queue pair or MAD agent */ +}; + extern unsigned int selinux_checkreqprot; #endif /* _SELINUX_OBJSEC_H_ */