[kernel/f14/master] Slay another rcu_dereference_check warning pointed out by rmcgrath

Kyle McMartin kyle at fedoraproject.org
Fri Sep 24 02:32:04 UTC 2010


commit b13dd03f62d92090a1d147aa4b92087aa29f7d1a
Author: Kyle McMartin <kyle at dreadnought.i.jkkm.org>
Date:   Thu Sep 23 20:59:39 2010 -0400

    Slay another rcu_dereference_check warning pointed out by rmcgrath

 ...cted-access-to-task-credentials-in-whatid.patch |   98 ++++++++++++++++++++
 kernel.spec                                        |    6 +
 2 files changed, 104 insertions(+), 0 deletions(-)
---
diff --git a/fix-unprotected-access-to-task-credentials-in-whatid.patch b/fix-unprotected-access-to-task-credentials-in-whatid.patch
new file mode 100644
index 0000000..2fd8b53
--- /dev/null
+++ b/fix-unprotected-access-to-task-credentials-in-whatid.patch
@@ -0,0 +1,98 @@
+From 2779f26ab085071a8a55d3cf31f31a7d3c3bfcd1 Mon Sep 17 00:00:00 2001
+From: Daniel J Blueman <daniel.blueman at gmail.com>
+Date: Tue, 17 Aug 2010 23:56:55 +0100
+Subject: Fix unprotected access to task credentials in waitid()
+
+Using a program like the following:
+
+	#include <stdlib.h>
+	#include <unistd.h>
+	#include <sys/types.h>
+	#include <sys/wait.h>
+
+	int main() {
+		id_t id;
+		siginfo_t infop;
+		pid_t res;
+
+		id = fork();
+		if (id == 0) { sleep(1); exit(0); }
+		kill(id, SIGSTOP);
+		alarm(1);
+		waitid(P_PID, id, &infop, WCONTINUED);
+		return 0;
+	}
+
+to call waitid() on a stopped process results in access to the child task's
+credentials without the RCU read lock being held - which may be replaced in the
+meantime - eliciting the following warning:
+
+	===================================================
+	[ INFO: suspicious rcu_dereference_check() usage. ]
+	---------------------------------------------------
+	kernel/exit.c:1460 invoked rcu_dereference_check() without protection!
+
+	other info that might help us debug this:
+
+	rcu_scheduler_active = 1, debug_locks = 1
+	2 locks held by waitid02/22252:
+	 #0:  (tasklist_lock){.?.?..}, at: [<ffffffff81061ce5>] do_wait+0xc5/0x310
+	 #1:  (&(&sighand->siglock)->rlock){-.-...}, at: [<ffffffff810611da>]
+	wait_consider_task+0x19a/0xbe0
+
+	stack backtrace:
+	Pid: 22252, comm: waitid02 Not tainted 2.6.35-323cd+ #3
+	Call Trace:
+	 [<ffffffff81095da4>] lockdep_rcu_dereference+0xa4/0xc0
+	 [<ffffffff81061b31>] wait_consider_task+0xaf1/0xbe0
+	 [<ffffffff81061d15>] do_wait+0xf5/0x310
+	 [<ffffffff810620b6>] sys_waitid+0x86/0x1f0
+	 [<ffffffff8105fce0>] ? child_wait_callback+0x0/0x70
+	 [<ffffffff81003282>] system_call_fastpath+0x16/0x1b
+
+This is fixed by holding the RCU read lock in wait_task_continued() to ensure
+that the task's current credentials aren't destroyed between us reading the
+cred pointer and us reading the UID from those credentials.
+
+Furthermore, protect wait_task_stopped() in the same way.
+
+We don't need to keep holding the RCU read lock once we've read the UID from
+the credentials as holding the RCU read lock doesn't stop the target task from
+changing its creds under us - so the credentials may be outdated immediately
+after we've read the pointer, lock or no lock.
+
+Signed-off-by: Daniel J Blueman <daniel.blueman at gmail.com>
+Signed-off-by: David Howells <dhowells at redhat.com>
+Acked-by: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
+Acked-by: Oleg Nesterov <oleg at redhat.com>
+Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
+---
+ kernel/exit.c |    5 ++---
+ 1 files changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/kernel/exit.c b/kernel/exit.c
+index ceffc67..ac90425 100644
+--- a/kernel/exit.c
++++ b/kernel/exit.c
+@@ -1383,8 +1383,7 @@ static int wait_task_stopped(struct wait_opts *wo,
+ 	if (!unlikely(wo->wo_flags & WNOWAIT))
+ 		*p_code = 0;
+ 
+-	/* don't need the RCU readlock here as we're holding a spinlock */
+-	uid = __task_cred(p)->uid;
++	uid = task_uid(p);
+ unlock_sig:
+ 	spin_unlock_irq(&p->sighand->siglock);
+ 	if (!exit_code)
+@@ -1457,7 +1456,7 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p)
+ 	}
+ 	if (!unlikely(wo->wo_flags & WNOWAIT))
+ 		p->signal->flags &= ~SIGNAL_STOP_CONTINUED;
+-	uid = __task_cred(p)->uid;
++	uid = task_uid(p);
+ 	spin_unlock_irq(&p->sighand->siglock);
+ 
+ 	pid = task_pid_vnr(p);
+-- 
+1.7.3
+
diff --git a/kernel.spec b/kernel.spec
index 78e86dd..a272ee9 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -724,6 +724,7 @@ Patch12080: kprobes-x86-fix-kprobes-to-skip-prefixes-correctly.patch
 # rhbz #622149
 Patch12085: fix-rcu_deref_check-warning.patch
 Patch12086: linux-2.6-cgroups-rcu.patch
+Patch12087: fix-unprotected-access-to-task-credentials-in-whatid.patch
 
 # rhbz #513530
 Patch12090: dell-wmi-add-support-for-eject-key-studio-1555.patch
@@ -1364,6 +1365,7 @@ ApplyPatch kprobes-x86-fix-kprobes-to-skip-prefixes-correctly.patch
 # bz 622149
 ApplyPatch fix-rcu_deref_check-warning.patch
 ApplyPatch linux-2.6-cgroups-rcu.patch
+ApplyPatch fix-unprotected-access-to-task-credentials-in-whatid.patch
 
 # bz 513530
 ApplyPatch dell-wmi-add-support-for-eject-key-studio-1555.patch
@@ -1984,6 +1986,10 @@ fi
 
 %changelog
 * Thu Sep 23 2010 Kyle McMartin <kyle at redhat.com> 2.6.35.5-31
+- Slay another rcu_dereference_check warning pointed out by
+  rmcgrath at .
+
+* Thu Sep 23 2010 Kyle McMartin <kyle at redhat.com>
 - Enable -debug flavours and switch default image to release builds.
 - Bump NR_CPUS on i686 to 64.
 


More information about the scm-commits mailing list