netdev - Re: [PATCH net-next] nfnetlink_queue: enable PID info retrieval

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] [day] [month] [year] [list]

Message-ID: <20160615184716.GM24102@mtj.duckdns.org>
Date:	Wed, 15 Jun 2016 14:47:16 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Daniel Wagner <daniel.wagner@...-carit.de>
Cc:	Daniel Borkmann <daniel@...earbox.net>,
	Florian Westphal <fw@...len.de>,
	Saeed Mahameed <saeedm@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	netfilter-devel@...r.kernel.org,
	Yevgeny Petrilin <yevgenyp@...lanox.com>,
	Andre Melkoumian <andre@...lanox.com>,
	Matthew Finlay <matt@...lanox.com>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	Patrick McHardy <kaber@...sh.net>,
	Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
	john.fastabend@...il.com, hannes@...essinduktion.org,
	viro@...iv.linux.org.uk
Subject: Re: [PATCH net-next] nfnetlink_queue: enable PID info retrieval

Hello,

On Fri, Jun 10, 2016 at 08:40:34AM +0200, Daniel Wagner wrote:
> > [ Cc'ing John, Daniel, et al ]
> > 
> > Btw, while I just looked at scm_detach_fds(), I think commits ...
> > 
> >  * 48a87cc26c13 ("net: netprio: fd passed in SCM_RIGHTS datagram not set
> > correctly")
> >  * d84295067fc7 ("net: net_cls: fd passed in SCM_RIGHTS datagram not set
> > correctly")
> > 
> > ... might not be correct, maybe I'm missing something ...? Lets say
> > process A
> > has a socket fd that it sends via SCM_RIGHTS to process B. Process A was
> > the
> > one that called sk_alloc() originally. Now in scm_detach_fds() we
> > install a new
> > fd for process B pointing to the same sock (file's private_data) and
> > above commits
> > update the cached socket cgroup data for net_cls/net_prio to the new
> > process B.
> > So, if process A for example still sends data over that socket, skbs
> > will then
> > wrongly match on B's cgroup membership instead of A's, no?
> 
> I can't remember the details right now (need to read up again but I wont
> have time till Wednesday).
> 
> From your analysis I would say that is not the desired effect. A should
> match against its own cgroup and not the one of B.

We don't have a good answer for resources which are shared across
different cgroups.  It is often too expensive to track such sharing
accurately and crude approximation (creator-owned, last-used or
whatever) is used widely even outside cgroup.  Different cgroup
controllers tried different approaches but most are settling down for
creator ownership with exceptions for high impact cases.

I don't think there's a solution which satifies all cases here.  Given
that, doing the minimum amount of work (not worrying about SCM_RIGHTS
transfers) is the right thing to do, but we've had this re-labeling
since 2012, so leaving as-is is likely the best option at this point.

Thanks.

-- 
tejun