linux-kernel - Re: [Security] [PATCH 2/2] taskstats: restrict access to user

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: <BANLkTi=DiVRV9dkTtYgq+CY-kxVs5ecoWw@mail.gmail.com>
Date:	Thu, 30 Jun 2011 09:40:52 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Balbir Singh <bsingharora@...il.com>
Cc:	Vasiliy Kulikov <segoon@...nwall.com>,
	Shailabh Nagar <nagar@...ibm.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 Thu, Jun 30, 2011 at 3:59 AM, Balbir Singh <bsingharora@...il.com> wrote:
> On Thu, Jun 30, 2011 at 1:27 PM, Vasiliy Kulikov <segoon@...nwall.com> wrote:
>> 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.
>>>
>
> I am not sure how you define BROKEN, BROKEN as per security rules?
> /proc/$pid/xxx also expose similar information.

No it doesn't.

/proc/$pid/xyz has always had proper security checks. Now, sometimes
they've been a bit too lax (iow, we've had cases where we just allowed
things we shouldn't - like this "io" file), but /proc/pid/ at least
*conceptually* has always had the "do I have permission to read this
or not". Do a "cat /proc/*/* > /dev/null" and you'll be getting a lot
of "permission denied" etc.

TASKSTATS? Nothing. Nada. Completely open.

That's just broken.

TASKSTATS also isn't apparently used by any normal program, and so
never got updated to namespaces. Again, /proc/xyz/ at least *tries*:
I'm not guaranteeing that it doesn't have some gaping holes, but I can
at least find the logic that saves and uses namespace information.

Again, TASKSTATS? Not so much.

> I must admit, I did not pay much attention to pid namespace changes as
> they were being made to taskstats. I'll look at the code later this
> week.

Some of the pid namespace issues could be fixed with the attached kind
of starting point.

But it's broken. See the comment I added. Why is it broken? Again -
taskstats is fundamentally broken, and doesn't seem to actually clean
up after itself. The "cleanup" depends on noticing at run-time that
some pid is stale and no longer listening. Again, that's just
fundamental brokenness in taskstats.

And it also only look sat pid namespaces. The network namespace issue
is separate.

So that's why I think it should be marked BROKEN. What applications
actually depend on this? iotop and what else? Because if it's just
iotop, I do suspect we might be better off telling people "ok,
disabling this will break iotop, but quite frankly, you're better off
without it".

                          Linus

View attachment "patch.diff" of type "text/x-patch" (2575 bytes)