[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110630075716.GB3377@albatros>
Date: Thu, 30 Jun 2011 11:57:16 +0400
From: Vasiliy Kulikov <segoon@...nwall.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Shailabh Nagar <nagar@...ibm.com>, Balbir Singh <bsingharora@...il.com>,
linux-kernel@...r.kernel.org, security@...nel.org,
Eric Paris <eparis@...hat.com>, Stephen Wilson <wilsons@...rt.ca>,
KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
David Rientjes <rientjes@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Balbir Singh <balbir@...ux.vnet.ibm.com>,
kernel-hardening@...ts.openwall.com
Subject: Re: [Security] [PATCH 2/2] taskstats: restrict access to user
On Wed, Jun 29, 2011 at 13:09 -0700, Linus Torvalds wrote:
> Ok, having looked at this some more, I'm quite ready to just mark the
> whole TASKSTATS config option as BROKEN. It does seem to be a horrible
> security hazard, and very little seems to use it.
>
> It also seems to be really fundamentally broken. Afaik, there's no way
> to filter taskstats not only by security issues (Vasiliy's patch
> really is very ugly), but it seems to be some global cross-namespace
> thing too, so it exposes taskstats across pid namespaces afaik.
The problem here is that it keeps a pid in a global list. This list is
then browsed by all namespaces. Looking into the code, I see 2 real
problems (didn't check, though):
1) If there are 2+ pid namespaces, one listener in pid_ns #1 and some
process dies in pid_ns #2, then the dying task wouldn't find the
listener via find_task_by_vpid() and would just delete the listener from
listeners list. Looks like this problem was created by optimization
patch f9fd8914c1acca0.
2) Netlink sockets are per net namespace, but this accounting thing is per
pid namespace. So, if the dying task and the listener are in a single
pid namespace, but in different net namespaces, the message will not be
sent via genlmsg_unicast(). I suspect it is a problem of all non-net
netlink sockets.
> It
> does that even with Vasiliy's patch, afaik, although then I think you
> need to have collissions in the namespaces if I read the code
> correctly.
>
> I suspect that could be fixed by adding a pid namespace to the
> 'listener' structure. Also adding a 'cred' pointer (or the actual
> listener thread pointer) to it would make Vasiliy's patch more
> palatable, since then you wouldn't need to look up the credentials at
> send_cpu_listeners() time.
>
> Maybe I have mis-read the code. But it does all make me shudder. There
> doesn't even seem to be all that many _users_ of the thing, so the
> problems it has really makes me go "is that code worth it"? We
> probably should never have merged it in the first place.
> but it seems to be some global cross-namespace
> thing too, so it exposes taskstats across pid namespaces afaik.
Thanks,
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
