On Fri, 7 Oct 2016, Peter Zijlstra wrote:
> top_waiter = futex_top_waiter(hb, &key);
> if (top_waiter) {
> - ret = wake_futex_pi(uaddr, uval, top_waiter, hb);
> + struct futex_pi_state *pi_state = top_waiter->pi_state;
> +
> + ret = -EINVAL;
> + if (!pi_state)
> + goto out_unlock;
> +
> + /*
> + * If current does not own the pi_state then the futex is
> + * inconsistent and user space fiddled with the futex value.
> + */
> + if (pi_state->owner != current)
> + goto out_unlock;
> +
> + /*
> + * Grab a reference on the pi_state and drop hb->lock.
> + *
> + * The reference ensures pi_state lives, dropping the hb->lock
> + * is tricky.. wake_futex_pi() will take rt_mutex::wait_lock to
> + * close the races against futex_lock_pi(), but in case of
> + * _any_ fail we'll abort and retry the whole deal.
> + */
> + WARN_ON_ONCE(!atomic_inc_not_zero(&pi_state->refcount));
> + spin_unlock(&hb->lock);
> +
> + ret = wake_futex_pi(uaddr, uval, pi_state);
> +
> + put_pi_state(pi_state);
put_pi_state() requires hb->lock protection AFAICT.
CPU0 CPU1
wake_futex_pi() attach_to_pi_state()
put_pi_state()
refcount--;
if (!refcount)
free_state();
WARN_ON(!pi_state->refcount);
we might not see the warning, but in any case the following access to
pi_state on cpu1 is borked.
Thanks,
tglx
