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

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.

Author: Centril

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:

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) };
     }

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()
     }
 

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")
+            }
         }
     }
 

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)
     }
 }
 

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)
             ))

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]

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())

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());
             }
         }
     }

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) {

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())
             }
         }

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")]