Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 28 Jun 2011 17:49:24 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Vasiliy Kulikov <segoon@...nwall.com>, oss-security@...ts.openwall.com,
        security@...nel.org
Subject: Re: [Security] CVE request: kernel: taskstats/procfs io infoleak
 (was: taskstats authorized_keys presence infoleak PoC)

On Tue, Jun 28, 2011 at 5:12 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
>> If rounding the counts to a 1k granularity will indeed defeat the
>> attack (I'm unsure) then I'd suggest that a fix would be to perform
>> that fuzzification if the receiving process doesn't have suitable
>> permissions.  So if the user is reading his own stats or is root, he
>> still gets byte-resolution results.  This keeps the stats as useful as
>> we can make them and reduces the back-compatibility damage.
>
> Sure.

Actually, due to the whole netlink thing, it's not obvious who the
data goes to, so I think the taskstats interface simply needs to round
unconditionally.

If you want the exact thing, you can use /proc/<pid>/io, which now
does the security checking as per Vasiliy.

So some patch like the appended? Vasiliy, this is different from your
2/2, but it's simpler and I think sufficient. And shouldn't break
iotop. What do you think? I agree that it's not perfect, but it seems
to be sufficient at least for the particular passwd attack, no? Or is
there some way you can fool sshd to read some other user-supplied data
so that you can trick it into giving multiple values that you control,
and thus see exactly when the IO counts overflow..

                   Linus

 kernel/tsacct.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 24dc60d9fa1f..197749dbe0f0 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -76,8 +76,10 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 
 #ifdef CONFIG_TASK_XACCT
 
+/* Expose things in kB granularity */
 #define KB 1024
 #define MB (1024*KB)
+#define MASKED(x) (x & ~(KB-1))
 /*
  * fill in extended accounting fields
  */
@@ -95,14 +97,14 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
 		stats->hiwater_vm    = get_mm_hiwater_vm(mm)  * PAGE_SIZE / KB;
 		mmput(mm);
 	}
-	stats->read_char	= p->ioac.rchar;
-	stats->write_char	= p->ioac.wchar;
-	stats->read_syscalls	= p->ioac.syscr;
-	stats->write_syscalls	= p->ioac.syscw;
+	stats->read_char	= MASKED(p->ioac.rchar);
+	stats->write_char	= MASKED(p->ioac.wchar);
+	stats->read_syscalls	= MASKED(p->ioac.syscr);
+	stats->write_syscalls	= MASKED(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	= MASKED(p->ioac.read_bytes);
+	stats->write_bytes	= MASKED(p->ioac.write_bytes);
+	stats->cancelled_write_bytes = MASKED(p->ioac.cancelled_write_bytes);
 #else
 	stats->read_bytes	= 0;
 	stats->write_bytes	= 0;
@@ -111,6 +113,7 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
 }
 #undef KB
 #undef MB
+#undef MASKED
 
 /**
  * acct_update_integrals - update mm integral fields in task_struct

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ