]> pilppa.com Git - linux-2.6-omap-h63xx.git/commitdiff
KVM: s390: fix locking order problem in enable_sie
authorChristian Borntraeger <borntraeger@de.ibm.com>
Wed, 21 May 2008 11:37:29 +0000 (13:37 +0200)
committerAvi Kivity <avi@qumranet.com>
Fri, 6 Jun 2008 18:08:26 +0000 (21:08 +0300)
There are potential locking problem in enable_sie. We take the task_lock
and the mmap_sem. As exit_mm uses the same locks vice versa, this triggers
a lockdep warning.
The second problem is that dup_mm and mmput might sleep, so we must not
hold the task_lock at that moment.

The solution is to dup the mm unconditional and use the task_lock before and
afterwards to check  if we can use the new mm. dup_mm and mmput are called
outside the task_lock, but we run update_mm while holding the task_lock,
protection us against ptrace.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
arch/s390/mm/pgtable.c

index 5c1aea97cd1229b2e231c7dd838ee71663087316..3d98ba82ea67a321620c94e387276700c65b7081 100644 (file)
@@ -254,36 +254,46 @@ void disable_noexec(struct mm_struct *mm, struct task_struct *tsk)
 int s390_enable_sie(void)
 {
        struct task_struct *tsk = current;
-       struct mm_struct *mm;
-       int rc;
+       struct mm_struct *mm, *old_mm;
 
-       task_lock(tsk);
-
-       rc = 0;
+       /* Do we have pgstes? if yes, we are done */
        if (tsk->mm->context.pgstes)
-               goto unlock;
+               return 0;
 
-       rc = -EINVAL;
+       /* lets check if we are allowed to replace the mm */
+       task_lock(tsk);
        if (!tsk->mm || atomic_read(&tsk->mm->mm_users) > 1 ||
-           tsk->mm != tsk->active_mm || tsk->mm->ioctx_list)
-               goto unlock;
+           tsk->mm != tsk->active_mm || tsk->mm->ioctx_list) {
+               task_unlock(tsk);
+               return -EINVAL;
+       }
+       task_unlock(tsk);
 
-       tsk->mm->context.pgstes = 1;    /* dirty little tricks .. */
+       /* we copy the mm with pgstes enabled */
+       tsk->mm->context.pgstes = 1;
        mm = dup_mm(tsk);
        tsk->mm->context.pgstes = 0;
-
-       rc = -ENOMEM;
        if (!mm)
-               goto unlock;
-       mmput(tsk->mm);
+               return -ENOMEM;
+
+       /* Now lets check again if somebody attached ptrace etc */
+       task_lock(tsk);
+       if (!tsk->mm || atomic_read(&tsk->mm->mm_users) > 1 ||
+           tsk->mm != tsk->active_mm || tsk->mm->ioctx_list) {
+               mmput(mm);
+               task_unlock(tsk);
+               return -EINVAL;
+       }
+
+       /* ok, we are alone. No ptrace, no threads, etc. */
+       old_mm = tsk->mm;
        tsk->mm = tsk->active_mm = mm;
        preempt_disable();
        update_mm(mm, tsk);
        cpu_set(smp_processor_id(), mm->cpu_vm_mask);
        preempt_enable();
-       rc = 0;
-unlock:
        task_unlock(tsk);
-       return rc;
+       mmput(old_mm);
+       return 0;
 }
 EXPORT_SYMBOL_GPL(s390_enable_sie);