linux-kernel - Re: [BUG] Bug in netprio_cgroup and netcls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50FFAC4F.6010602@monom.org>
Date:	Wed, 23 Jan 2013 10:24:31 +0100
From:	Daniel Wagner <wagi@...om.org>
To:	John Fastabend <john.fastabend@...il.com>
CC:	Li Zefan <lizefan@...wei.com>,
	John Fastabend <john.r.fastabend@...el.com>,
	Neil Horman <nhorman@...driver.com>,
	Daniel Wagner <daniel.wagner@...-carit.de>,
	LKML <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
	Cgroups <cgroups@...r.kernel.org>
Subject: Re: [BUG] Bug in netprio_cgroup and netcls_cgroup ?

On 23.01.2013 01:02, John Fastabend wrote:
> [...]
>
>>>
>>> OK, I guess we should do something similar in the netprio, netcls
>>> cgroups and
>>> yes document it as you noted in your last comment.
>>
>> Here is my attempt to add such a check. I really don't know if this is
>> the
>> correct way to do so. To test this I have written a test program, which
>> seems to test the right thing. Please have a look and let me know if
>> it is correct: http://www.monom.org/misc/scm_rights.c
>>
>> And here a dirty first version of the patch:
>>
>>
>>  From 49a78d907eaf31c16673025e7e3b4844e419e416 Mon Sep 17 00:00:00 2001
>> From: Daniel Wagner <daniel.wagner@...-carit.de>
>> Date: Tue, 22 Jan 2013 11:08:22 +0100
>> Subject: [PATCH] net: net_prio: Block attach if a socket is shared
>>
>> ---
>>   net/core/netprio_cgroup.c | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
>> index 847c02b..de4e6c5 100644
>> --- a/net/core/netprio_cgroup.c
>> +++ b/net/core/netprio_cgroup.c
>> @@ -274,9 +274,39 @@ static struct cftype ss_files[] = {
>>       { }    /* terminate */
>>   };
>>
>> +static int check_cnt(const void *v, struct file *file, unsigned n)
>> +{
>> +    unsigned *flag = (unsigned *)v;
>> +    int err;
>> +
>> +    struct socket *sock = sock_from_file(file, &err);
>> +    if (sock && file_count(file) > 1)
>> +        *flag = 1;
>> +
>
> I think this check will catch a lot of cases that are not necessarily
> sharing a socket across tasks though. For example iscsid passes a file
> descriptor to the kernel which does a sockfd_lookup() incrementing
> f_count. Similarly look at dup/clone/etc.

Yep, I expected that this patch was too simple.

> In many of these cases I believe it should be OK to move the task
> around when the sockets are not shared between multiple tasks.

Do you know of a different way to identify the shared sockets?

thanks,
daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/