Skip to content

Commit

Permalink
Fix undefined behavior when re-locking a mutex from the same thread
Browse files Browse the repository at this point in the history
The only applies to pthread mutexes. We solve this by creating the
mutex with the PTHREAD_MUTEX_NORMAL type, which guarantees that
re-locking from the same thread will deadlock.
  • Loading branch information
Amanieu committed Jun 2, 2016
1 parent eea4f0c commit d73f5e6
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/libstd/sync/mutex.rs
Expand Up @@ -190,10 +190,14 @@ impl<T> Mutex<T> {
/// Creates a new mutex in an unlocked state ready for use.
#[stable(feature = "rust1", since = "1.0.0")]
pub fn new(t: T) -> Mutex<T> {
Mutex {
let mut m = Mutex {
inner: box StaticMutex::new(),
data: UnsafeCell::new(t),
};
unsafe {
m.inner.lock.init();
}
m
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/libstd/sys/common/mutex.rs
Expand Up @@ -27,6 +27,12 @@ impl Mutex {
/// first used with any of the functions below.
pub const fn new() -> Mutex { Mutex(imp::Mutex::new()) }

/// Prepare the mutex for use.
///
/// This should be called once the mutex is at a stable memory address.
#[inline]
pub unsafe fn init(&mut self) { self.0.init() }

/// Locks the mutex blocking the current thread until it is available.
///
/// Behavior is undefined if the mutex has been moved between this and any
Expand Down
33 changes: 33 additions & 0 deletions src/libstd/sys/unix/mutex.rs
Expand Up @@ -30,6 +30,39 @@ impl Mutex {
Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
}
#[inline]
pub unsafe fn init(&mut self) {
// Issue #33770
//
// A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have
// a type of PTHREAD_MUTEX_DEFAULT, which has undefined behavior if you
// try to re-lock it from the same thread when you already hold a lock.
//
// In practice, glibc takes advantage of this undefined behavior to
// implement hardware lock elision, which uses hardware transactional
// memory to avoid acquiring the lock. While a transaction is in
// progress, the lock appears to be unlocked. This isn't a problem for
// other threads since the transactional memory will abort if a conflict
// is detected, however no abort is generated if re-locking from the
// same thread.
//
// Since locking the same mutex twice will result in two aliasing &mut
// references, we instead create the mutex with type
// PTHREAD_MUTEX_NORMAL which is guaranteed to deadlock if we try to
// re-lock it from the same thread, thus avoiding undefined behavior.
//
// We can't do anything for StaticMutex, but that type is deprecated
// anyways.
let mut attr: libc::pthread_mutexattr_t = mem::uninitialized();
let r = libc::pthread_mutexattr_init(&mut attr);
debug_assert_eq!(r, 0);
let r = libc::pthread_mutexattr_settype(&mut attr, libc::PTHREAD_MUTEX_NORMAL);
debug_assert_eq!(r, 0);
let r = libc::pthread_mutex_init(self.inner.get(), &attr);
debug_assert_eq!(r, 0);
let r = libc::pthread_mutexattr_destroy(&mut attr);
debug_assert_eq!(r, 0);
}
#[inline]
pub unsafe fn lock(&self) {
let r = libc::pthread_mutex_lock(self.inner.get());
debug_assert_eq!(r, 0);
Expand Down
2 changes: 2 additions & 0 deletions src/libstd/sys/windows/mutex.rs
Expand Up @@ -64,6 +64,8 @@ impl Mutex {
held: UnsafeCell::new(false),
}
}
#[inline]
pub unsafe fn init(&mut self) {}
pub unsafe fn lock(&self) {
match kind() {
Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)),
Expand Down

0 comments on commit d73f5e6

Please sign in to comment.