Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4137)

Unified Diff: base/synchronization/lock_impl_posix.cc

Issue 2178503003: Add PTHREAD_PRIO_INHERIT Locks for Mac (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Adjust Macro Location Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/synchronization/lock_impl.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/synchronization/lock_impl_posix.cc
diff --git a/base/synchronization/lock_impl_posix.cc b/base/synchronization/lock_impl_posix.cc
index 5619adaf5d829df72f93fd57b0687d551fb9390d..7024ab2b2190aeef8c214508e33906da2f6de4fe 100644
--- a/base/synchronization/lock_impl_posix.cc
+++ b/base/synchronization/lock_impl_posix.cc
@@ -8,26 +8,43 @@
#include <string.h>
#include "base/logging.h"
+#include "base/synchronization/lock.h"
namespace base {
namespace internal {
+// Determines which platforms can consider using priority inheritance locks. Use
+// this define for platform code that may not compile if priority inheritance
+// locks aren't available. For this platform code,
+// PRIORITY_INHERITANCE_LOCKS_POSSIBLE() is a necessary but insufficient check.
+// Lock::PriorityInheritanceAvailable still must be checked as the code may
+// compile but the underlying platform still may not correctly support priority
+// inheritance locks.
+#if defined(OS_NACL) || defined(OS_ANDROID)
+#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 0
+#else
+#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 1
+#endif
+
LockImpl::LockImpl() {
-#ifndef NDEBUG
- // In debug, setup attributes for lock error checking.
pthread_mutexattr_t mta;
int rv = pthread_mutexattr_init(&mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
+#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE()
+ if (PriorityInheritanceAvailable()) {
+ rv = pthread_mutexattr_setprotocol(&mta, PTHREAD_PRIO_INHERIT);
+ DCHECK_EQ(rv, 0) << ". " << strerror(rv);
+ }
+#endif
+#ifndef NDEBUG
+ // In debug, setup attributes for lock error checking.
rv = pthread_mutexattr_settype(&mta, PTHREAD_MUTEX_ERRORCHECK);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
+#endif
rv = pthread_mutex_init(&native_handle_, &mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
rv = pthread_mutexattr_destroy(&mta);
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
-#else
- // In release, go with the default lock attributes.
- pthread_mutex_init(&native_handle_, NULL);
-#endif
}
LockImpl::~LockImpl() {
@@ -51,5 +68,29 @@ void LockImpl::Unlock() {
DCHECK_EQ(rv, 0) << ". " << strerror(rv);
}
+// static
+bool LockImpl::PriorityInheritanceAvailable() {
+#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE() && defined(OS_MACOSX)
+ return true;
+#else
+ // Security concerns prevent the use of priority inheritance mutexes on Linux.
+ // * CVE-2010-0622 - wake_futex_pi unlocks incorrect, possible DoS.
+ // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-0622
+ // * CVE-2012-6647 - Linux < 3.5.1, futex_wait_requeue_pi possible DoS.
+ // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2012-6647
+ // * CVE-2014-3153 - Linux <= 3.14.5, futex_requeue, privilege escalation.
+ // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-3153
+ //
+ // If the above were all addressed, we still need a runtime check to deal with
+ // the bug below.
+ // * glibc Bug 14652: https://sourceware.org/bugzilla/show_bug.cgi?id=14652
+ // Fixed in glibc 2.17.
+ // Priority inheritance mutexes may deadlock with condition variables
+ // during recacquisition of the mutex after the condition variable is
+ // signalled.
+ return false;
+#endif
+}
+
} // namespace internal
} // namespace base
« no previous file with comments | « base/synchronization/lock_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698