lkml.org 
[lkml]   [2010]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[07/45] futex: Fix errors in nested key ref-counting
    2.6.32-stable review patch.  If anyone has any objections, please let us know.

    ------------------

    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 = &current_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. */

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-11-19 22:57    [W:0.029 / U:59.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site