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