From d2e3ee9b29f5de5b01e611b04e6fb29760589b01 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Fri, 17 Jul 2009 09:09:36 +1000 Subject: kernel: fix is_single_threaded - Fix the comment, is_single_threaded(p) actually means that nobody shares ->mm with p. I think this helper should be renamed, and it should not have arguments. With or without this patch it must not be used unless p == current, otherwise we can't safely use p->signal or p->mm. - "if (atomic_read(&p->signal->count) != 1)" is not right when we have a zombie group leader, use signal->live instead. - Add PF_KTHREAD check to skip kernel threads which may borrow p->mm, otherwise we can return the wrong "false". - Use for_each_process() instead of do_each_thread(), all threads must use the same ->mm. - Use down_write(mm->mmap_sem) + rcu_read_lock() instead of tasklist_lock to iterate over the process list. If there is another CLONE_VM process it can't pass exit_mm() which takes the same mm->mmap_sem. We can miss a freshly forked CLONE_VM task, but this doesn't matter because we must see its parent and return false. Signed-off-by: Oleg Nesterov Cc: David Howells Cc: James Morris Cc: Roland McGrath Cc: Stephen Smalley Signed-off-by: Andrew Morton Signed-off-by: James Morris --- lib/is_single_threaded.c | 62 ++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 26 deletions(-) (limited to 'lib') diff --git a/lib/is_single_threaded.c b/lib/is_single_threaded.c index f1ed2fe..2762516 100644 --- a/lib/is_single_threaded.c +++ b/lib/is_single_threaded.c @@ -12,34 +12,44 @@ #include -/** - * is_single_threaded - Determine if a thread group is single-threaded or not - * @p: A task in the thread group in question - * - * This returns true if the thread group to which a task belongs is single - * threaded, false if it is not. +/* + * Returns true if the task does not share ->mm with another thread/process. */ -bool is_single_threaded(struct task_struct *p) +bool is_single_threaded(struct task_struct *task) { - struct task_struct *g, *t; - struct mm_struct *mm = p->mm; - - if (atomic_read(&p->signal->count) != 1) - goto no; - - if (atomic_read(&p->mm->mm_users) != 1) { - read_lock(&tasklist_lock); - do_each_thread(g, t) { - if (t->mm == mm && t != p) - goto no_unlock; - } while_each_thread(g, t); - read_unlock(&tasklist_lock); - } + struct mm_struct *mm = task->mm; + struct task_struct *p, *t; + bool ret; + + might_sleep(); + + if (atomic_read(&task->signal->live) != 1) + return false; - return true; + if (atomic_read(&mm->mm_users) == 1) + return true; + + ret = false; + down_write(&mm->mmap_sem); + rcu_read_lock(); + for_each_process(p) { + if (unlikely(p->flags & PF_KTHREAD)) + continue; + if (unlikely(p == task->group_leader)) + continue; + + t = p; + do { + if (unlikely(t->mm == mm)) + goto found; + if (likely(t->mm)) + break; + } while_each_thread(p, t); + } + ret = true; +found: + rcu_read_unlock(); + up_write(&mm->mmap_sem); -no_unlock: - read_unlock(&tasklist_lock); -no: - return false; + return ret; } -- cgit v1.1 From 5bb459bb45d1ad3c177485dcf0af01580aa31125 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Fri, 10 Jul 2009 03:48:23 +0200 Subject: kernel: rename is_single_threaded(task) to current_is_single_threaded(void) - is_single_threaded(task) is not safe unless task == current, we can't use task->signal or task->mm. - it doesn't make sense unless task == current, the task can fork right after the check. Rename it to current_is_single_threaded() and kill the argument. Signed-off-by: Oleg Nesterov Acked-by: David Howells Signed-off-by: James Morris --- lib/is_single_threaded.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/is_single_threaded.c b/lib/is_single_threaded.c index 2762516..4340109 100644 --- a/lib/is_single_threaded.c +++ b/lib/is_single_threaded.c @@ -15,8 +15,9 @@ /* * Returns true if the task does not share ->mm with another thread/process. */ -bool is_single_threaded(struct task_struct *task) +bool current_is_single_threaded(void) { + struct task_struct *task = current; struct mm_struct *mm = task->mm; struct task_struct *p, *t; bool ret; -- cgit v1.1 From 967cc5371113f9806b39a2ebb2174af2883d96fe Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Thu, 9 Jul 2009 23:28:49 +0200 Subject: kernel: is_current_single_threaded: don't use ->mmap_sem is_current_single_threaded() can safely miss a freshly forked CLONE_VM task, but in this case it must not miss its parent. That is why we take mm->mmap_sem for writing to make sure a thread/task with the same ->mm can't pass exit_mm() and disappear. However we can avoid ->mmap_sem and rely on rcu/barriers: - if we do not see the exiting parent on thread/process list we see the result of list_del_rcu(), in this case we must also see the result of list_add_rcu() which does wmb(). - if we do see the parent but its ->mm == NULL, we need rmb() to make sure we can't miss the child. Signed-off-by: Oleg Nesterov Acked-by: David Howells Signed-off-by: James Morris --- lib/is_single_threaded.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/is_single_threaded.c b/lib/is_single_threaded.c index 4340109..bd2bea9 100644 --- a/lib/is_single_threaded.c +++ b/lib/is_single_threaded.c @@ -22,8 +22,6 @@ bool current_is_single_threaded(void) struct task_struct *p, *t; bool ret; - might_sleep(); - if (atomic_read(&task->signal->live) != 1) return false; @@ -31,7 +29,6 @@ bool current_is_single_threaded(void) return true; ret = false; - down_write(&mm->mmap_sem); rcu_read_lock(); for_each_process(p) { if (unlikely(p->flags & PF_KTHREAD)) @@ -45,12 +42,17 @@ bool current_is_single_threaded(void) goto found; if (likely(t->mm)) break; + /* + * t->mm == NULL. Make sure next_thread/next_task + * will see other CLONE_VM tasks which might be + * forked before exiting. + */ + smp_rmb(); } while_each_thread(p, t); } ret = true; found: rcu_read_unlock(); - up_write(&mm->mmap_sem); return ret; } -- cgit v1.1 From e0e817392b9acf2c98d3be80c233dddb1b52003d Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 2 Sep 2009 09:13:40 +0100 Subject: CRED: Add some configurable debugging [try #6] Add a config option (CONFIG_DEBUG_CREDENTIALS) to turn on some debug checking for credential management. The additional code keeps track of the number of pointers from task_structs to any given cred struct, and checks to see that this number never exceeds the usage count of the cred struct (which includes all references, not just those from task_structs). Furthermore, if SELinux is enabled, the code also checks that the security pointer in the cred struct is never seen to be invalid. This attempts to catch the bug whereby inode_has_perm() faults in an nfsd kernel thread on seeing cred->security be a NULL pointer (it appears that the credential struct has been previously released): http://www.kerneloops.org/oops.php?number=252883 Signed-off-by: David Howells Signed-off-by: James Morris --- lib/Kconfig.debug | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'lib') diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 12327b2..fbb87cf 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -653,6 +653,21 @@ config DEBUG_NOTIFIERS This is a relatively cheap check but if you care about maximum performance, say N. +config DEBUG_CREDENTIALS + bool "Debug credential management" + depends on DEBUG_KERNEL + help + Enable this to turn on some debug checking for credential + management. The additional code keeps track of the number of + pointers from task_structs to any given cred struct, and checks to + see that this number never exceeds the usage count of the cred + struct. + + Furthermore, if SELinux is enabled, this also checks that the + security pointer in the cred struct is never seen to be invalid. + + If unsure, say N. + # # Select this config option from the architecture Kconfig, if it # it is preferred to always offer frame pointers as a config -- cgit v1.1