Commit 7ada876a authored by Darren Hart's avatar Darren Hart Committed by Thomas Gleixner
Browse files

futex: Fix errors in nested key ref-counting 



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: default avatarDarren Hart <dvhart@linux.intel.com>
Reported-and-tested-by: default avatarMatthieu <Fertré&lt;matthieu.fertre@kerlabs.com>
Reported-by: default avatarLouis <Rilling&lt;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: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: stable@kernel.org
parent 2b666ca4

kernel/futex.c

+16 −15
Original line number Diff line number Diff line
@@ -1363,7 +1363,6 @@ static inline struct futex_hash_bucket *queue_lock(struct futex_q *q) 
{

	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) 
	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, int fshared, 
	}



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 __user *uaddr, int fshared, 
	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 __user *uaddr, int fshared, 
	 * 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. */

CRA Git | Maintained and supported by SUSTech CRA and CCSE