[PATCH] Fix possible futex mmap_sem deadlock

Some futex functions do get_user calls while holding mmap_sem for
reading.  If get_user() faults, and another thread happens to be in mmap
(or somewhere else holding waiting on down_write for the same
semaphore), then do_page_fault will deadlock.  Most architectures seem
to be exposed to this.

To avoid it, make sure the page is available.  If not, release the
semaphore, fault it in and retry.

I also found another exposure by inspection, moving some of the code
around avoids the possible deadlock there.

Signed-off-by: Olof Johansson <olof@austin.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/kernel/futex.c b/kernel/futex.c
index 645a4301f1e60fe0bff5f8d7a6e278bddfa951fd..0977ce91129d16ed06b5883b10e220e0fb7d36c5 100644 (file)
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -258,6 +258,18 @@ static void drop_key_refs(union futex_key *key)
        }
 }
 
+static inline int get_futex_value_locked(int *dest, int __user *from)
+{
+       int ret;
+
+       inc_preempt_count();
+       ret = __copy_from_user_inatomic(dest, from, sizeof(int));
+       dec_preempt_count();
+       preempt_check_resched();
+
+       return ret ? -EFAULT : 0;
+}
+
 /*
  * The hash bucket lock must be held when this is called.
  * Afterwards, the futex_q must not be accessed.
@@ -329,6 +341,7 @@ static int futex_requeue(unsigned long uaddr1, unsigned long uaddr2,
        int ret, drop_count = 0;
        unsigned int nqueued;
 
+ retry:
        down_read(&current->mm->mmap_sem);
 
        ret = get_futex_key(uaddr1, &key1);
@@ -355,9 +368,20 @@ static int futex_requeue(unsigned long uaddr1, unsigned long uaddr2,
                   before *uaddr1.  */
                smp_mb();
 
-               if (get_user(curval, (int __user *)uaddr1) != 0) {
-                       ret = -EFAULT;
-                       goto out;
+               ret = get_futex_value_locked(&curval, (int __user *)uaddr1);
+
+               if (unlikely(ret)) {
+                       /* If we would have faulted, release mmap_sem, fault
+                        * it in and start all over again.
+                        */
+                       up_read(&current->mm->mmap_sem);
+
+                       ret = get_user(curval, (int __user *)uaddr1);
+
+                       if (!ret)
+                               goto retry;
+
+                       return ret;
                }
                if (curval != *valp) {
                        ret = -EAGAIN;
@@ -480,6 +504,7 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time)
        int ret, curval;
        struct futex_q q;
 
+ retry:
        down_read(&current->mm->mmap_sem);
 
        ret = get_futex_key(uaddr, &q.key);
@@ -508,9 +533,23 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time)
         * We hold the mmap semaphore, so the mapping cannot have changed
         * since we looked it up in get_futex_key.
         */
-       if (get_user(curval, (int __user *)uaddr) != 0) {
-               ret = -EFAULT;
-               goto out_unqueue;
+
+       ret = get_futex_value_locked(&curval, (int __user *)uaddr);
+
+       if (unlikely(ret)) {
+               /* If we would have faulted, release mmap_sem, fault it in and
+                * start all over again.
+                */
+               up_read(&current->mm->mmap_sem);
+
+               if (!unqueue_me(&q)) /* There's a chance we got woken already */
+                       return 0;
+
+               ret = get_user(curval, (int __user *)uaddr);
+
+               if (!ret)
+                       goto retry;
+               return ret;
        }
        if (curval != val) {
                ret = -EWOULDBLOCK;

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index c50402632c6d38e483e0029cc08d04c0d212761e..1511786462f3847234a8de44dfe8439956a62479 100644 (file)
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -524,9 +524,13 @@ asmlinkage long sys_get_mempolicy(int __user *policy,
        } else
                pval = pol->policy;
 
-       err = -EFAULT;
+       if (vma) {
+               up_read(&current->mm->mmap_sem);
+               vma = NULL;
+       }
+
        if (policy && put_user(pval, policy))
-               goto out;
+               return -EFAULT;
 
        err = 0;
        if (nmask) {