|
Kyle McMartin |
b13dd03 |
From 2779f26ab085071a8a55d3cf31f31a7d3c3bfcd1 Mon Sep 17 00:00:00 2001
|
|
Kyle McMartin |
b13dd03 |
From: Daniel J Blueman <daniel.blueman@gmail.com>
|
|
Kyle McMartin |
b13dd03 |
Date: Tue, 17 Aug 2010 23:56:55 +0100
|
|
Kyle McMartin |
b13dd03 |
Subject: Fix unprotected access to task credentials in waitid()
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
Using a program like the following:
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
#include <stdlib.h>
|
|
Kyle McMartin |
b13dd03 |
#include <unistd.h>
|
|
Kyle McMartin |
b13dd03 |
#include <sys/types.h>
|
|
Kyle McMartin |
b13dd03 |
#include <sys/wait.h>
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
int main() {
|
|
Kyle McMartin |
b13dd03 |
id_t id;
|
|
Kyle McMartin |
b13dd03 |
siginfo_t infop;
|
|
Kyle McMartin |
b13dd03 |
pid_t res;
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
id = fork();
|
|
Kyle McMartin |
b13dd03 |
if (id == 0) { sleep(1); exit(0); }
|
|
Kyle McMartin |
b13dd03 |
kill(id, SIGSTOP);
|
|
Kyle McMartin |
b13dd03 |
alarm(1);
|
|
Kyle McMartin |
b13dd03 |
waitid(P_PID, id, &infop, WCONTINUED);
|
|
Kyle McMartin |
b13dd03 |
return 0;
|
|
Kyle McMartin |
b13dd03 |
}
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
to call waitid() on a stopped process results in access to the child task's
|
|
Kyle McMartin |
b13dd03 |
credentials without the RCU read lock being held - which may be replaced in the
|
|
Kyle McMartin |
b13dd03 |
meantime - eliciting the following warning:
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
===================================================
|
|
Kyle McMartin |
b13dd03 |
[ INFO: suspicious rcu_dereference_check() usage. ]
|
|
Kyle McMartin |
b13dd03 |
---------------------------------------------------
|
|
Kyle McMartin |
b13dd03 |
kernel/exit.c:1460 invoked rcu_dereference_check() without protection!
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
other info that might help us debug this:
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
rcu_scheduler_active = 1, debug_locks = 1
|
|
Kyle McMartin |
b13dd03 |
2 locks held by waitid02/22252:
|
|
Kyle McMartin |
b13dd03 |
#0: (tasklist_lock){.?.?..}, at: [<ffffffff81061ce5>] do_wait+0xc5/0x310
|
|
Kyle McMartin |
b13dd03 |
#1: (&(&sighand->siglock)->rlock){-.-...}, at: [<ffffffff810611da>]
|
|
Kyle McMartin |
b13dd03 |
wait_consider_task+0x19a/0xbe0
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
stack backtrace:
|
|
Kyle McMartin |
b13dd03 |
Pid: 22252, comm: waitid02 Not tainted 2.6.35-323cd+ #3
|
|
Kyle McMartin |
b13dd03 |
Call Trace:
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff81095da4>] lockdep_rcu_dereference+0xa4/0xc0
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff81061b31>] wait_consider_task+0xaf1/0xbe0
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff81061d15>] do_wait+0xf5/0x310
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff810620b6>] sys_waitid+0x86/0x1f0
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff8105fce0>] ? child_wait_callback+0x0/0x70
|
|
Kyle McMartin |
b13dd03 |
[<ffffffff81003282>] system_call_fastpath+0x16/0x1b
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
This is fixed by holding the RCU read lock in wait_task_continued() to ensure
|
|
Kyle McMartin |
b13dd03 |
that the task's current credentials aren't destroyed between us reading the
|
|
Kyle McMartin |
b13dd03 |
cred pointer and us reading the UID from those credentials.
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
Furthermore, protect wait_task_stopped() in the same way.
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
We don't need to keep holding the RCU read lock once we've read the UID from
|
|
Kyle McMartin |
b13dd03 |
the credentials as holding the RCU read lock doesn't stop the target task from
|
|
Kyle McMartin |
b13dd03 |
changing its creds under us - so the credentials may be outdated immediately
|
|
Kyle McMartin |
b13dd03 |
after we've read the pointer, lock or no lock.
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
|
|
Kyle McMartin |
b13dd03 |
Signed-off-by: David Howells <dhowells@redhat.com>
|
|
Kyle McMartin |
b13dd03 |
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
|
|
Kyle McMartin |
b13dd03 |
Acked-by: Oleg Nesterov <oleg@redhat.com>
|
|
Kyle McMartin |
b13dd03 |
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
Kyle McMartin |
b13dd03 |
---
|
|
Kyle McMartin |
b13dd03 |
kernel/exit.c | 5 ++---
|
|
Kyle McMartin |
b13dd03 |
1 files changed, 2 insertions(+), 3 deletions(-)
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
diff --git a/kernel/exit.c b/kernel/exit.c
|
|
Kyle McMartin |
b13dd03 |
index ceffc67..ac90425 100644
|
|
Kyle McMartin |
b13dd03 |
--- a/kernel/exit.c
|
|
Kyle McMartin |
b13dd03 |
+++ b/kernel/exit.c
|
|
Kyle McMartin |
b13dd03 |
@@ -1383,8 +1383,7 @@ static int wait_task_stopped(struct wait_opts *wo,
|
|
Kyle McMartin |
b13dd03 |
if (!unlikely(wo->wo_flags & WNOWAIT))
|
|
Kyle McMartin |
b13dd03 |
*p_code = 0;
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
- /* don't need the RCU readlock here as we're holding a spinlock */
|
|
Kyle McMartin |
b13dd03 |
- uid = __task_cred(p)->uid;
|
|
Kyle McMartin |
b13dd03 |
+ uid = task_uid(p);
|
|
Kyle McMartin |
b13dd03 |
unlock_sig:
|
|
Kyle McMartin |
b13dd03 |
spin_unlock_irq(&p->sighand->siglock);
|
|
Kyle McMartin |
b13dd03 |
if (!exit_code)
|
|
Kyle McMartin |
b13dd03 |
@@ -1457,7 +1456,7 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p)
|
|
Kyle McMartin |
b13dd03 |
}
|
|
Kyle McMartin |
b13dd03 |
if (!unlikely(wo->wo_flags & WNOWAIT))
|
|
Kyle McMartin |
b13dd03 |
p->signal->flags &= ~SIGNAL_STOP_CONTINUED;
|
|
Kyle McMartin |
b13dd03 |
- uid = __task_cred(p)->uid;
|
|
Kyle McMartin |
b13dd03 |
+ uid = task_uid(p);
|
|
Kyle McMartin |
b13dd03 |
spin_unlock_irq(&p->sighand->siglock);
|
|
Kyle McMartin |
b13dd03 |
|
|
Kyle McMartin |
b13dd03 |
pid = task_pid_vnr(p);
|
|
Kyle McMartin |
b13dd03 |
--
|
|
Kyle McMartin |
b13dd03 |
1.7.3
|
|
Kyle McMartin |
b13dd03 |
|