Rollup merge of #59613 - jethrogb:jb/waitqueue-wait-unwind, r=alexcrichton

Centril · web-flow · commit d0d34663375f · 2019-04-02T13:47:31.000+02:00
SGX target: convert a bunch of panics to aborts Fixes fortanix/rust-sgx#86, fortanix/rust-sgx#103 and in general protect preemptively against Iago attacks by aborting instead of unwinding in potentially unexpected situations.
diff --git a/src/libstd/sys/sgx/abi/mod.rs b/src/libstd/sys/sgx/abi/mod.rs
@@ -69,9 +69,9 @@ extern "C" fn entry(p1: u64, p2: u64, p3: u64, secondary: bool, p4: u64, p5: u64
         }
 
         // check entry is being called according to ABI
-        assert_eq!(p3, 0);
-        assert_eq!(p4, 0);
-        assert_eq!(p5, 0);
+        rtassert!(p3 == 0);
+        rtassert!(p4 == 0);
+        rtassert!(p5 == 0);
 
         unsafe {
             // The actual types of these arguments are `p1: *const Arg, p2:
diff --git a/src/libstd/sys/sgx/abi/reloc.rs b/src/libstd/sys/sgx/abi/reloc.rs
@@ -23,7 +23,7 @@ pub fn relocate_elf_rela() {
     };
     for rela in relas {
         if rela.info != (/*0 << 32 |*/ R_X86_64_RELATIVE as u64) {
-            panic!("Invalid relocation");
+            rtabort!("Invalid relocation");
         }
         unsafe { *mem::rel_ptr_mut::<*const ()>(rela.offset) = mem::rel_ptr(rela.addend) };
     }
diff --git a/src/libstd/sys/sgx/abi/tls.rs b/src/libstd/sys/sgx/abi/tls.rs
@@ -100,20 +100,24 @@ impl Tls {
     }
 
     pub fn create(dtor: Option<unsafe extern fn(*mut u8)>) -> Key {
-        let index = TLS_KEY_IN_USE.set().expect("TLS limit exceeded");
+        let index = if let Some(index) = TLS_KEY_IN_USE.set() {
+            index
+        } else {
+            rtabort!("TLS limit exceeded")
+        };
         TLS_DESTRUCTOR[index].store(dtor.map_or(0, |f| f as usize), Ordering::Relaxed);
         Key::from_index(index)
     }
 
     pub fn set(key: Key, value: *mut u8) {
         let index = key.to_index();
-        assert!(TLS_KEY_IN_USE.get(index));
+        rtassert!(TLS_KEY_IN_USE.get(index));
         unsafe { Self::current() }.data[index].set(value);
     }
 
     pub fn get(key: Key) -> *mut u8 {
         let index = key.to_index();
-        assert!(TLS_KEY_IN_USE.get(index));
+        rtassert!(TLS_KEY_IN_USE.get(index));
         unsafe { Self::current() }.data[index].get()
     }
 
diff --git a/src/libstd/sys/sgx/abi/usercalls/alloc.rs b/src/libstd/sys/sgx/abi/usercalls/alloc.rs
@@ -190,11 +190,15 @@ impl<T: ?Sized> User<T> where T: UserSafe {
         unsafe {
             // Mustn't call alloc with size 0.
             let ptr = if size > 0 {
-                super::alloc(size, T::align_of()).expect("User memory allocation failed") as _
+                rtunwrap!(Ok, super::alloc(size, T::align_of())) as _
             } else {
                 T::align_of() as _ // dangling pointer ok for size 0
             };
-            User(NonNull::new_userref(T::from_raw_sized(ptr, size)))
+            if let Ok(v) = crate::panic::catch_unwind(|| T::from_raw_sized(ptr, size)) {
+                User(NonNull::new_userref(v))
+            } else {
+                rtabort!("Got invalid pointer from alloc() usercall")
+            }
         }
     }
 
diff --git a/src/libstd/sys/sgx/abi/usercalls/mod.rs b/src/libstd/sys/sgx/abi/usercalls/mod.rs
@@ -52,7 +52,7 @@ pub fn close(fd: Fd) {
 
 fn string_from_bytebuffer(buf: &alloc::UserRef<ByteBuffer>, usercall: &str, arg: &str) -> String {
     String::from_utf8(buf.copy_user_buffer())
-        .unwrap_or_else(|_| panic!("Usercall {}: expected {} to be valid UTF-8", usercall, arg))
+        .unwrap_or_else(|_| rtabort!("Usercall {}: expected {} to be valid UTF-8", usercall, arg))
 }
 
 /// Usercall `bind_stream`. See the ABI documentation for more information.
@@ -176,7 +176,7 @@ fn check_os_error(err: Result) -> i32 {
     {
         err
     } else {
-        panic!("Usercall: returned invalid error value {}", err)
+        rtabort!("Usercall: returned invalid error value {}", err)
     }
 }
 
diff --git a/src/libstd/sys/sgx/abi/usercalls/raw.rs b/src/libstd/sys/sgx/abi/usercalls/raw.rs
@@ -131,22 +131,22 @@ impl<T: RegisterArgument> RegisterArgument for Option<NonNull<T>> {
 
 impl ReturnValue for ! {
     fn from_registers(call: &'static str, _regs: (Register, Register)) -> Self {
-        panic!("Usercall {}: did not expect to be re-entered", call);
+        rtabort!("Usercall {}: did not expect to be re-entered", call);
     }
 }
 
 impl ReturnValue for () {
-    fn from_registers(call: &'static str, regs: (Register, Register)) -> Self {
-        assert_eq!(regs.0, 0, "Usercall {}: expected {} return value to be 0", call, "1st");
-        assert_eq!(regs.1, 0, "Usercall {}: expected {} return value to be 0", call, "2nd");
+    fn from_registers(call: &'static str, usercall_retval: (Register, Register)) -> Self {
+        rtassert!(usercall_retval.0 == 0);
+        rtassert!(usercall_retval.1 == 0);
         ()
     }
 }
 
 impl<T: RegisterArgument> ReturnValue for T {
-    fn from_registers(call: &'static str, regs: (Register, Register)) -> Self {
-        assert_eq!(regs.1, 0, "Usercall {}: expected {} return value to be 0", call, "2nd");
-        T::from_register(regs.0)
+    fn from_registers(call: &'static str, usercall_retval: (Register, Register)) -> Self {
+        rtassert!(usercall_retval.1 == 0);
+        T::from_register(usercall_retval.0)
     }
 }
 
@@ -174,8 +174,7 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
         #[inline(always)]
         pub unsafe fn $f($n1: $t1, $n2: $t2, $n3: $t3, $n4: $t4) -> $r {
             ReturnValue::from_registers(stringify!($f), do_usercall(
-                NonZeroU64::new(Usercalls::$f as Register)
-                    .expect("Usercall number must be non-zero"),
+                rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
                 RegisterArgument::into_register($n1),
                 RegisterArgument::into_register($n2),
                 RegisterArgument::into_register($n3),
@@ -191,8 +190,7 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
         #[inline(always)]
         pub unsafe fn $f($n1: $t1, $n2: $t2, $n3: $t3) -> $r {
             ReturnValue::from_registers(stringify!($f), do_usercall(
-                NonZeroU64::new(Usercalls::$f as Register)
-                    .expect("Usercall number must be non-zero"),
+                rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
                 RegisterArgument::into_register($n1),
                 RegisterArgument::into_register($n2),
                 RegisterArgument::into_register($n3),
@@ -208,8 +206,7 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
         #[inline(always)]
         pub unsafe fn $f($n1: $t1, $n2: $t2) -> $r {
             ReturnValue::from_registers(stringify!($f), do_usercall(
-                NonZeroU64::new(Usercalls::$f as Register)
-                    .expect("Usercall number must be non-zero"),
+                rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
                 RegisterArgument::into_register($n1),
                 RegisterArgument::into_register($n2),
                 0,0,
@@ -224,8 +221,7 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
         #[inline(always)]
         pub unsafe fn $f($n1: $t1) -> $r {
             ReturnValue::from_registers(stringify!($f), do_usercall(
-                NonZeroU64::new(Usercalls::$f as Register)
-                    .expect("Usercall number must be non-zero"),
+                rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
                 RegisterArgument::into_register($n1),
                 0,0,0,
                 return_type_is_abort!($r)
@@ -239,8 +235,7 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
         #[inline(always)]
         pub unsafe fn $f() -> $r {
             ReturnValue::from_registers(stringify!($f), do_usercall(
-                NonZeroU64::new(Usercalls::$f as Register)
-                    .expect("Usercall number must be non-zero"),
+                rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
                 0,0,0,0,
                 return_type_is_abort!($r)
             ))
diff --git a/src/libstd/sys/sgx/condvar.rs b/src/libstd/sys/sgx/condvar.rs
@@ -32,9 +32,8 @@ impl Condvar {
         mutex.lock()
     }
 
-    pub unsafe fn wait_timeout(&self, mutex: &Mutex, _dur: Duration) -> bool {
-        mutex.unlock(); // don't hold the lock while panicking
-        panic!("timeout not supported in SGX");
+    pub unsafe fn wait_timeout(&self, _mutex: &Mutex, _dur: Duration) -> bool {
+        rtabort!("timeout not supported in SGX");
     }
 
     #[inline]
diff --git a/src/libstd/sys/sgx/mod.rs b/src/libstd/sys/sgx/mod.rs
@@ -139,7 +139,7 @@ pub fn hashmap_random_keys() -> (u64, u64) {
                     return ret;
                 }
             }
-            panic!("Failed to obtain random data");
+            rtabort!("Failed to obtain random data");
         }
     }
     (rdrand64(), rdrand64())
diff --git a/src/libstd/sys/sgx/rwlock.rs b/src/libstd/sys/sgx/rwlock.rs
@@ -105,7 +105,7 @@ impl RWLock {
                 *wguard.lock_var_mut() = true;
             } else {
                 // No writers were waiting, the lock is released
-                assert!(rguard.queue_empty());
+                rtassert!(rguard.queue_empty());
             }
         }
     }
diff --git a/src/libstd/sys/sgx/thread.rs b/src/libstd/sys/sgx/thread.rs
@@ -62,25 +62,23 @@ impl Thread {
     }
 
     pub(super) fn entry() {
-        let mut guard = task_queue::lock();
-        let task = guard.pop().expect("Thread started but no tasks pending");
-        drop(guard); // make sure to not hold the task queue lock longer than necessary
+        let mut pending_tasks = task_queue::lock();
+        let task = rtunwrap!(Some, pending_tasks.pop());
+        drop(pending_tasks); // make sure to not hold the task queue lock longer than necessary
         task.run()
     }
 
     pub fn yield_now() {
-        assert_eq!(
-            usercalls::wait(0, usercalls::raw::WAIT_NO).unwrap_err().kind(),
-            io::ErrorKind::WouldBlock
-        );
+        let wait_error = rtunwrap!(Err, usercalls::wait(0, usercalls::raw::WAIT_NO));
+        rtassert!(wait_error.kind() == io::ErrorKind::WouldBlock);
     }
 
     pub fn set_name(_name: &CStr) {
         // FIXME: could store this pointer in TLS somewhere
     }
 
     pub fn sleep(_dur: Duration) {
-        panic!("can't sleep"); // FIXME
+        rtabort!("can't sleep"); // FIXME
     }
 
     pub fn join(self) {
diff --git a/src/libstd/sys/sgx/waitqueue.rs b/src/libstd/sys/sgx/waitqueue.rs
@@ -121,7 +121,7 @@ impl<'a, T> Drop for WaitGuard<'a, T> {
             NotifiedTcs::Single(tcs) => Some(tcs),
             NotifiedTcs::All { .. } => None
         };
-        usercalls::send(EV_UNPARK, target_tcs).unwrap();
+        rtunwrap!(Ok, usercalls::send(EV_UNPARK, target_tcs));
     }
 }
 
@@ -141,6 +141,7 @@ impl WaitQueue {
     ///
     /// This function does not return until this thread has been awoken.
     pub fn wait<T>(mut guard: SpinMutexGuard<'_, WaitVariable<T>>) {
+        // very unsafe: check requirements of UnsafeList::push
         unsafe {
             let mut entry = UnsafeListEntry::new(SpinMutex::new(WaitEntry {
                 tcs: thread::current(),
@@ -149,10 +150,9 @@ impl WaitQueue {
             let entry = guard.queue.inner.push(&mut entry);
             drop(guard);
             while !entry.lock().wake {
-                assert_eq!(
-                    usercalls::wait(EV_UNPARK, WAIT_INDEFINITE).unwrap() & EV_UNPARK,
-                    EV_UNPARK
-                );
+                // don't panic, this would invalidate `entry` during unwinding
+                let eventset = rtunwrap!(Ok, usercalls::wait(EV_UNPARK, WAIT_INDEFINITE));
+                rtassert!(eventset & EV_UNPARK == EV_UNPARK);
             }
         }
     }
@@ -269,7 +269,7 @@ mod unsafe_list {
                         // ,-------> /---------\ next ---,
                         // |         |head_tail|         |
                         // `--- prev \---------/ <-------`
-                        assert_eq!(self.head_tail.as_ref().prev, first);
+                        rtassert!(self.head_tail.as_ref().prev == first);
                         true
                     } else {
                         false
@@ -285,7 +285,9 @@ mod unsafe_list {
         /// # Safety
         ///
         /// The entry must remain allocated until the entry is removed from the
-        /// list AND the caller who popped is done using the entry.
+        /// list AND the caller who popped is done using the entry. Special
+        /// care must be taken in the caller of `push` to ensure unwinding does
+        /// not destroy the stack frame containing the entry.
         pub unsafe fn push<'a>(&mut self, entry: &'a mut UnsafeListEntry<T>) -> &'a T {
             self.init();
 
@@ -303,6 +305,7 @@ mod unsafe_list {
             entry.as_mut().prev = prev_tail;
             entry.as_mut().next = self.head_tail;
             prev_tail.as_mut().next = entry;
+            // unwrap ok: always `Some` on non-dummy entries
             (*entry.as_ptr()).value.as_ref().unwrap()
         }
 
@@ -333,6 +336,7 @@ mod unsafe_list {
                 second.as_mut().prev = self.head_tail;
                 first.as_mut().next = NonNull::dangling();
                 first.as_mut().prev = NonNull::dangling();
+                // unwrap ok: always `Some` on non-dummy entries
                 Some((*first.as_ptr()).value.as_ref().unwrap())
             }
         }
diff --git a/src/libstd/sys_common/mod.rs b/src/libstd/sys_common/mod.rs
@@ -28,6 +28,15 @@ macro_rules! rtassert {
     })
 }
 
+#[allow(unused_macros)] // not used on all platforms
+macro_rules! rtunwrap {
+    ($ok:ident, $e:expr) => (if let $ok(v) = $e {
+        v
+    } else {
+        rtabort!(concat!("unwrap failed: ", stringify!($e)));
+    })
+}
+
 pub mod alloc;
 pub mod at_exit_imp;
 #[cfg(feature = "backtrace")]

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ pub fn relocate_elf_rela() {`
`23`	`23`	`};`
`24`	`24`	`for rela in relas {`
`25`	`25`	`if rela.info != (/0 << 32 \|/ R_X86_64_RELATIVE as u64) {`
`26`		`- panic!("Invalid relocation");`
	`26`	`+ rtabort!("Invalid relocation");`
`27`	`27`	`}`
`28`	`28`	`unsafe { mem::rel_ptr_mut::<const ()>(rela.offset) = mem::rel_ptr(rela.addend) };`
`29`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -100,20 +100,24 @@ impl Tls {`
`100`	`100`	`}`
`101`	`101`
`102`	`102`	`pub fn create(dtor: Option<unsafe extern fn(*mut u8)>) -> Key {`
`103`		`- let index = TLS_KEY_IN_USE.set().expect("TLS limit exceeded");`
	`103`	`+ let index = if let Some(index) = TLS_KEY_IN_USE.set() {`
	`104`	`+ index`
	`105`	`+ } else {`
	`106`	`+ rtabort!("TLS limit exceeded")`
	`107`	`+ };`
`104`	`108`	`TLS_DESTRUCTOR[index].store(dtor.map_or(0, \|f\| f as usize), Ordering::Relaxed);`
`105`	`109`	`Key::from_index(index)`
`106`	`110`	`}`
`107`	`111`
`108`	`112`	`pub fn set(key: Key, value: *mut u8) {`
`109`	`113`	`let index = key.to_index();`
`110`		`- assert!(TLS_KEY_IN_USE.get(index));`
	`114`	`+ rtassert!(TLS_KEY_IN_USE.get(index));`
`111`	`115`	`unsafe { Self::current() }.data[index].set(value);`
`112`	`116`	`}`
`113`	`117`
`114`	`118`	`pub fn get(key: Key) -> *mut u8 {`
`115`	`119`	`let index = key.to_index();`
`116`		`- assert!(TLS_KEY_IN_USE.get(index));`
	`120`	`+ rtassert!(TLS_KEY_IN_USE.get(index));`
`117`	`121`	`unsafe { Self::current() }.data[index].get()`
`118`	`122`	`}`
`119`	`123`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ pub fn close(fd: Fd) {`
`52`	`52`
`53`	`53`	`fn string_from_bytebuffer(buf: &alloc::UserRef<ByteBuffer>, usercall: &str, arg: &str) -> String {`
`54`	`54`	`String::from_utf8(buf.copy_user_buffer())`
`55`		`- .unwrap_or_else(\|_\| panic!("Usercall {}: expected {} to be valid UTF-8", usercall, arg))`
	`55`	`+ .unwrap_or_else(\|_\| rtabort!("Usercall {}: expected {} to be valid UTF-8", usercall, arg))`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	/// Usercall `bind_stream`. See the ABI documentation for more information.
`@@ -176,7 +176,7 @@ fn check_os_error(err: Result) -> i32 {`
`176`	`176`	`{`
`177`	`177`	`err`
`178`	`178`	`} else {`
`179`		`- panic!("Usercall: returned invalid error value {}", err)`
	`179`	`+ rtabort!("Usercall: returned invalid error value {}", err)`
`180`	`180`	`}`
`181`	`181`	`}`
`182`	`182`
Original file line number	Diff line number	Diff line change
`@@ -32,9 +32,8 @@ impl Condvar {`
`32`	`32`	`mutex.lock()`
`33`	`33`	`}`
`34`	`34`
`35`		`- pub unsafe fn wait_timeout(&self, mutex: &Mutex, _dur: Duration) -> bool {`
`36`		`- mutex.unlock(); // don't hold the lock while panicking`
`37`		`- panic!("timeout not supported in SGX");`
	`35`	`+ pub unsafe fn wait_timeout(&self, _mutex: &Mutex, _dur: Duration) -> bool {`
	`36`	`+ rtabort!("timeout not supported in SGX");`
`38`	`37`	`}`
`39`	`38`
`40`	`39`	`#[inline]`
Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ pub fn hashmap_random_keys() -> (u64, u64) {`
`139`	`139`	`return ret;`
`140`	`140`	`}`
`141`	`141`	`}`
`142`		`- panic!("Failed to obtain random data");`
	`142`	`+ rtabort!("Failed to obtain random data");`
`143`	`143`	`}`
`144`	`144`	`}`
`145`	`145`	`(rdrand64(), rdrand64())`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ impl RWLock {`
`105`	`105`	`*wguard.lock_var_mut() = true;`
`106`	`106`	`} else {`
`107`	`107`	`// No writers were waiting, the lock is released`
`108`		`- assert!(rguard.queue_empty());`
	`108`	`+ rtassert!(rguard.queue_empty());`
`109`	`109`	`}`
`110`	`110`	`}`
`111`	`111`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,25 +62,23 @@ impl Thread {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`pub(super) fn entry() {`
`65`		`- let mut guard = task_queue::lock();`
`66`		`- let task = guard.pop().expect("Thread started but no tasks pending");`
`67`		`- drop(guard); // make sure to not hold the task queue lock longer than necessary`
	`65`	`+ let mut pending_tasks = task_queue::lock();`
	`66`	`+ let task = rtunwrap!(Some, pending_tasks.pop());`
	`67`	`+ drop(pending_tasks); // make sure to not hold the task queue lock longer than necessary`
`68`	`68`	`task.run()`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`pub fn yield_now() {`
`72`		`- assert_eq!(`
`73`		`- usercalls::wait(0, usercalls::raw::WAIT_NO).unwrap_err().kind(),`
`74`		`- io::ErrorKind::WouldBlock`
`75`		`- );`
	`72`	`+ let wait_error = rtunwrap!(Err, usercalls::wait(0, usercalls::raw::WAIT_NO));`
	`73`	`+ rtassert!(wait_error.kind() == io::ErrorKind::WouldBlock);`
`76`	`74`	`}`
`77`	`75`
`78`	`76`	`pub fn set_name(_name: &CStr) {`
`79`	`77`	`// FIXME: could store this pointer in TLS somewhere`
`80`	`78`	`}`
`81`	`79`
`82`	`80`	`pub fn sleep(_dur: Duration) {`
`83`		`- panic!("can't sleep"); // FIXME`
	`81`	`+ rtabort!("can't sleep"); // FIXME`
`84`	`82`	`}`
`85`	`83`
`86`	`84`	`pub fn join(self) {`