Re: taskstats root only breaking iotop

Re: taskstats root only breaking iotop

[Guillaume Chazarain]

On Thu, Sep 22, 2011 at 2:32 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>      Make TASKSTATS require root access
>      Make taskstats round statistics down to nearest 1k bytes/events

Hello, iotop author here.

With taskstats being root only, what's the benefit of making the stats
imprecise (not that it matters to iotop)?

How about this plan instead:
- byte level stats only if ptrace_may_access() though iotop can
certainly live with 1k rounded values
- rounded stats or just nothing otherwise
- potentially an extra field in the taskstats structure indicating
when the information is degraded

Thanks

-- 
Guillaume

Re: taskstats root only breaking iotop

[Linus Torvalds]

On Sat, Oct 1, 2011 at 2:54 PM, Guillaume Chazarain <guichaz@gmail.com> wrote:
>
> With taskstats being root only, what's the benefit of making the stats
> imprecise (not that it matters to iotop)?

I think the question should be: why is it *ever* a good idea to let
*anybody* read how many bytes anybody has read.

If you want that kind of detail, do "strace". Don't do that abortion
that is taskstats.

> - byte level stats only if ptrace_may_access() though iotop can
> certainly live with 1k rounded values

Again, do a real trace.

Right now, TASKSTATS is a total and utter disaster. I want to make it
*less* used, not more. And I sure as hell don't want to make it even
*more* of a burden on the kernel by having to do the whole ptrace
check on each and every task.

Seriously. TASKSTATS is crap. It should die. I was *this* close to
deciding to just rip it out entirely. The whole thing is badly
designed, it's known-broken wrt namespaces, and it's a crazy idea to
begin with.

Castrating it a bit and limiting it to root-only made it just
palatable enough that I felt it didn't need to be removed entirely.

But if it gets annoying enough and people try to extend it to be even
more complicated, my response to that will just be "No. HELL NO!".

Especially since I haven't heard a peep about the fact that it was
totally broken wrt namespaces despite reporting it to the people who
were ostensibly in charge of it..

No new complications. People should work at fixing the mess that it
is, rather than trying to make it even messier.

                   Linus

Re: taskstats root only breaking iotop

[Guillaume Chazarain]

On Sat, Oct 1, 2011 at 11:59 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> I think the question should be: why is it *ever* a good idea to let
> *anybody* read how many bytes anybody has read.

I use iotop to get the throughput of a cp or tar because I don't
always think of piping to 'pv'. People also use it as a progress
indicator for their less verbose applications (total bytes
read/written instead of rate).

I see on forums people use it to find which process is hammering their
disk, though in this case the time spent on I/Os is more useful than
the throughput and this is not broken by these changes assuming they
have root.

> If you want that kind of detail, do "strace". Don't do that abortion
> that is taskstats.

strace is not always appropriate: it's invasive as it slows down the
traced process and it's not aggregating anything.

> Right now, TASKSTATS is a total and utter disaster.

I'm not attached to taskstats, I'd be happy to migrate to what comes next.

> and it's a crazy idea to
> begin with.

But it's not clear to me if you dislike just the taskstats
implementation or the general idea of precise per task I/O stats.

Thanks for taking the time to explain your reasoning, I'm not trying
to resurrect taskstats, just checking if I should expect a brighter
future for iotop.

-- 
Guillaume

Re: taskstats root only breaking iotop

[Linus Torvalds]

On Sat, Oct 1, 2011 at 3:41 PM, Guillaume Chazarain <guichaz@gmail.com> wrote:
> On Sat, Oct 1, 2011 at 11:59 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> I think the question should be: why is it *ever* a good idea to let
>> *anybody* read how many bytes anybody has read.
>
> I use iotop to get the throughput of a cp or tar because I don't
> always think of piping to 'pv'. People also use it as a progress
> indicator for their less verbose applications (total bytes
> read/written instead of rate).

I can understand the use of "throughput".

I don't understand why you then even *mention* byte-granularity. NOBODY CARES.

It really is that simple. Either you care about a single process (and
you damn well use strace or something similar), or you care about
"statistics".

For the statistics, something *like* TASKSTATS can make sense, but
never *ever* does it make sense to give byte-granularity stats in that
case.

So I don't see why you ask for it. What could possibly be a valid use-case?

                         Linus

Re: taskstats root only breaking iotop

[Guillaume Chazarain]

On Sun, Oct 2, 2011 at 2:20 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> So I don't see why you ask for it. What could possibly be a valid use-case?

Right, kbyte granularity is enough. And that's consistent with
/proc/vmstat, which nobody is complaining about.

-- 
Guillaume

Re: taskstats root only breaking iotop

[Vasiliy Kulikov]

(cc'ed kernel-hardening)

On Sun, Oct 02, 2011 at 12:22 +0200, Guillaume Chazarain wrote:
> On Sun, Oct 2, 2011 at 2:20 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > So I don't see why you ask for it. What could possibly be a valid use-case?
> 
> Right, kbyte granularity is enough.

It is not enough.  In some border cases an attacker may still learn
private information given the counters with _arbitrary_ granularity:

http://www.openwall.com/lists/oss-security/2011/06/29/9


> And that's consistent with
> /proc/vmstat, which nobody is complaining about.

<jumping with a raised hand>Me, me, it was me!</jumping with a raised hand>

Seriously, most of procfs files were created with relaxed permissions in
old days when nobody thought about such infoleaks.  Now it is much
harder to close all of them without breaking existing users.

http://www.openwall.com/lists/kernel-hardening/2011/07/28/1
http://www.openwall.com/lists/kernel-hardening/2011/09/27/3
http://www.openwall.com/lists/kernel-hardening/2011/09/19/24
http://www.openwall.com/lists/kernel-hardening/2011/09/21/2


Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: taskstats root only breaking iotop

[Adrian Bunk]

On Sun, Oct 02, 2011 at 02:54:57PM +0400, Vasiliy Kulikov wrote:
> (cc'ed kernel-hardening)
> 
> On Sun, Oct 02, 2011 at 12:22 +0200, Guillaume Chazarain wrote:
> > On Sun, Oct 2, 2011 at 2:20 AM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > > So I don't see why you ask for it. What could possibly be a valid use-case?
> > 
> > Right, kbyte granularity is enough.
> 
> It is not enough.  In some border cases an attacker may still learn
> private information given the counters with _arbitrary_ granularity:
> 
> http://www.openwall.com/lists/oss-security/2011/06/29/9

If you request a CVE for that, shouldn't there also be a CVE for
/proc/<pid>/cmdline being readable by all users?

I'd expect "ps -ef" to be more likely to give private information to an 
attacker than counters with kbyte granularity, or am I wrong on that?

>...
> Thanks,

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: taskstats root only breaking iotop

[Balbir Singh]

On Sun, Oct 2, 2011 at 3:29 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sat, Oct 1, 2011 at 2:54 PM, Guillaume Chazarain <guichaz@gmail.com> wrote:
> >
> > With taskstats being root only, what's the benefit of making the stats
> > imprecise (not that it matters to iotop)?
>
> I think the question should be: why is it *ever* a good idea to let
> *anybody* read how many bytes anybody has read.
>
> If you want that kind of detail, do "strace". Don't do that abortion
> that is taskstats.
>
> > - byte level stats only if ptrace_may_access() though iotop can
> > certainly live with 1k rounded values
>
> Again, do a real trace.
>
> Right now, TASKSTATS is a total and utter disaster. I want to make it
> *less* used, not more. And I sure as hell don't want to make it even
> *more* of a burden on the kernel by having to do the whole ptrace
> check on each and every task.
>
> Seriously. TASKSTATS is crap. It should die. I was *this* close to
> deciding to just rip it out entirely. The whole thing is badly
> designed, it's known-broken wrt namespaces, and it's a crazy idea to
> begin with.
>

Namespaces came in later, much later after taskstats. I think your
biased and not even thinking about your bias. What was badly designed?
It was reviewed several times in the community and tested for
performance. I've tried convincing you before, but I need a constant
supply of energy drinks to keep this fight going. How about we be
practical about the issue at hand, IOW

1. What would a better design look like
2. What can we do to fix the broken namespace
3. The theoretical security issues might exist (I say might), but no
one has demonstrated an attack with it. I agree on the ssh key size
issue being exposed via io portion of taskstats, but beyond that I;d a
dummies guide to help me understand.


Balbir Singh

Re: taskstats root only breaking iotop

[Linus Torvalds]

On Mon, Oct 3, 2011 at 7:52 AM, Balbir Singh <bsingharora@gmail.com> wrote:
>>
>
> Namespaces came in later, much later after taskstats. I think your
> biased and not even thinking about your bias.

Umm. Yes, I'm biased. I'm biased as hell by the fact that I reported
the taskstats namespace breakage with a suggested approach to fix it,
and never heard back from anybody involved with it.

And yes, you were cc'd on that.

> What was badly designed?

The fact that it had no security what-so-ever? Unlike /proc, which at
least has the *structure* for security, even if it then often for
historical reasons leaves things a bit too open.

                           Linus

Re: taskstats root only breaking iotop

[Vasiliy Kulikov]

On Mon, Oct 03, 2011 at 15:31 +0300, Adrian Bunk wrote:
> On Sun, Oct 02, 2011 at 02:54:57PM +0400, Vasiliy Kulikov wrote:
> > (cc'ed kernel-hardening)
> > 
> > On Sun, Oct 02, 2011 at 12:22 +0200, Guillaume Chazarain wrote:
> > > On Sun, Oct 2, 2011 at 2:20 AM, Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > > So I don't see why you ask for it. What could possibly be a valid use-case?
> > > 
> > > Right, kbyte granularity is enough.
> > 
> > It is not enough.  In some border cases an attacker may still learn
> > private information given the counters with _arbitrary_ granularity:
> > 
> > http://www.openwall.com/lists/oss-security/2011/06/29/9
> 
> If you request a CVE for that, shouldn't there also be a CVE for
> /proc/<pid>/cmdline being readable by all users?
> 
> I'd expect "ps -ef" to be more likely to give private information to an 
> attacker than counters with kbyte granularity, or am I wrong on that?

I agree that world-readable cmdline can be a privacy issue in some
cases.  I tried to push a patch introducing procfs mount option to
restrict /proc/PID/ to PID owner to address the issue (as world-readable
cmdline and other files are already used by plenty of programs and
unconditionally breaking backward compatibility is not good, a
configuration mechanism is needed), but it didn't receive positive
feedback.  A more detailed explanation from Solar Designer:

http://www.openwall.com/lists/kernel-hardening/2011/06/20/5

Andrew Morton complained that it is too specific to our needs and one
might want to define more fine granted procfs security model:

http://www.openwall.com/lists/kernel-hardening/2011/06/21/3

I've tried to address it and defined per-file policy:

http://www.openwall.com/lists/kernel-hardening/2011/08/10/12

No comments so far :(

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments