Skip to content

Commit

Permalink
Auto merge of rust-lang#98949 - ehuss:update-beta-cargo, r=ehuss
Browse files Browse the repository at this point in the history
[beta] Beta 1.63 backports

* fix data race in thread::scope rust-lang#98503
* Mitigate MMIO stale data vulnerability rust-lang#98126
* Cargo:
    * [BETA-1.63] Fix zsh completions for add and locate-project (rust-lang/cargo#10811)
    * [BETA-1.63] Bump cargo-util version. (rust-lang/cargo#10805)
  • Loading branch information
bors committed Jul 6, 2022
2 parents 59f577d + 372fa7c commit 94811fd
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 22 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ dependencies = [

[[package]]
name = "cargo-util"
version = "0.1.4"
version = "0.2.1"
dependencies = [
"anyhow",
"core-foundation",
Expand Down
120 changes: 110 additions & 10 deletions library/std/src/sys/sgx/abi/usercalls/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
#![allow(unused)]

use crate::arch::asm;
use crate::cell::UnsafeCell;
use crate::cmp;
use crate::convert::TryInto;
use crate::mem;
use crate::ops::{CoerceUnsized, Deref, DerefMut, Index, IndexMut};
use crate::ptr::{self, NonNull};
use crate::slice;
use crate::slice::SliceIndex;

use super::super::mem::is_user_range;
use super::super::mem::{is_enclave_range, is_user_range};
use fortanix_sgx_abi::*;

/// A type that can be safely read from or written to userspace.
Expand Down Expand Up @@ -210,7 +213,9 @@ where
unsafe {
// Mustn't call alloc with size 0.
let ptr = if size > 0 {
rtunwrap!(Ok, super::alloc(size, T::align_of())) as _
// `copy_to_userspace` is more efficient when data is 8-byte aligned
let alignment = cmp::max(T::align_of(), 8);
rtunwrap!(Ok, super::alloc(size, alignment)) as _
} else {
T::align_of() as _ // dangling pointer ok for size 0
};
Expand All @@ -225,13 +230,9 @@ where
/// Copies `val` into freshly allocated space in user memory.
pub fn new_from_enclave(val: &T) -> Self {
unsafe {
let ret = Self::new_uninit_bytes(mem::size_of_val(val));
ptr::copy(
val as *const T as *const u8,
ret.0.as_ptr() as *mut u8,
mem::size_of_val(val),
);
ret
let mut user = Self::new_uninit_bytes(mem::size_of_val(val));
user.copy_from_enclave(val);
user
}
}

Expand Down Expand Up @@ -304,6 +305,105 @@ where
}
}

/// Copies `len` bytes of data from enclave pointer `src` to userspace `dst`
///
/// This function mitigates stale data vulnerabilities by ensuring all writes to untrusted memory are either:
/// - preceded by the VERW instruction and followed by the MFENCE; LFENCE instruction sequence
/// - or are in multiples of 8 bytes, aligned to an 8-byte boundary
///
/// # Panics
/// This function panics if:
///
/// * The `src` pointer is null
/// * The `dst` pointer is null
/// * The `src` memory range is not in enclave memory
/// * The `dst` memory range is not in user memory
///
/// # References
/// - https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00615.html
/// - https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/processor-mmio-stale-data-vulnerabilities.html#inpage-nav-3-2-2
pub(crate) unsafe fn copy_to_userspace(src: *const u8, dst: *mut u8, len: usize) {
unsafe fn copy_bytewise_to_userspace(src: *const u8, dst: *mut u8, len: usize) {
unsafe {
let mut seg_sel: u16 = 0;
for off in 0..len {
asm!("
mov %ds, ({seg_sel})
verw ({seg_sel})
movb {val}, ({dst})
mfence
lfence
",
val = in(reg_byte) *src.offset(off as isize),
dst = in(reg) dst.offset(off as isize),
seg_sel = in(reg) &mut seg_sel,
options(nostack, att_syntax)
);
}
}
}

unsafe fn copy_aligned_quadwords_to_userspace(src: *const u8, dst: *mut u8, len: usize) {
unsafe {
asm!(
"rep movsq (%rsi), (%rdi)",
inout("rcx") len / 8 => _,
inout("rdi") dst => _,
inout("rsi") src => _,
options(att_syntax, nostack, preserves_flags)
);
}
}
assert!(!src.is_null());
assert!(!dst.is_null());
assert!(is_enclave_range(src, len));
assert!(is_user_range(dst, len));
assert!(len < isize::MAX as usize);
assert!(!(src as usize).overflowing_add(len).1);
assert!(!(dst as usize).overflowing_add(len).1);

if len < 8 {
// Can't align on 8 byte boundary: copy safely byte per byte
unsafe {
copy_bytewise_to_userspace(src, dst, len);
}
} else if len % 8 == 0 && dst as usize % 8 == 0 {
// Copying 8-byte aligned quadwords: copy quad word per quad word
unsafe {
copy_aligned_quadwords_to_userspace(src, dst, len);
}
} else {
// Split copies into three parts:
// +--------+
// | small0 | Chunk smaller than 8 bytes
// +--------+
// | big | Chunk 8-byte aligned, and size a multiple of 8 bytes
// +--------+
// | small1 | Chunk smaller than 8 bytes
// +--------+

unsafe {
// Copy small0
let small0_size = (8 - dst as usize % 8) as u8;
let small0_src = src;
let small0_dst = dst;
copy_bytewise_to_userspace(small0_src as _, small0_dst, small0_size as _);

// Copy big
let small1_size = ((len - small0_size as usize) % 8) as u8;
let big_size = len - small0_size as usize - small1_size as usize;
let big_src = src.offset(small0_size as _);
let big_dst = dst.offset(small0_size as _);
copy_aligned_quadwords_to_userspace(big_src as _, big_dst, big_size);

// Copy small1
let small1_src = src.offset(big_size as isize + small0_size as isize);
let small1_dst = dst.offset(big_size as isize + small0_size as isize);
copy_bytewise_to_userspace(small1_src, small1_dst, small1_size as _);
}
}
}

#[unstable(feature = "sgx_platform", issue = "56975")]
impl<T: ?Sized> UserRef<T>
where
Expand Down Expand Up @@ -352,7 +452,7 @@ where
pub fn copy_from_enclave(&mut self, val: &T) {
unsafe {
assert_eq!(mem::size_of_val(val), mem::size_of_val(&*self.0.get()));
ptr::copy(
copy_to_userspace(
val as *const T as *const u8,
self.0.get() as *mut T as *mut u8,
mem::size_of_val(val),
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/sgx/abi/usercalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use crate::time::{Duration, Instant};
pub(crate) mod alloc;
#[macro_use]
pub(crate) mod raw;
#[cfg(test)]
mod tests;

use self::raw::*;

Expand Down
30 changes: 30 additions & 0 deletions library/std/src/sys/sgx/abi/usercalls/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use super::alloc::copy_to_userspace;
use super::alloc::User;

#[test]
fn test_copy_function() {
let mut src = [0u8; 100];
let mut dst = User::<[u8]>::uninitialized(100);

for i in 0..src.len() {
src[i] = i as _;
}

for size in 0..48 {
// For all possible alignment
for offset in 0..8 {
// overwrite complete dst
dst.copy_from_enclave(&[0u8; 100]);

// Copy src[0..size] to dst + offset
unsafe { copy_to_userspace(src.as_ptr(), dst.as_mut_ptr().offset(offset), size) };

// Verify copy
for byte in 0..size {
unsafe {
assert_eq!(*dst.as_ptr().offset(offset + byte as isize), src[byte as usize]);
}
}
}
}
}
17 changes: 11 additions & 6 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ use crate::cell::UnsafeCell;
use crate::ffi::{CStr, CString};
use crate::fmt;
use crate::io;
use crate::marker::PhantomData;
use crate::mem;
use crate::num::NonZeroU64;
use crate::num::NonZeroUsize;
Expand Down Expand Up @@ -462,7 +463,7 @@ impl Builder {
unsafe fn spawn_unchecked_<'a, 'scope, F, T>(
self,
f: F,
scope_data: Option<&'scope scoped::ScopeData>,
scope_data: Option<Arc<scoped::ScopeData>>,
) -> io::Result<JoinInner<'scope, T>>
where
F: FnOnce() -> T,
Expand All @@ -479,8 +480,11 @@ impl Builder {
}));
let their_thread = my_thread.clone();

let my_packet: Arc<Packet<'scope, T>> =
Arc::new(Packet { scope: scope_data, result: UnsafeCell::new(None) });
let my_packet: Arc<Packet<'scope, T>> = Arc::new(Packet {
scope: scope_data,
result: UnsafeCell::new(None),
_marker: PhantomData,
});
let their_packet = my_packet.clone();

let output_capture = crate::io::set_output_capture(None);
Expand All @@ -507,7 +511,7 @@ impl Builder {
unsafe { *their_packet.result.get() = Some(try_result) };
};

if let Some(scope_data) = scope_data {
if let Some(scope_data) = &my_packet.scope {
scope_data.increment_num_running_threads();
}

Expand Down Expand Up @@ -1298,8 +1302,9 @@ pub type Result<T> = crate::result::Result<T, Box<dyn Any + Send + 'static>>;
// An Arc to the packet is stored into a `JoinInner` which in turns is placed
// in `JoinHandle`.
struct Packet<'scope, T> {
scope: Option<&'scope scoped::ScopeData>,
scope: Option<Arc<scoped::ScopeData>>,
result: UnsafeCell<Option<Result<T>>>,
_marker: PhantomData<Option<&'scope scoped::ScopeData>>,
}

// Due to the usage of `UnsafeCell` we need to manually implement Sync.
Expand Down Expand Up @@ -1330,7 +1335,7 @@ impl<'scope, T> Drop for Packet<'scope, T> {
rtabort!("thread result panicked on drop");
}
// Book-keeping so the scope knows when it's done.
if let Some(scope) = self.scope {
if let Some(scope) = &self.scope {
// Now that there will be no more user code running on this thread
// that can use 'scope, mark the thread as 'finished'.
// It's important we only do this after the `result` has been dropped,
Expand Down
10 changes: 6 additions & 4 deletions library/std/src/thread/scoped.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::sync::Arc;
/// See [`scope`] for details.
#[stable(feature = "scoped_threads", since = "1.63.0")]
pub struct Scope<'scope, 'env: 'scope> {
data: ScopeData,
data: Arc<ScopeData>,
/// Invariance over 'scope, to make sure 'scope cannot shrink,
/// which is necessary for soundness.
///
Expand Down Expand Up @@ -130,12 +130,14 @@ pub fn scope<'env, F, T>(f: F) -> T
where
F: for<'scope> FnOnce(&'scope Scope<'scope, 'env>) -> T,
{
// We put the `ScopeData` into an `Arc` so that other threads can finish their
// `decrement_num_running_threads` even after this function returns.
let scope = Scope {
data: ScopeData {
data: Arc::new(ScopeData {
num_running_threads: AtomicUsize::new(0),
main_thread: current(),
a_thread_panicked: AtomicBool::new(false),
},
}),
env: PhantomData,
scope: PhantomData,
};
Expand Down Expand Up @@ -250,7 +252,7 @@ impl Builder {
F: FnOnce() -> T + Send + 'scope,
T: Send + 'scope,
{
Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(f, Some(&scope.data)) }?))
Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(f, Some(scope.data.clone())) }?))
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/cargo

0 comments on commit 94811fd

Please sign in to comment.