mm: fix move/migrate_pages() race on task struct

Migration functions perform the rcu_read_unlock too early.  As a result
the task pointed to may change from under us.  This can result in an oops,
as reported by Dave Hansen in https://lkml.org/lkml/2012/2/23/302.

The following patch extend the period of the rcu_read_lock until after the
permissions checks are done.  We also take a refcount so that the task
reference is stable when calling security check functions and performing
cpuset node validation (which takes a mutex).

The refcount is dropped before actual page migration occurs so there is no
change to the refcounts held during page migration.

Also move the determination of the mm of the task struct to immediately
before the do_migrate*() calls so that it is clear that we switch from
handling the task during permission checks to the mm for the actual
migration.  Since the determination is only done once and we then no
longer use the task_struct we can be sure that we operate on a specific
address space that will not change from under us.

[akpm@linux-foundation.org: checkpatch fixes]
Signed-off-by: Christoph Lameter <cl@linux.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Reported-by: Dave Hansen <dave@linux.vnet.ibm.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 19 insertions, 13 deletions

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 0a3757067631..71e1a523e209 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1323,12 +1323,9 @@ SYSCALL_DEFINE4(migrate_pages, pid_t, pid, unsigned long, maxnode,
                 err = -ESRCH;
                 goto out;
         }
-        mm = get_task_mm(task);
-        rcu_read_unlock();
+        get_task_struct(task);
 
         err = -EINVAL;
-        if (!mm)
-                goto out;
 
         /*
          * Check if this process has the right to modify the specified
@@ -1336,14 +1333,13 @@ SYSCALL_DEFINE4(migrate_pages, pid_t, pid, unsigned long, maxnode,
          * capabilities, superuser privileges or the same
          * userid as the target process.
          */
-        rcu_read_lock();
         tcred = __task_cred(task);
         if (cred->euid != tcred->suid && cred->euid != tcred->uid &&
             cred->uid  != tcred->suid && cred->uid  != tcred->uid &&
             !capable(CAP_SYS_NICE)) {
                 rcu_read_unlock();
                 err = -EPERM;
-                goto out;
+                goto out_put;
         }
         rcu_read_unlock();
 
@@ -1351,26 +1347,36 @@ SYSCALL_DEFINE4(migrate_pages, pid_t, pid, unsigned long, maxnode,
         /* Is the user allowed to access the target nodes? */
         if (!nodes_subset(*new, task_nodes) && !capable(CAP_SYS_NICE)) {
                 err = -EPERM;
-                goto out;
+                goto out_put;
         }
 
         if (!nodes_subset(*new, node_states[N_HIGH_MEMORY])) {
                 err = -EINVAL;
-                goto out;
+                goto out_put;
         }
 
         err = security_task_movememory(task);
         if (err)
-                goto out;
+                goto out_put;
 
-        err = do_migrate_pages(mm, old, new,
-                capable(CAP_SYS_NICE) ? MPOL_MF_MOVE_ALL : MPOL_MF_MOVE);
-out:
+        mm = get_task_mm(task);
+        put_task_struct(task);
         if (mm)
-                mmput(mm);
+                err = do_migrate_pages(mm, old, new,
+                        capable(CAP_SYS_NICE) ? MPOL_MF_MOVE_ALL : MPOL_MF_MOVE);
+        else
+                err = -EINVAL;
+
+        mmput(mm);
+out:
         NODEMASK_SCRATCH_FREE(scratch);
 
         return err;
+
+out_put:
+        put_task_struct(task);
+        goto out;
+
 }