Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code

[Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>]

On 07/08/2015 09:37 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> Originates from:
>>
>> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  ipc/kdbus/bus.c        | 12 ++++++++++-
>>  ipc/kdbus/bus.h        |  3 +++
>>  ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ipc/kdbus/connection.h |  4 ++++
>>  ipc/kdbus/domain.c     |  9 ++++++++-
>>  ipc/kdbus/domain.h     |  2 ++
>>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>>  ipc/kdbus/names.c      | 11 ++++++++++
>>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>>  9 files changed, 124 insertions(+), 12 deletions(-)
>>
>>
> 
>> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
>> index 9993753..b85cdc7 100644
>> --- a/ipc/kdbus/connection.c
>> +++ b/ipc/kdbus/connection.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/syscalls.h>
>>  #include <linux/uio.h>
>> +#include <linux/security.h>
>>  
>>  #include "bus.h"
>>  #include "connection.h"
>> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  	bool is_activator;
>>  	bool is_monitor;
>>  	struct kvec kvec;
>> +	u32 sid, len;
>> +	char *label;
>>  	int ret;
>>  
>>  	struct {
>> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  		}
>>  	}
>>  
>> +	security_task_getsecid(current, &sid);
>> +	security_secid_to_secctx(sid, &label, &len);
>> +	ret = security_kdbus_connect(conn, label, len);
>> +	if (ret) {
>> +		ret = -EPERM;
>> +		goto exit_unref;
>> +	}
> 
> This seems convoluted and expensive.  If you always want the label of
> the current task here, then why not just have security_kdbus_connect()
> internally extract the label of the current task?

Furthermore, why do we need a separate security field and copy of the
current label in the conn->security, when we already have
conn->cred->security available to us?

I don't think we need new security fields unless we are going to assign
some kind of object labeling to these structures separate from their
cred, and offhand I don't see why we would do that.