From 65810f0ef05e8c9e333f17a44e77808b163ca298 Mon Sep 17 00:00:00 2001 From: Torvald Riegel Date: Thu, 22 Dec 2016 10:20:43 +0100 Subject: robust mutexes: Fix broken x86 assembly by removing it lll_robust_unlock on i386 and x86_64 first sets the futex word to FUTEX_WAITERS|0 before calling __lll_unlock_wake, which will set the futex word to 0. If the thread is killed between these steps, then the futex word will be FUTEX_WAITERS|0, and the kernel (at least current upstream) will not set it to FUTEX_OWNER_DIED|FUTEX_WAITERS because 0 is not equal to the TID of the crashed thread. The lll_robust_lock assembly code on i386 and x86_64 is not prepared to deal with this case because the fastpath tries to only CAS 0 to TID and not FUTEX_WAITERS|0 to TID; the slowpath simply waits until it can CAS 0 to TID or the futex_word has the FUTEX_OWNER_DIED bit set. This issue is fixed by removing the custom x86 assembly code and using the generic C code instead. However, instead of adding more duplicate code to the custom x86 lowlevellock.h, the code of the lll_robust* functions is inlined into the single call sites that exist for each of these functions in the pthread_mutex_* functions. The robust mutex paths in the latter have been slightly reorganized to make them simpler. This patch is meant to be easy to backport, so C11-style atomics are not used. [BZ #20985] * nptl/Makefile: Adapt. * nptl/pthread_mutex_cond_lock.c (LLL_ROBUST_MUTEX_LOCK): Remove. (LLL_ROBUST_MUTEX_LOCK_MODIFIER): New. * nptl/pthread_mutex_lock.c (LLL_ROBUST_MUTEX_LOCK): Remove. (LLL_ROBUST_MUTEX_LOCK_MODIFIER): New. (__pthread_mutex_lock_full): Inline lll_robust* functions and adapt. * nptl/pthread_mutex_timedlock.c (pthread_mutex_timedlock): Inline lll_robust* functions and adapt. * nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_full): Likewise. * sysdeps/nptl/lowlevellock.h (__lll_robust_lock_wait, __lll_robust_lock, lll_robust_cond_lock, __lll_robust_timedlock_wait, __lll_robust_timedlock, __lll_robust_unlock): Remove. * sysdeps/unix/sysv/linux/i386/lowlevellock.h (lll_robust_lock, lll_robust_cond_lock, lll_robust_timedlock, lll_robust_unlock): Remove. * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_robust_lock, lll_robust_cond_lock, lll_robust_timedlock, lll_robust_unlock): Remove. * sysdeps/unix/sysv/linux/sparc/lowlevellock.h (__lll_robust_lock_wait, __lll_robust_lock, lll_robust_cond_lock, __lll_robust_timedlock_wait, __lll_robust_timedlock, __lll_robust_unlock): Remove. * nptl/lowlevelrobustlock.c: Remove file. * nptl/lowlevelrobustlock.sym: Likewise. * sysdeps/unix/sysv/linux/i386/lowlevelrobustlock.S: Likewise. * sysdeps/unix/sysv/linux/x86_64/lowlevelrobustlock.S: Likewise. --- nptl/pthread_mutex_unlock.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'nptl/pthread_mutex_unlock.c') diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c index 130883c4bb..d3b39900bc 100644 --- a/nptl/pthread_mutex_unlock.c +++ b/nptl/pthread_mutex_unlock.c @@ -96,6 +96,7 @@ internal_function __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr) { int newowner = 0; + int private; switch (PTHREAD_MUTEX_TYPE (mutex)) { @@ -149,9 +150,14 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr) /* One less user. */ --mutex->__data.__nusers; - /* Unlock. */ - lll_robust_unlock (mutex->__data.__lock, - PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); + /* Unlock by setting the lock to 0 (not acquired); if the lock had + FUTEX_WAITERS set previously, then wake any waiters. + The unlock operation must be the last access to the mutex to not + violate the mutex destruction requirements (see __lll_unlock). */ + private = PTHREAD_ROBUST_MUTEX_PSHARED (mutex); + if (__glibc_unlikely ((atomic_exchange_rel (&mutex->__data.__lock, 0) + & FUTEX_WAITERS) != 0)) + lll_futex_wake (&mutex->__data.__lock, 1, private); THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); break; @@ -234,9 +240,9 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr) to not violate the mutex destruction requirements (see lll_unlock). */ int robust = mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP; - int private = (robust - ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex) - : PTHREAD_MUTEX_PSHARED (mutex)); + private = (robust + ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex) + : PTHREAD_MUTEX_PSHARED (mutex)); /* Unlock the mutex using a CAS unless there are futex waiters or our TID is not the value of __lock anymore, in which case we let the kernel take care of the situation. Use release MO in the CAS to -- cgit v1.2.3