futex: Simplify futex_lock_pi_atomic() and make it more robust

futex_lock_pi_atomic() is a maze of retry hoops and loops.

Reduce it to simple and understandable states:

First step is to lookup existing waiters (state) in the kernel.

If there is an existing waiter, validate it and attach to it.

If there is no existing waiter, check the user space value

If the TID encoded in the user space value is 0, take over the futex
preserving the owner died bit.

If the TID encoded in the user space value is != 0, lookup the owner
task, validate it and attach to it.

Reduces text size by 128 bytes on x8664.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Davidlohr Bueso <davidlohr@hp.com>
Cc: Kees Cook <kees@outflux.net>
Cc: wad@chromium.org
Cc: Darren Hart <darren@dvhart.com>
Link: http://lkml.kernel.org/r/alpine.DEB.2.10.1406131137020.5170@nanos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
This commit is contained in:
Thomas Gleixner 2014-06-11 20:45:41 +00:00
parent 04e1b2e52b
commit af54d6a1c3
1 changed files with 64 additions and 90 deletions

View File

@ -956,6 +956,17 @@ static int lookup_pi_state(u32 uval, struct futex_hash_bucket *hb,
return attach_to_pi_owner(uval, key, ps); return attach_to_pi_owner(uval, key, ps);
} }
static int lock_pi_update_atomic(u32 __user *uaddr, u32 uval, u32 newval)
{
u32 uninitialized_var(curval);
if (unlikely(cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)))
return -EFAULT;
/*If user space value changed, let the caller retry */
return curval != uval ? -EAGAIN : 0;
}
/** /**
* futex_lock_pi_atomic() - Atomic work required to acquire a pi aware futex * futex_lock_pi_atomic() - Atomic work required to acquire a pi aware futex
* @uaddr: the pi futex user address * @uaddr: the pi futex user address
@ -979,113 +990,69 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb,
struct futex_pi_state **ps, struct futex_pi_state **ps,
struct task_struct *task, int set_waiters) struct task_struct *task, int set_waiters)
{ {
int lock_taken, ret, force_take = 0; u32 uval, newval, vpid = task_pid_vnr(task);
u32 uval, newval, curval, vpid = task_pid_vnr(task); struct futex_q *match;
int ret;
retry:
ret = lock_taken = 0;
/* /*
* To avoid races, we attempt to take the lock here again * Read the user space value first so we can validate a few
* (by doing a 0 -> TID atomic cmpxchg), while holding all * things before proceeding further.
* the locks. It will most likely not succeed.
*/ */
newval = vpid; if (get_futex_value_locked(&uval, uaddr))
if (set_waiters)
newval |= FUTEX_WAITERS;
if (unlikely(cmpxchg_futex_value_locked(&curval, uaddr, 0, newval)))
return -EFAULT; return -EFAULT;
/* /*
* Detect deadlocks. * Detect deadlocks.
*/ */
if ((unlikely((curval & FUTEX_TID_MASK) == vpid))) if ((unlikely((uval & FUTEX_TID_MASK) == vpid)))
return -EDEADLK; return -EDEADLK;
/* /*
* Surprise - we got the lock, but we do not trust user space at all. * Lookup existing state first. If it exists, try to attach to
* its pi_state.
*/ */
if (unlikely(!curval)) { match = futex_top_waiter(hb, key);
if (match)
return attach_to_pi_state(uval, match->pi_state, ps);
/*
* No waiter and user TID is 0. We are here because the
* waiters or the owner died bit is set or called from
* requeue_cmp_pi or for whatever reason something took the
* syscall.
*/
if (!(uval & FUTEX_TID_MASK)) {
/* /*
* We verify whether there is kernel state for this * We take over the futex. No other waiters and the user space
* futex. If not, we can safely assume, that the 0 -> * TID is 0. We preserve the owner died bit.
* TID transition is correct. If state exists, we do
* not bother to fixup the user space state as it was
* corrupted already.
*/ */
return futex_top_waiter(hb, key) ? -EINVAL : 1; newval = uval & FUTEX_OWNER_DIED;
newval |= vpid;
/* The futex requeue_pi code can enforce the waiters bit */
if (set_waiters)
newval |= FUTEX_WAITERS;
ret = lock_pi_update_atomic(uaddr, uval, newval);
/* If the take over worked, return 1 */
return ret < 0 ? ret : 1;
} }
uval = curval;
/* /*
* Set the FUTEX_WAITERS flag, so the owner will know it has someone * First waiter. Set the waiters bit before attaching ourself to
* to wake at the next unlock. * the owner. If owner tries to unlock, it will be forced into
* the kernel and blocked on hb->lock.
*/ */
newval = curval | FUTEX_WAITERS; newval = uval | FUTEX_WAITERS;
ret = lock_pi_update_atomic(uaddr, uval, newval);
if (ret)
return ret;
/* /*
* Should we force take the futex? See below. * If the update of the user space value succeeded, we try to
* attach to the owner. If that fails, no harm done, we only
* set the FUTEX_WAITERS bit in the user space variable.
*/ */
if (unlikely(force_take)) { return attach_to_pi_owner(uval, key, ps);
/*
* Keep the OWNER_DIED and the WAITERS bit and set the
* new TID value.
*/
newval = (curval & ~FUTEX_TID_MASK) | vpid;
force_take = 0;
lock_taken = 1;
}
if (unlikely(cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)))
return -EFAULT;
if (unlikely(curval != uval))
goto retry;
/*
* We took the lock due to forced take over.
*/
if (unlikely(lock_taken))
return 1;
/*
* We dont have the lock. Look up the PI state (or create it if
* we are the first waiter):
*/
ret = lookup_pi_state(uval, hb, key, ps);
if (unlikely(ret)) {
switch (ret) {
case -ESRCH:
/*
* We failed to find an owner for this
* futex. So we have no pi_state to block
* on. This can happen in two cases:
*
* 1) The owner died
* 2) A stale FUTEX_WAITERS bit
*
* Re-read the futex value.
*/
if (get_futex_value_locked(&curval, uaddr))
return -EFAULT;
/*
* If the owner died or we have a stale
* WAITERS bit the owner TID in the user space
* futex is 0.
*/
if (!(curval & FUTEX_TID_MASK)) {
force_take = 1;
goto retry;
}
default:
break;
}
}
return ret;
} }
/** /**
@ -1659,7 +1626,12 @@ retry_private:
goto retry; goto retry;
goto out; goto out;
case -EAGAIN: case -EAGAIN:
/* The owner was exiting, try again. */ /*
* Two reasons for this:
* - Owner is exiting and we just wait for the
* exit to complete.
* - The user space value changed.
*/
double_unlock_hb(hb1, hb2); double_unlock_hb(hb1, hb2);
hb_waiters_dec(hb2); hb_waiters_dec(hb2);
put_futex_key(&key2); put_futex_key(&key2);
@ -2316,8 +2288,10 @@ retry_private:
goto uaddr_faulted; goto uaddr_faulted;
case -EAGAIN: case -EAGAIN:
/* /*
* Task is exiting and we just wait for the * Two reasons for this:
* exit to complete. * - Task is exiting and we just wait for the
* exit to complete.
* - The user space value changed.
*/ */
queue_unlock(hb); queue_unlock(hb);
put_futex_key(&q.key); put_futex_key(&q.key);