lkml.org 
[lkml]   [2011]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [Security] [PATCH 2/2] taskstats: restrict access to user
On Thu, Jun 30, 2011 at 8:02 PM, Balbir Singh <bsingharora@gmail.com> wrote:
>>
>> 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

So nothing ever happened on this thread, afaik.

You can still read sensitive information at a byte granularity with taskstats.

Balbir never sent any of the fixes he was supposed to, and none of the
namespace issues have gotten fixed.

It's now almost three months later, and things are still equally broken.

I think we need to just disable TASKSTAT's. Nobody maintains it, it's
been a known issue for months, people pointed out problems and even
sent patches, and nothing happened.

Maybe we can minimize it with the appended patch, but dammit, we need
to do *something*. If I don't get any reasonable replies, I'm really
going to have to mark this as known-BROKEN, since nothing ever
happens, and the "maintainer" clearly doesn't care about security
issues.

Linus
kernel/taskstats.c | 2 ++
kernel/tsacct.c | 7 ++++---
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index e19ce1454ee1..5874d4866b3c 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -457,6 +457,8 @@ static int cmd_attr_register_cpumask(struct genl_info *info)
cpumask_var_t mask;
int rc;

+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
if (!alloc_cpumask_var(&mask, GFP_KERNEL))
return -ENOMEM;
rc = parse(info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK], mask);
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 24dc60d9fa1f..110ca5a03bd6 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -78,6 +78,7 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)

#define KB 1024
#define MB (1024*KB)
+#define KB_MASK (~(KB-1))
/*
* fill in extended accounting fields
*/
@@ -100,9 +101,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
stats->read_syscalls = p->ioac.syscr;
stats->write_syscalls = p->ioac.syscw;
#ifdef CONFIG_TASK_IO_ACCOUNTING
- stats->read_bytes = p->ioac.read_bytes;
- stats->write_bytes = p->ioac.write_bytes;
- stats->cancelled_write_bytes = p->ioac.cancelled_write_bytes;
+ stats->read_bytes = p->ioac.read_bytes & KB_MASK;
+ stats->write_bytes = p->ioac.write_bytes & KB_MASK;
+ stats->cancelled_write_bytes = p->ioac.cancelled_write_bytes & KB_MASK;
#else
stats->read_bytes = 0;
stats->write_bytes = 0;
\
 
 \ /
  Last update: 2011-09-19 18:43    [W:0.095 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site