aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyan Prichard <rprichard@google.com>2019-04-18 22:47:04 -0700
committerRyan Prichard <rprichard@google.com>2019-04-23 00:19:30 -0700
commitea722a077963cff958d2dcd58f9ff4075a69df24 (patch)
tree3144569590c0edda0e342c3955dd2c1e0fa37cd6
parent5bab966ca807b95ca8be32a71cc7cecc36b62106 (diff)
downloadandroid_bionic-ea722a077963cff958d2dcd58f9ff4075a69df24.tar.gz
android_bionic-ea722a077963cff958d2dcd58f9ff4075a69df24.tar.bz2
android_bionic-ea722a077963cff958d2dcd58f9ff4075a69df24.zip
PIMutexUnlock: load owner_tid in non-common case
For a recursive or errorcheck PI mutex, the old_owner variable wasn't being initialized. As a result, unlocking a doubly-locked recursive mutex owned by another thread decremented the mutex counter. Instead, the unlock call should fail with EPERM. Bug: http://b/130841532 Test: bionic-unit-tests Test: bionic-unit-tests-glibc --gtest_filter='pthread.pthread_mutex_lock*' Change-Id: I37adb094cb2ce8d51df7b4f48e8d6bc144436418 (cherry picked from commit 4b6c0f5dce5ad8d93e4e707977e09153a5399139)
-rw-r--r--libc/bionic/pthread_mutex.cpp2
-rw-r--r--tests/pthread_test.cpp18
2 files changed, 20 insertions, 0 deletions
diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp
index d9ddf1058..f92184e50 100644
--- a/libc/bionic/pthread_mutex.cpp
+++ b/libc/bionic/pthread_mutex.cpp
@@ -199,6 +199,8 @@ static int PIMutexUnlock(PIMutex& mutex) {
memory_order_relaxed))) {
return 0;
}
+ } else {
+ old_owner = atomic_load_explicit(&mutex.owner_tid, memory_order_relaxed);
}
if (tid != (old_owner & FUTEX_TID_MASK)) {
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 7b64401db..0bf8e294b 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -1843,10 +1843,25 @@ struct PthreadMutex {
DISALLOW_COPY_AND_ASSIGN(PthreadMutex);
};
+static int UnlockFromAnotherThread(pthread_mutex_t* mutex) {
+ pthread_t thread;
+ pthread_create(&thread, nullptr, [](void* mutex_voidp) -> void* {
+ pthread_mutex_t* mutex = static_cast<pthread_mutex_t*>(mutex_voidp);
+ intptr_t result = pthread_mutex_unlock(mutex);
+ return reinterpret_cast<void*>(result);
+ }, mutex);
+ void* result;
+ EXPECT_EQ(0, pthread_join(thread, &result));
+ return reinterpret_cast<intptr_t>(result);
+};
+
static void TestPthreadMutexLockNormal(int protocol) {
PthreadMutex m(PTHREAD_MUTEX_NORMAL, protocol);
ASSERT_EQ(0, pthread_mutex_lock(&m.lock));
+ if (protocol == PTHREAD_PRIO_INHERIT) {
+ ASSERT_EQ(EPERM, UnlockFromAnotherThread(&m.lock));
+ }
ASSERT_EQ(0, pthread_mutex_unlock(&m.lock));
ASSERT_EQ(0, pthread_mutex_trylock(&m.lock));
ASSERT_EQ(EBUSY, pthread_mutex_trylock(&m.lock));
@@ -1857,6 +1872,7 @@ static void TestPthreadMutexLockErrorCheck(int protocol) {
PthreadMutex m(PTHREAD_MUTEX_ERRORCHECK, protocol);
ASSERT_EQ(0, pthread_mutex_lock(&m.lock));
+ ASSERT_EQ(EPERM, UnlockFromAnotherThread(&m.lock));
ASSERT_EQ(EDEADLK, pthread_mutex_lock(&m.lock));
ASSERT_EQ(0, pthread_mutex_unlock(&m.lock));
ASSERT_EQ(0, pthread_mutex_trylock(&m.lock));
@@ -1873,7 +1889,9 @@ static void TestPthreadMutexLockRecursive(int protocol) {
PthreadMutex m(PTHREAD_MUTEX_RECURSIVE, protocol);
ASSERT_EQ(0, pthread_mutex_lock(&m.lock));
+ ASSERT_EQ(EPERM, UnlockFromAnotherThread(&m.lock));
ASSERT_EQ(0, pthread_mutex_lock(&m.lock));
+ ASSERT_EQ(EPERM, UnlockFromAnotherThread(&m.lock));
ASSERT_EQ(0, pthread_mutex_unlock(&m.lock));
ASSERT_EQ(0, pthread_mutex_unlock(&m.lock));
ASSERT_EQ(0, pthread_mutex_trylock(&m.lock));