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 <dani...@mellanox.com> wrote:
>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>
>>> Implement and attach hooks to allocate and free Infiniband QP and MAD
>>> agent security structures.
>>>
>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>> Reviewed-by: Eli Cohen <e...@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 <p...@paul-moore.com>
>>> * Copyright (C) 2007 Hitachi Software Engineering Co., Ltd.
>>> * Yuichi Nakamura <yna...@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
>
_______________________________________________
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.
