Messages in this thread |  | | Date | Sun, 9 Oct 2016 13:17:50 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [RFC][PATCH 4/4] futex: Rewrite FUTEX_UNLOCK_PI |
| |
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
|  |