diff options
author | Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> | 2017-01-23 14:39:47 -0200 |
---|---|---|
committer | Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> | 2017-01-23 14:40:10 -0200 |
commit | da9f68f2b9f653a49d0d0c6148d4649dc9ad7fd6 (patch) | |
tree | 3dae2f4cd6d986f3877d0ec9ca31c33ea4c8a7f0 | |
parent | e9d53ff4d46e27d321161a2fb8f16820ea2cb68e (diff) | |
download | glibc-da9f68f2b9f653a49d0d0c6148d4649dc9ad7fd6.tar.gz glibc-da9f68f2b9f653a49d0d0c6148d4649dc9ad7fd6.tar.xz glibc-da9f68f2b9f653a49d0d0c6148d4649dc9ad7fd6.zip |
powerpc: Fix write-after-destroy in lock elision [BZ #20822]
The update of *adapt_count after the release of the lock causes a race condition when thread A unlocks, thread B continues and destroys the mutex, and thread A writes to *adapt_count. (cherry picked from commit e9a96ea1aca4ebaa7c86e8b83b766f118d689d0f) (with changes from commit eb1321f291515dae75c83a40c39e775fdd38e97a)
-rw-r--r-- | ChangeLog | 13 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/powerpc/elision-lock.c | 10 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/powerpc/elision-trylock.c | 7 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/powerpc/elision-unlock.c | 15 |
4 files changed, 33 insertions, 12 deletions
diff --git a/ChangeLog b/ChangeLog index 5a49870eb1..ae1c767692 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2017-01-23 Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com> + Steven Munroe <sjmunroe@us.ibm.com> + Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> + + [BZ #20822] + * sysdeps/unix/sysv/linux/powerpc/elision-lock.c + (__lll_lock_elision): Access adapt_count via C11 atomics. + * sysdeps/unix/sysv/linux/powerpc/elision-trylock.c + (__lll_trylock_elision): Likewise. + * sysdeps/unix/sysv/linux/powerpc/elision-unlock.c + (__lll_unlock_elision): Update adapt_count variable inside the + critical section using C11 atomics. + 2016-08-02 Aurelien Jarno <aurelien@aurel32.net> 2016-08-02 Aurelien Jarno <aurelien@aurel32.net> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c index dd1e4c3b17..7dd3d835b6 100644 --- a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c +++ b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c @@ -45,7 +45,9 @@ int __lll_lock_elision (int *lock, short *adapt_count, EXTRAARG int pshared) { - if (*adapt_count > 0) + /* adapt_count is accessed concurrently but is just a hint. Thus, + use atomic accesses but relaxed MO is sufficient. */ + if (atomic_load_relaxed (adapt_count) > 0) { goto use_lock; } @@ -67,7 +69,8 @@ __lll_lock_elision (int *lock, short *adapt_count, EXTRAARG int pshared) if (_TEXASRU_FAILURE_PERSISTENT (__builtin_get_texasru ())) { if (aconf.skip_lock_internal_abort > 0) - *adapt_count = aconf.skip_lock_internal_abort; + atomic_store_relaxed (adapt_count, + aconf.skip_lock_internal_abort); goto use_lock; } } @@ -75,7 +78,8 @@ __lll_lock_elision (int *lock, short *adapt_count, EXTRAARG int pshared) /* Fall back to locks for a bit if retries have been exhausted */ if (aconf.try_tbegin > 0 && aconf.skip_lock_out_of_tbegin_retries > 0) - *adapt_count = aconf.skip_lock_out_of_tbegin_retries; + atomic_store_relaxed (adapt_count, + aconf.skip_lock_out_of_tbegin_retries); use_lock: return LLL_LOCK ((*lock), pshared); diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c index 0807a6a432..606185670d 100644 --- a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c +++ b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c @@ -34,7 +34,7 @@ __lll_trylock_elision (int *futex, short *adapt_count) __libc_tabort (_ABORT_NESTED_TRYLOCK); /* Only try a transaction if it's worth it. */ - if (*adapt_count > 0) + if (atomic_load_relaxed (adapt_count) > 0) { goto use_lock; } @@ -49,7 +49,7 @@ __lll_trylock_elision (int *futex, short *adapt_count) __libc_tend (0); if (aconf.skip_lock_busy > 0) - *adapt_count = aconf.skip_lock_busy; + atomic_store_relaxed (adapt_count, aconf.skip_lock_busy); } else { @@ -59,7 +59,8 @@ __lll_trylock_elision (int *futex, short *adapt_count) result in another failure. Use normal locking now and for the next couple of calls. */ if (aconf.skip_trylock_internal_abort > 0) - *adapt_count = aconf.skip_trylock_internal_abort; + atomic_store_relaxed (adapt_count, + aconf.skip_trylock_internal_abort); } } diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c b/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c index 43c5a67df2..51d7018e4c 100644 --- a/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c +++ b/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c @@ -28,13 +28,16 @@ __lll_unlock_elision (int *lock, short *adapt_count, int pshared) __libc_tend (0); else { - lll_unlock ((*lock), pshared); + /* Update adapt_count in the critical section to prevent a + write-after-destroy error as mentioned in BZ 20822. The + following update of adapt_count has to be contained within + the critical region of the fall-back lock in order to not violate + the mutex destruction requirements. */ + short __tmp = atomic_load_relaxed (adapt_count); + if (__tmp > 0) + atomic_store_relaxed (adapt_count, __tmp - 1); - /* Update the adapt count AFTER completing the critical section. - Doing this here prevents unneeded stalling when entering - a critical section. Saving about 8% runtime on P8. */ - if (*adapt_count > 0) - (*adapt_count)--; + lll_unlock ((*lock), pshared); } return 0; } |