Skip to content

Commit

Permalink
Auto merge of #77346 - Caduser2020:master, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
`#[deny(unsafe_op_in_unsafe_fn)]` in sys/sgx

This is part of #73904.

Enclose unsafe operations in unsafe blocks in `libstd/sys/sgx`.
  • Loading branch information
bors committed Oct 8, 2020
2 parents ccea570 + 1fb0a1d commit 6b8d791
Show file tree
Hide file tree
Showing 14 changed files with 170 additions and 119 deletions.
2 changes: 1 addition & 1 deletion library/std/src/sys/sgx/abi/mod.rs
Expand Up @@ -45,7 +45,7 @@ unsafe extern "C" fn tcs_init(secondary: bool) {
// We need to wait until the initialization is done.
BUSY => {
while RELOC_STATE.load(Ordering::Acquire) == BUSY {
core::arch::x86_64::_mm_pause()
core::hint::spin_loop();
}
}
// Initialization is done.
Expand Down
9 changes: 6 additions & 3 deletions library/std/src/sys/sgx/abi/tls.rs
Expand Up @@ -87,18 +87,21 @@ impl Tls {
}

pub unsafe fn activate(&self) -> ActiveTls<'_> {
set_tls_ptr(self as *const Tls as _);
// FIXME: Needs safety information. See entry.S for `set_tls_ptr` definition.
unsafe { set_tls_ptr(self as *const Tls as _) };
ActiveTls { tls: self }
}

#[allow(unused)]
pub unsafe fn activate_persistent(self: Box<Self>) {
set_tls_ptr((&*self) as *const Tls as _);
// FIXME: Needs safety information. See entry.S for `set_tls_ptr` definition.
unsafe { set_tls_ptr((&*self) as *const Tls as _) };
mem::forget(self);
}

unsafe fn current<'a>() -> &'a Tls {
&*(get_tls_ptr() as *const Tls)
// FIXME: Needs safety information. See entry.S for `set_tls_ptr` definition.
unsafe { &*(get_tls_ptr() as *const Tls) }
}

pub fn create(dtor: Option<unsafe extern "C" fn(*mut u8)>) -> Key {
Expand Down
71 changes: 49 additions & 22 deletions library/std/src/sys/sgx/abi/usercalls/alloc.rs
Expand Up @@ -89,9 +89,12 @@ pub unsafe trait UserSafe {
/// * the pointed-to range is not in user memory.
unsafe fn from_raw_sized(ptr: *mut u8, size: usize) -> NonNull<Self> {
assert!(ptr.wrapping_add(size) >= ptr);
let ret = Self::from_raw_sized_unchecked(ptr, size);
Self::check_ptr(ret);
NonNull::new_unchecked(ret as _)
// SAFETY: The caller has guaranteed the pointer is valid
let ret = unsafe { Self::from_raw_sized_unchecked(ptr, size) };
unsafe {
Self::check_ptr(ret);
NonNull::new_unchecked(ret as _)
}
}

/// Checks if a pointer may point to `Self` in user memory.
Expand All @@ -112,7 +115,7 @@ pub unsafe trait UserSafe {
let is_aligned = |p| -> bool { 0 == (p as usize) & (Self::align_of() - 1) };

assert!(is_aligned(ptr as *const u8));
assert!(is_user_range(ptr as _, mem::size_of_val(&*ptr)));
assert!(is_user_range(ptr as _, mem::size_of_val(unsafe { &*ptr })));
assert!(!ptr.is_null());
}
}
Expand All @@ -135,11 +138,23 @@ unsafe impl<T: UserSafeSized> UserSafe for [T] {
mem::align_of::<T>()
}

/// # Safety
/// Behavior is undefined if any of these conditions are violated:
/// * `ptr` must be [valid] for writes of `size` many bytes, and it must be
/// properly aligned.
///
/// [valid]: core::ptr#safety
/// # Panics
///
/// This function panics if:
///
/// * the element size is not a factor of the size
unsafe fn from_raw_sized_unchecked(ptr: *mut u8, size: usize) -> *mut Self {
let elem_size = mem::size_of::<T>();
assert_eq!(size % elem_size, 0);
let len = size / elem_size;
slice::from_raw_parts_mut(ptr as _, len)
// SAFETY: The caller must uphold the safety contract for `from_raw_sized_unchecked`
unsafe { slice::from_raw_parts_mut(ptr as _, len) }
}
}

Expand Down Expand Up @@ -170,13 +185,15 @@ trait NewUserRef<T: ?Sized> {

impl<T: ?Sized> NewUserRef<*mut T> for NonNull<UserRef<T>> {
unsafe fn new_userref(v: *mut T) -> Self {
NonNull::new_unchecked(v as _)
// SAFETY: The caller has guaranteed the pointer is valid
unsafe { NonNull::new_unchecked(v as _) }
}
}

impl<T: ?Sized> NewUserRef<NonNull<T>> for NonNull<UserRef<T>> {
unsafe fn new_userref(v: NonNull<T>) -> Self {
NonNull::new_userref(v.as_ptr())
// SAFETY: The caller has guaranteed the pointer is valid
unsafe { NonNull::new_userref(v.as_ptr()) }
}
}

Expand Down Expand Up @@ -231,8 +248,9 @@ where
/// * The pointer is null
/// * The pointed-to range is not in user memory
pub unsafe fn from_raw(ptr: *mut T) -> Self {
T::check_ptr(ptr);
User(NonNull::new_userref(ptr))
// SAFETY: the caller must uphold the safety contract for `from_raw`.
unsafe { T::check_ptr(ptr) };
User(unsafe { NonNull::new_userref(ptr) })
}

/// Converts this value into a raw pointer. The value will no longer be
Expand Down Expand Up @@ -280,7 +298,9 @@ where
/// * The pointed-to range does not fit in the address space
/// * The pointed-to range is not in user memory
pub unsafe fn from_raw_parts(ptr: *mut T, len: usize) -> Self {
User(NonNull::new_userref(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>())))
User(unsafe {
NonNull::new_userref(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>()))
})
}
}

Expand All @@ -301,8 +321,9 @@ where
/// * The pointer is null
/// * The pointed-to range is not in user memory
pub unsafe fn from_ptr<'a>(ptr: *const T) -> &'a Self {
T::check_ptr(ptr);
&*(ptr as *const Self)
// SAFETY: The caller must uphold the safety contract for `from_ptr`.
unsafe { T::check_ptr(ptr) };
unsafe { &*(ptr as *const Self) }
}

/// Creates a `&mut UserRef<[T]>` from a raw pointer. See the struct
Expand All @@ -318,8 +339,9 @@ where
/// * The pointer is null
/// * The pointed-to range is not in user memory
pub unsafe fn from_mut_ptr<'a>(ptr: *mut T) -> &'a mut Self {
T::check_ptr(ptr);
&mut *(ptr as *mut Self)
// SAFETY: The caller must uphold the safety contract for `from_mut_ptr`.
unsafe { T::check_ptr(ptr) };
unsafe { &mut *(ptr as *mut Self) }
}

/// Copies `val` into user memory.
Expand Down Expand Up @@ -394,7 +416,10 @@ where
/// * The pointed-to range does not fit in the address space
/// * The pointed-to range is not in user memory
pub unsafe fn from_raw_parts<'a>(ptr: *const T, len: usize) -> &'a Self {
&*(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>()).as_ptr() as *const Self)
// SAFETY: The caller must uphold the safety contract for `from_raw_parts`.
unsafe {
&*(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>()).as_ptr() as *const Self)
}
}

/// Creates a `&mut UserRef<[T]>` from a raw thin pointer and a slice length.
Expand All @@ -412,7 +437,10 @@ where
/// * The pointed-to range does not fit in the address space
/// * The pointed-to range is not in user memory
pub unsafe fn from_raw_parts_mut<'a>(ptr: *mut T, len: usize) -> &'a mut Self {
&mut *(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>()).as_ptr() as *mut Self)
// SAFETY: The caller must uphold the safety contract for `from_raw_parts_mut`.
unsafe {
&mut *(<[T]>::from_raw_sized(ptr as _, len * mem::size_of::<T>()).as_ptr() as *mut Self)
}
}

/// Obtain a raw pointer to the first element of this user slice.
Expand All @@ -437,13 +465,12 @@ where
/// This function panics if the destination doesn't have the same size as
/// the source. This can happen for dynamically-sized types such as slices.
pub fn copy_to_enclave_vec(&self, dest: &mut Vec<T>) {
unsafe {
if let Some(missing) = self.len().checked_sub(dest.capacity()) {
dest.reserve(missing)
}
dest.set_len(self.len());
self.copy_to_enclave(&mut dest[..]);
if let Some(missing) = self.len().checked_sub(dest.capacity()) {
dest.reserve(missing)
}
// SAFETY: We reserve enough space above.
unsafe { dest.set_len(self.len()) };
self.copy_to_enclave(&mut dest[..]);
}

/// Copies the value from user memory into a vector in enclave memory.
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/sys/sgx/abi/usercalls/mod.rs
Expand Up @@ -140,7 +140,8 @@ pub fn connect_stream(addr: &str) -> IoResult<(Fd, String, String)> {
/// Usercall `launch_thread`. See the ABI documentation for more information.
#[unstable(feature = "sgx_platform", issue = "56975")]
pub unsafe fn launch_thread() -> IoResult<()> {
raw::launch_thread().from_sgx_result()
// SAFETY: The caller must uphold the safety contract for `launch_thread`.
unsafe { raw::launch_thread().from_sgx_result() }
}

/// Usercall `exit`. See the ABI documentation for more information.
Expand Down
70 changes: 35 additions & 35 deletions library/std/src/sys/sgx/abi/usercalls/raw.rs
Expand Up @@ -33,7 +33,7 @@ pub unsafe fn do_usercall(
p4: u64,
abort: bool,
) -> (u64, u64) {
let UsercallReturn(a, b) = usercall(nr, p1, p2, abort as _, p3, p4);
let UsercallReturn(a, b) = unsafe { usercall(nr, p1, p2, abort as _, p3, p4) };
(a, b)
}

Expand Down Expand Up @@ -175,14 +175,14 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
#[unstable(feature = "sgx_platform", issue = "56975")]
#[inline(always)]
pub unsafe fn $f($n1: $t1, $n2: $t2, $n3: $t3, $n4: $t4) -> $r {
ReturnValue::from_registers(stringify!($f), do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
RegisterArgument::into_register($n3),
RegisterArgument::into_register($n4),
return_type_is_abort!($r)
))
ReturnValue::from_registers(stringify!($f), unsafe { do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
RegisterArgument::into_register($n3),
RegisterArgument::into_register($n4),
return_type_is_abort!($r)
) })
}
);
(def fn $f:ident($n1:ident: $t1:ty, $n2:ident: $t2:ty, $n3:ident: $t3:ty) -> $r:tt) => (
Expand All @@ -191,14 +191,14 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
#[unstable(feature = "sgx_platform", issue = "56975")]
#[inline(always)]
pub unsafe fn $f($n1: $t1, $n2: $t2, $n3: $t3) -> $r {
ReturnValue::from_registers(stringify!($f), do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
RegisterArgument::into_register($n3),
0,
return_type_is_abort!($r)
))
ReturnValue::from_registers(stringify!($f), unsafe { do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
RegisterArgument::into_register($n3),
0,
return_type_is_abort!($r)
) })
}
);
(def fn $f:ident($n1:ident: $t1:ty, $n2:ident: $t2:ty) -> $r:tt) => (
Expand All @@ -207,13 +207,13 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
#[unstable(feature = "sgx_platform", issue = "56975")]
#[inline(always)]
pub unsafe fn $f($n1: $t1, $n2: $t2) -> $r {
ReturnValue::from_registers(stringify!($f), do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
0,0,
return_type_is_abort!($r)
))
ReturnValue::from_registers(stringify!($f), unsafe { do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
RegisterArgument::into_register($n2),
0,0,
return_type_is_abort!($r)
) })
}
);
(def fn $f:ident($n1:ident: $t1:ty) -> $r:tt) => (
Expand All @@ -222,12 +222,12 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
#[unstable(feature = "sgx_platform", issue = "56975")]
#[inline(always)]
pub unsafe fn $f($n1: $t1) -> $r {
ReturnValue::from_registers(stringify!($f), do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
0,0,0,
return_type_is_abort!($r)
))
ReturnValue::from_registers(stringify!($f), unsafe { do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
RegisterArgument::into_register($n1),
0,0,0,
return_type_is_abort!($r)
) })
}
);
(def fn $f:ident() -> $r:tt) => (
Expand All @@ -236,11 +236,11 @@ macro_rules! enclave_usercalls_internal_define_usercalls {
#[unstable(feature = "sgx_platform", issue = "56975")]
#[inline(always)]
pub unsafe fn $f() -> $r {
ReturnValue::from_registers(stringify!($f), do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
0,0,0,0,
return_type_is_abort!($r)
))
ReturnValue::from_registers(stringify!($f), unsafe { do_usercall(
rtunwrap!(Some, NonZeroU64::new(Usercalls::$f as Register)),
0,0,0,0,
return_type_is_abort!($r)
) })
}
);
(def fn $f:ident($($n:ident: $t:ty),*)) => (
Expand Down
20 changes: 14 additions & 6 deletions library/std/src/sys/sgx/alloc.rs
Expand Up @@ -4,6 +4,10 @@ use super::waitqueue::SpinMutex;

// Using a SpinMutex because we never want to exit the enclave waiting for the
// allocator.
//
// The current allocator here is the `dlmalloc` crate which we've got included
// in the rust-lang/rust repository as a submodule. The crate is a port of
// dlmalloc.c from C to Rust.
#[cfg_attr(test, linkage = "available_externally")]
#[export_name = "_ZN16__rust_internals3std3sys3sgx5alloc8DLMALLOCE"]
static DLMALLOC: SpinMutex<dlmalloc::Dlmalloc> = SpinMutex::new(dlmalloc::DLMALLOC_INIT);
Expand All @@ -12,22 +16,26 @@ static DLMALLOC: SpinMutex<dlmalloc::Dlmalloc> = SpinMutex::new(dlmalloc::DLMALL
unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
DLMALLOC.lock().malloc(layout.size(), layout.align())
// SAFETY: the caller must uphold the safety contract for `malloc`
unsafe { DLMALLOC.lock().malloc(layout.size(), layout.align()) }
}

#[inline]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
DLMALLOC.lock().calloc(layout.size(), layout.align())
// SAFETY: the caller must uphold the safety contract for `malloc`
unsafe { DLMALLOC.lock().calloc(layout.size(), layout.align()) }
}

#[inline]
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
DLMALLOC.lock().free(ptr, layout.size(), layout.align())
// SAFETY: the caller must uphold the safety contract for `malloc`
unsafe { DLMALLOC.lock().free(ptr, layout.size(), layout.align()) }
}

#[inline]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
DLMALLOC.lock().realloc(ptr, layout.size(), layout.align(), new_size)
// SAFETY: the caller must uphold the safety contract for `malloc`
unsafe { DLMALLOC.lock().realloc(ptr, layout.size(), layout.align(), new_size) }
}
}

Expand All @@ -36,11 +44,11 @@ unsafe impl GlobalAlloc for System {
#[cfg(not(test))]
#[no_mangle]
pub unsafe extern "C" fn __rust_c_alloc(size: usize, align: usize) -> *mut u8 {
crate::alloc::alloc(Layout::from_size_align_unchecked(size, align))
unsafe { crate::alloc::alloc(Layout::from_size_align_unchecked(size, align)) }
}

#[cfg(not(test))]
#[no_mangle]
pub unsafe extern "C" fn __rust_c_dealloc(ptr: *mut u8, size: usize, align: usize) {
crate::alloc::dealloc(ptr, Layout::from_size_align_unchecked(size, align))
unsafe { crate::alloc::dealloc(ptr, Layout::from_size_align_unchecked(size, align)) }
}
2 changes: 1 addition & 1 deletion library/std/src/sys/sgx/args.rs
Expand Up @@ -13,7 +13,7 @@ type ArgsStore = Vec<OsString>;
#[cfg_attr(test, allow(dead_code))]
pub unsafe fn init(argc: isize, argv: *const *const u8) {
if argc != 0 {
let args = alloc::User::<[ByteBuffer]>::from_raw_parts(argv as _, argc as _);
let args = unsafe { alloc::User::<[ByteBuffer]>::from_raw_parts(argv as _, argc as _) };
let args = args
.iter()
.map(|a| OsString::from_inner(Buf { inner: a.copy_user_buffer() }))
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/sgx/condvar.rs
Expand Up @@ -29,13 +29,13 @@ impl Condvar {

pub unsafe fn wait(&self, mutex: &Mutex) {
let guard = self.inner.lock();
WaitQueue::wait(guard, || mutex.unlock());
mutex.lock()
WaitQueue::wait(guard, || unsafe { mutex.unlock() });
unsafe { mutex.lock() }
}

pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
let success = WaitQueue::wait_timeout(&self.inner, dur, || mutex.unlock());
mutex.lock();
let success = WaitQueue::wait_timeout(&self.inner, dur, || unsafe { mutex.unlock() });
unsafe { mutex.lock() };
success
}

Expand Down

0 comments on commit 6b8d791

Please sign in to comment.