Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754539Ab1GADCJ (ORCPT ); Thu, 30 Jun 2011 23:02:09 -0400 Received: from mail-iw0-f174.google.com ([209.85.214.174]:44190 "EHLO mail-iw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753687Ab1GADCI (ORCPT ); Thu, 30 Jun 2011 23:02:08 -0400 MIME-Version: 1.0 In-Reply-To: References: <1308917362-4795-1-git-send-email-segoon@openwall.com> <20110630075716.GB3377@albatros> Date: Fri, 1 Jul 2011 08:32:07 +0530 Message-ID: Subject: Re: [Security] [PATCH 2/2] taskstats: restrict access to user From: Balbir Singh To: Linus Torvalds Cc: Vasiliy Kulikov , Shailabh Nagar , linux-kernel@vger.kernel.org, security@kernel.org, Eric Paris , Stephen Wilson , KOSAKI Motohiro , David Rientjes , Andrew Morton , Balbir Singh , kernel-hardening@lists.openwall.com Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3728 Lines: 88 On Thu, Jun 30, 2011 at 10:10 PM, Linus Torvalds wrote: > On Thu, Jun 30, 2011 at 3:59 AM, Balbir Singh wrote: >> On Thu, Jun 30, 2011 at 1:27 PM, Vasiliy Kulikov 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. > I've tried that, but fundamentally the statistics we export in taskstats is what a utility like top would need. Compare them against cat /proc/$pid/sched and /proc/$pid/status > TASKSTATS? Nothing. Nada. Completely open. > > That's just broken. > Except for the I/O part, which turned about to be a cause of concern, look at struct taskstats and please see the comment above. We have some additional delay statistics, but I don't see them as a core broken issue > 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. > Or namespaces are not being used as much, I know a lot of users use iotop(), I've frequently gotten and handled requests to enable the feature across distros > 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. > That is lazy cleanup to avoid trapping exit events of listeners and then having to check against the list. But if the cleanup needs to be more active, it should be easy to fix. What I am saying is that it is not as BROKEN as you make it sound. The attached patch is definitely a step in the right direction and should be applied. > 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". I beg to differ, due to the reasons above. I'd rather find time and fix the pending issues (network namespace), you've fixed the pid namespace issue. I'd also look for exiting listeners Balbir Singh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/