| From 7ada876a8703f23befbb20a7465a702ee39b1704 Mon Sep 17 00:00:00 2001 |
| From: Darren Hart <dvhart@linux.intel.com> |
| Date: Sun, 17 Oct 2010 08:35:04 -0700 |
| Subject: futex: Fix errors in nested key ref-counting |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Darren Hart <dvhart@linux.intel.com> |
| |
| commit 7ada876a8703f23befbb20a7465a702ee39b1704 upstream. |
| |
| futex_wait() is leaking key references due to futex_wait_setup() |
| acquiring an additional reference via the queue_lock() routine. The |
| nested key ref-counting has been masking bugs and complicating code |
| analysis. queue_lock() is only called with a previously ref-counted |
| key, so remove the additional ref-counting from the queue_(un)lock() |
| functions. |
| |
| Also futex_wait_requeue_pi() drops one key reference too many in |
| unqueue_me_pi(). Remove the key reference handling from |
| unqueue_me_pi(). This was paired with a queue_lock() in |
| futex_lock_pi(), so the count remains unchanged. |
| |
| Document remaining nested key ref-counting sites. |
| |
| Signed-off-by: Darren Hart <dvhart@linux.intel.com> |
| Reported-and-tested-by: Matthieu Fertré<matthieu.fertre@kerlabs.com> |
| Reported-by: Louis Rilling<louis.rilling@kerlabs.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Eric Dumazet <eric.dumazet@gmail.com> |
| Cc: John Kacur <jkacur@redhat.com> |
| Cc: Rusty Russell <rusty@rustcorp.com.au> |
| LKML-Reference: <4CBB17A8.70401@linux.intel.com> |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| kernel/futex.c | 31 ++++++++++++++++--------------- |
| 1 file changed, 16 insertions(+), 15 deletions(-) |
| |
| --- a/kernel/futex.c |
| +++ b/kernel/futex.c |
| @@ -1363,7 +1363,6 @@ static inline struct futex_hash_bucket * |
| { |
| struct futex_hash_bucket *hb; |
| |
| - get_futex_key_refs(&q->key); |
| hb = hash_futex(&q->key); |
| q->lock_ptr = &hb->lock; |
| |
| @@ -1375,7 +1374,6 @@ static inline void |
| queue_unlock(struct futex_q *q, struct futex_hash_bucket *hb) |
| { |
| spin_unlock(&hb->lock); |
| - drop_futex_key_refs(&q->key); |
| } |
| |
| /** |
| @@ -1480,8 +1478,6 @@ static void unqueue_me_pi(struct futex_q |
| q->pi_state = NULL; |
| |
| spin_unlock(q->lock_ptr); |
| - |
| - drop_futex_key_refs(&q->key); |
| } |
| |
| /* |
| @@ -1812,7 +1808,10 @@ static int futex_wait(u32 __user *uaddr, |
| } |
| |
| retry: |
| - /* Prepare to wait on uaddr. */ |
| + /* |
| + * Prepare to wait on uaddr. On success, holds hb lock and increments |
| + * q.key refs. |
| + */ |
| ret = futex_wait_setup(uaddr, val, fshared, &q, &hb); |
| if (ret) |
| goto out; |
| @@ -1822,24 +1821,23 @@ retry: |
| |
| /* If we were woken (and unqueued), we succeeded, whatever. */ |
| ret = 0; |
| + /* unqueue_me() drops q.key ref */ |
| if (!unqueue_me(&q)) |
| - goto out_put_key; |
| + goto out; |
| ret = -ETIMEDOUT; |
| if (to && !to->task) |
| - goto out_put_key; |
| + goto out; |
| |
| /* |
| * We expect signal_pending(current), but we might be the |
| * victim of a spurious wakeup as well. |
| */ |
| - if (!signal_pending(current)) { |
| - put_futex_key(fshared, &q.key); |
| + if (!signal_pending(current)) |
| goto retry; |
| - } |
| |
| ret = -ERESTARTSYS; |
| if (!abs_time) |
| - goto out_put_key; |
| + goto out; |
| |
| restart = ¤t_thread_info()->restart_block; |
| restart->fn = futex_wait_restart; |
| @@ -1856,8 +1854,6 @@ retry: |
| |
| ret = -ERESTART_RESTARTBLOCK; |
| |
| -out_put_key: |
| - put_futex_key(fshared, &q.key); |
| out: |
| if (to) { |
| hrtimer_cancel(&to->timer); |
| @@ -2236,7 +2232,10 @@ static int futex_wait_requeue_pi(u32 __u |
| q.rt_waiter = &rt_waiter; |
| q.requeue_pi_key = &key2; |
| |
| - /* Prepare to wait on uaddr. */ |
| + /* |
| + * Prepare to wait on uaddr. On success, increments q.key (key1) ref |
| + * count. |
| + */ |
| ret = futex_wait_setup(uaddr, val, fshared, &q, &hb); |
| if (ret) |
| goto out_key2; |
| @@ -2254,7 +2253,9 @@ static int futex_wait_requeue_pi(u32 __u |
| * In order for us to be here, we know our q.key == key2, and since |
| * we took the hb->lock above, we also know that futex_requeue() has |
| * completed and we no longer have to concern ourselves with a wakeup |
| - * race with the atomic proxy lock acquition by the requeue code. |
| + * race with the atomic proxy lock acquisition by the requeue code. The |
| + * futex_requeue dropped our key1 reference and incremented our key2 |
| + * reference count. |
| */ |
| |
| /* Check if the requeue code acquired the second futex for us. */ |