aboutsummaryrefslogtreecommitdiffstats
path: root/kernel
diff options
context:
space:
mode:
authorBalbir Singh <balbir@linux.vnet.ibm.com>2008-09-28 18:09:31 -0400
committerLinus Torvalds <torvalds@linux-foundation.org>2008-09-29 11:41:47 -0400
commit31a78f23bac0069004e69f98808b6988baccb6b6 (patch)
treeedca8cffb4682de6be2e79b0b8d381dbb1b70964 /kernel
parentbf5cb66447e7d9f7f111c1d0ebb6d7c90ec24b4d (diff)
mm owner: fix race between swapoff and exit
There's a race between mm->owner assignment and swapoff, more easily seen when task slab poisoning is turned on. The condition occurs when try_to_unuse() runs in parallel with an exiting task. A similar race can occur with callers of get_task_mm(), such as /proc/<pid>/<mmstats> or ptrace or page migration. CPU0 CPU1 try_to_unuse looks at mm = task0->mm increments mm->mm_users task 0 exits mm->owner needs to be updated, but no new owner is found (mm_users > 1, but no other task has task->mm = task0->mm) mm_update_next_owner() leaves mmput(mm) decrements mm->mm_users task0 freed dereferencing mm->owner fails The fix is to notify the subsystem via mm_owner_changed callback(), if no new owner is found, by specifying the new task as NULL. Jiri Slaby: mm->owner was set to NULL prior to calling cgroup_mm_owner_callbacks(), but must be set after that, so as not to pass NULL as old owner causing oops. Daisuke Nishimura: mm_update_next_owner() may set mm->owner to NULL, but mem_cgroup_from_task() and its callers need to take account of this situation to avoid oops. Hugh Dickins: Lockdep warning and hang below exec_mmap() when testing these patches. exit_mm() up_reads mmap_sem before calling mm_update_next_owner(), so exec_mmap() now needs to do the same. And with that repositioning, there's now no point in mm_need_new_owner() allowing for NULL mm. Reported-by: Hugh Dickins <hugh@veritas.com> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com> Signed-off-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Signed-off-by: Hugh Dickins <hugh@veritas.com> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: Paul Menage <menage@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'kernel')
-rw-r--r--kernel/cgroup.c5
-rw-r--r--kernel/exit.c12
2 files changed, 13 insertions, 4 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 13932abde159..a0123d75ec9a 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2738,14 +2738,15 @@ void cgroup_fork_callbacks(struct task_struct *child)
2738 */ 2738 */
2739void cgroup_mm_owner_callbacks(struct task_struct *old, struct task_struct *new) 2739void cgroup_mm_owner_callbacks(struct task_struct *old, struct task_struct *new)
2740{ 2740{
2741 struct cgroup *oldcgrp, *newcgrp; 2741 struct cgroup *oldcgrp, *newcgrp = NULL;
2742 2742
2743 if (need_mm_owner_callback) { 2743 if (need_mm_owner_callback) {
2744 int i; 2744 int i;
2745 for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) { 2745 for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
2746 struct cgroup_subsys *ss = subsys[i]; 2746 struct cgroup_subsys *ss = subsys[i];
2747 oldcgrp = task_cgroup(old, ss->subsys_id); 2747 oldcgrp = task_cgroup(old, ss->subsys_id);
2748 newcgrp = task_cgroup(new, ss->subsys_id); 2748 if (new)
2749 newcgrp = task_cgroup(new, ss->subsys_id);
2749 if (oldcgrp == newcgrp) 2750 if (oldcgrp == newcgrp)
2750 continue; 2751 continue;
2751 if (ss->mm_owner_changed) 2752 if (ss->mm_owner_changed)
diff --git a/kernel/exit.c b/kernel/exit.c
index 16395644a98f..85a83c831856 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -583,8 +583,6 @@ mm_need_new_owner(struct mm_struct *mm, struct task_struct *p)
583 * If there are other users of the mm and the owner (us) is exiting 583 * If there are other users of the mm and the owner (us) is exiting
584 * we need to find a new owner to take on the responsibility. 584 * we need to find a new owner to take on the responsibility.
585 */ 585 */
586 if (!mm)
587 return 0;
588 if (atomic_read(&mm->mm_users) <= 1) 586 if (atomic_read(&mm->mm_users) <= 1)
589 return 0; 587 return 0;
590 if (mm->owner != p) 588 if (mm->owner != p)
@@ -627,6 +625,16 @@ retry:
627 } while_each_thread(g, c); 625 } while_each_thread(g, c);
628 626
629 read_unlock(&tasklist_lock); 627 read_unlock(&tasklist_lock);
628 /*
629 * We found no owner yet mm_users > 1: this implies that we are
630 * most likely racing with swapoff (try_to_unuse()) or /proc or
631 * ptrace or page migration (get_task_mm()). Mark owner as NULL,
632 * so that subsystems can understand the callback and take action.
633 */
634 down_write(&mm->mmap_sem);
635 cgroup_mm_owner_callbacks(mm->owner, NULL);
636 mm->owner = NULL;
637 up_write(&mm->mmap_sem);
630 return; 638 return;
631 639
632assign_new_owner: 640assign_new_owner: