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. --- sysdeps/nptl/lowlevellock.h | 68 --------------------------------------------- 1 file changed, 68 deletions(-) (limited to 'sysdeps/nptl') diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h index 36a76fa..42d9658 100644 --- a/sysdeps/nptl/lowlevellock.h +++ b/sysdeps/nptl/lowlevellock.h @@ -74,7 +74,6 @@ extern void __lll_lock_wait_private (int *futex) attribute_hidden; extern void __lll_lock_wait (int *futex, int private) attribute_hidden; -extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden; /* This is an expression rather than a statement even though its value is void, so that it can be used in a comma expression or as an expression @@ -103,28 +102,6 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden; __lll_lock (&(futex), private) -/* If FUTEX is 0 (not acquired), set to ID (acquired with no waiters) and - return 0. Otherwise, ensure that it is set to FUTEX | FUTEX_WAITERS - (acquired, possibly with waiters) and block until we acquire the lock. - FUTEX will now be ID | FUTEX_WAITERS and we return 0. - If the previous owner of the lock dies before we acquire the lock then FUTEX - will be the value of id as set by the previous owner, with FUTEX_OWNER_DIED - set (FUTEX_WAITERS may or may not be set). We return this value to indicate - that the lock is not acquired. */ -#define __lll_robust_lock(futex, id, private) \ - ({ \ - int *__futex = (futex); \ - int __val = 0; \ - \ - if (__glibc_unlikely \ - (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \ - __val = __lll_robust_lock_wait (__futex, private); \ - __val; \ - }) -#define lll_robust_lock(futex, id, private) \ - __lll_robust_lock (&(futex), id, private) - - /* This is an expression rather than a statement even though its value is void, so that it can be used in a comma expression or as an expression that's cast to void. */ @@ -142,16 +119,8 @@ extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden; #define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private) -/* As __lll_robust_lock, but set to ID | FUTEX_WAITERS (acquired, possibly with - waiters) if FUTEX is 0. */ -#define lll_robust_cond_lock(futex, id, private) \ - __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private) - - extern int __lll_timedlock_wait (int *futex, const struct timespec *, int private) attribute_hidden; -extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, - int private) attribute_hidden; /* As __lll_lock, but with a timeout. If the timeout occurs then return @@ -170,22 +139,6 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, __lll_timedlock (&(futex), abstime, private) -/* As __lll_robust_lock, but with a timeout. If the timeout occurs then return - ETIMEDOUT. If ABSTIME is invalid, return EINVAL. */ -#define __lll_robust_timedlock(futex, abstime, id, private) \ - ({ \ - int *__futex = (futex); \ - int __val = 0; \ - \ - if (__glibc_unlikely \ - (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \ - __val = __lll_robust_timedlock_wait (__futex, abstime, private); \ - __val; \ - }) -#define lll_robust_timedlock(futex, abstime, id, private) \ - __lll_robust_timedlock (&(futex), abstime, id, private) - - /* This is an expression rather than a statement even though its value is void, so that it can be used in a comma expression or as an expression that's cast to void. */ @@ -211,27 +164,6 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, __lll_unlock (&(futex), private) -/* This is an expression rather than a statement even though its value is - void, so that it can be used in a comma expression or as an expression - that's cast to void. */ -/* Unconditionally set FUTEX to 0 (not acquired), releasing the lock. If FUTEX - had FUTEX_WAITERS set then wake any waiters. The waiter that acquires the - lock will set FUTEX_WAITERS. - Evaluate PRIVATE before releasing the lock so that we do not violate the - mutex destruction requirements (see __lll_unlock). */ -#define __lll_robust_unlock(futex, private) \ - ((void) \ - ({ \ - int *__futex = (futex); \ - int __private = (private); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__glibc_unlikely (__oldval & FUTEX_WAITERS)) \ - lll_futex_wake (__futex, 1, __private); \ - })) -#define lll_robust_unlock(futex, private) \ - __lll_robust_unlock (&(futex), private) - - #define lll_islocked(futex) \ ((futex) != LLL_LOCK_INITIALIZER) -- cgit v1.1