Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[deny(unsafe_op_in_unsafe_fn)] in sys/sgx #77346

Merged
merged 1 commit into from Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion library/std/src/sys/sgx/abi/mod.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
/// 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,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
Loading