From 21c29b1e954d494026b07241aa8365837d5ecfdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 20 Oct 2020 00:00:00 +0000 Subject: [PATCH] Check that pthread mutex initialization succeeded If pthread mutex initialization fails, the failure will go unnoticed unless debug assertions are enabled. Any subsequent use of mutex will also silently fail, since return values from lock & unlock operations are similarly checked only through debug assertions. In some implementations the mutex initialization requires a memory allocation and so it does fail in practice. Check that initialization succeeds to ensure that mutex guarantees mutual exclusion. --- library/std/src/sys/unix/mod.rs | 4 ++ library/std/src/sys/unix/mutex.rs | 39 +++++++++++-------- .../std/src/sys/unix/process/process_unix.rs | 6 +-- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index 776f4f18ecfa2..b28c6d85b7c72 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -220,6 +220,10 @@ where } } +pub fn cvt_nz(error: libc::c_int) -> crate::io::Result<()> { + if error == 0 { Ok(()) } else { Err(crate::io::Error::from_raw_os_error(error)) } +} + // On Unix-like platforms, libc::abort will unregister signal handlers // including the SIGABRT handler, preventing the abort from being blocked, and // fclose streams, with the side effect of flushing them so libc buffered diff --git a/library/std/src/sys/unix/mutex.rs b/library/std/src/sys/unix/mutex.rs index ebc737b75ae0b..89c55eb859d09 100644 --- a/library/std/src/sys/unix/mutex.rs +++ b/library/std/src/sys/unix/mutex.rs @@ -1,5 +1,6 @@ use crate::cell::UnsafeCell; use crate::mem::MaybeUninit; +use crate::sys::cvt_nz; pub struct Mutex { inner: UnsafeCell, @@ -51,14 +52,11 @@ impl Mutex { // PTHREAD_MUTEX_NORMAL which is guaranteed to deadlock if we try to // re-lock it from the same thread, thus avoiding undefined behavior. let mut attr = MaybeUninit::::uninit(); - let r = libc::pthread_mutexattr_init(attr.as_mut_ptr()); - debug_assert_eq!(r, 0); - let r = libc::pthread_mutexattr_settype(attr.as_mut_ptr(), libc::PTHREAD_MUTEX_NORMAL); - debug_assert_eq!(r, 0); - let r = libc::pthread_mutex_init(self.inner.get(), attr.as_ptr()); - debug_assert_eq!(r, 0); - let r = libc::pthread_mutexattr_destroy(attr.as_mut_ptr()); - debug_assert_eq!(r, 0); + cvt_nz(libc::pthread_mutexattr_init(attr.as_mut_ptr())).unwrap(); + let attr = PthreadMutexAttr(&mut attr); + cvt_nz(libc::pthread_mutexattr_settype(attr.0.as_mut_ptr(), libc::PTHREAD_MUTEX_NORMAL)) + .unwrap(); + cvt_nz(libc::pthread_mutex_init(self.inner.get(), attr.0.as_ptr())).unwrap(); } #[inline] pub unsafe fn lock(&self) { @@ -106,15 +104,11 @@ impl ReentrantMutex { pub unsafe fn init(&self) { let mut attr = MaybeUninit::::uninit(); - let result = libc::pthread_mutexattr_init(attr.as_mut_ptr()); - debug_assert_eq!(result, 0); - let result = - libc::pthread_mutexattr_settype(attr.as_mut_ptr(), libc::PTHREAD_MUTEX_RECURSIVE); - debug_assert_eq!(result, 0); - let result = libc::pthread_mutex_init(self.inner.get(), attr.as_ptr()); - debug_assert_eq!(result, 0); - let result = libc::pthread_mutexattr_destroy(attr.as_mut_ptr()); - debug_assert_eq!(result, 0); + cvt_nz(libc::pthread_mutexattr_init(attr.as_mut_ptr())).unwrap(); + let attr = PthreadMutexAttr(&mut attr); + cvt_nz(libc::pthread_mutexattr_settype(attr.0.as_mut_ptr(), libc::PTHREAD_MUTEX_RECURSIVE)) + .unwrap(); + cvt_nz(libc::pthread_mutex_init(self.inner.get(), attr.0.as_ptr())).unwrap(); } pub unsafe fn lock(&self) { @@ -137,3 +131,14 @@ impl ReentrantMutex { debug_assert_eq!(result, 0); } } + +struct PthreadMutexAttr<'a>(&'a mut MaybeUninit); + +impl Drop for PthreadMutexAttr<'_> { + fn drop(&mut self) { + unsafe { + let result = libc::pthread_mutexattr_destroy(self.0.as_mut_ptr()); + debug_assert_eq!(result, 0); + } + } +} diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index b05319c89e8f2..a590c74435639 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -281,7 +281,7 @@ impl Command { envp: Option<&CStringArray>, ) -> io::Result> { use crate::mem::MaybeUninit; - use crate::sys; + use crate::sys::{self, cvt_nz}; if self.get_gid().is_some() || self.get_uid().is_some() @@ -343,10 +343,6 @@ impl Command { } } - fn cvt_nz(error: libc::c_int) -> io::Result<()> { - if error == 0 { Ok(()) } else { Err(io::Error::from_raw_os_error(error)) } - } - unsafe { let mut attrs = MaybeUninit::uninit(); cvt_nz(libc::posix_spawnattr_init(attrs.as_mut_ptr()))?;