Comment about lock destruction and unlocking
Add a couple of comments explaining that a mutex can be freed during
the unlock call, immediately after the unlock's atomic exchange call but
before its futex wakeup call.
Bug: http://b/129744706
Test: bionic unit tests
Change-Id: I2d290ebde880f46866098d022720896039e7022e
diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp
index d9ddf10..f03e55b 100644
--- a/libc/bionic/pthread_mutex.cpp
+++ b/libc/bionic/pthread_mutex.cpp
@@ -644,6 +644,15 @@
// we call wake, the thread we eventually wake will find an unlocked mutex
// and will execute. Either way we have correct behavior and nobody is
// orphaned on the wait queue.
+ //
+ // The pthread_mutex_internal_t object may have been deallocated between the
+ // atomic exchange and the wake call. In that case, this wake call could
+ // target unmapped memory or memory used by an otherwise unrelated futex
+ // operation. Even if the kernel avoids spurious futex wakeups from its
+ // point of view, this wake call could trigger a spurious wakeup in any
+ // futex accessible from this process. References:
+ // - https://lkml.org/lkml/2014/11/27/472
+ // - http://austingroupbugs.net/view.php?id=811#c2267
__futex_wake_ex(&mutex->state, shared, 1);
}
}
diff --git a/libc/private/bionic_lock.h b/libc/private/bionic_lock.h
index ec179d1..d70ba6c 100644
--- a/libc/private/bionic_lock.h
+++ b/libc/private/bionic_lock.h
@@ -72,6 +72,12 @@
void unlock() {
bool shared = process_shared; /* cache to local variable */
if (atomic_exchange_explicit(&state, Unlocked, memory_order_release) == LockedWithWaiter) {
+ // The Lock object may have been deallocated between the atomic exchange and the futex wake
+ // call, so avoid accessing any fields of Lock here. In that case, the wake call may target
+ // unmapped memory or trigger a spurious futex wakeup. The same situation happens with
+ // pthread mutexes. References:
+ // - https://lkml.org/lkml/2014/11/27/472
+ // - http://austingroupbugs.net/view.php?id=811#c2267
__futex_wake_ex(&state, shared, 1);
}
}