[04/12] selinux: Allocate and free infiniband security hooks
diff mbox

Message ID 1466711578-64398-5-git-send-email-danielj@mellanox.com
State New
Headers show

Commit Message

Daniel Jurgens June 23, 2016, 7:52 p.m. UTC
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(+)

Comments

Yuval Shaia June 30, 2016, 3:15 p.m. UTC | #1
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore June 30, 2016, 8:42 p.m. UTC | #2
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
Casey Schaufler June 30, 2016, 9:06 p.m. UTC | #3
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Jurgens June 30, 2016, 9:48 p.m. UTC | #4
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore July 1, 2016, 6:54 p.m. UTC | #5
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.
Daniel Jurgens July 1, 2016, 6:59 p.m. UTC | #6
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore July 1, 2016, 7:17 p.m. UTC | #7
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.
Casey Schaufler July 1, 2016, 8:13 p.m. UTC | #8
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Jurgens July 1, 2016, 8:46 p.m. UTC | #9
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Casey Schaufler July 1, 2016, 9:16 p.m. UTC | #10
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-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore July 1, 2016, 10:15 p.m. UTC | #11
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.

Patch
diff mbox

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_ */