From b52666868ffd88a52bf1471f567505ab950da383 Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Fri, 28 Feb 2025 13:33:05 +0100 Subject: [PATCH 1/3] Copy from userspace to MaybeUninit Co-authored-by: Thalia Archibald --- .../src/sys/pal/sgx/abi/usercalls/alloc.rs | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs b/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs index 301e3299c0572..0132c1bd3f2e4 100644 --- a/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs +++ b/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs @@ -6,7 +6,7 @@ use super::super::mem::{is_enclave_range, is_user_range}; use crate::arch::asm; use crate::cell::UnsafeCell; use crate::convert::TryInto; -use crate::mem::{self, ManuallyDrop}; +use crate::mem::{self, ManuallyDrop, MaybeUninit}; use crate::ops::{CoerceUnsized, Deref, DerefMut, Index, IndexMut}; use crate::pin::PinCoerceUnsized; use crate::ptr::{self, NonNull}; @@ -209,6 +209,45 @@ impl NewUserRef> for NonNull> { } } +/// A type which can a destination for safely copying from userspace. +/// +/// # Safety +/// +/// Requires that `T` and `Self` have identical layouts. +#[unstable(feature = "sgx_platform", issue = "56975")] +pub unsafe trait UserSafeCopyDestination { + /// Returns a pointer for writing to the value. + fn as_mut_ptr(&mut self) -> *mut T; +} + +#[unstable(feature = "sgx_platform", issue = "56975")] +unsafe impl UserSafeCopyDestination for T { + fn as_mut_ptr(&mut self) -> *mut T { + self as _ + } +} + +#[unstable(feature = "sgx_platform", issue = "56975")] +unsafe impl UserSafeCopyDestination<[T]> for [T] { + fn as_mut_ptr(&mut self) -> *mut [T] { + self as _ + } +} + +#[unstable(feature = "sgx_platform", issue = "56975")] +unsafe impl UserSafeCopyDestination for MaybeUninit { + fn as_mut_ptr(&mut self) -> *mut T { + self as *mut Self as _ + } +} + +#[unstable(feature = "sgx_platform", issue = "56975")] +unsafe impl UserSafeCopyDestination<[T]> for [MaybeUninit] { + fn as_mut_ptr(&mut self) -> *mut [T] { + self as *mut Self as _ + } +} + #[unstable(feature = "sgx_platform", issue = "56975")] impl User where @@ -544,12 +583,12 @@ where /// # Panics /// 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(&self, dest: &mut T) { + pub fn copy_to_enclave>(&self, dest: &mut U) { unsafe { assert_eq!(size_of_val(dest), size_of_val(&*self.0.get())); copy_from_userspace( self.0.get() as *const T as *const u8, - dest as *mut T as *mut u8, + dest.as_mut_ptr() as *mut u8, size_of_val(dest), ); } From 8c7a94e4cdd9ad70550b19ca2bf6f16046d59506 Mon Sep 17 00:00:00 2001 From: Thalia Archibald Date: Fri, 7 Feb 2025 15:02:08 -0800 Subject: [PATCH 2/3] Implement read_buf and vectored read/write for SGX stdio --- .../std/src/sys/pal/sgx/abi/usercalls/mod.rs | 17 +++++++++- library/std/src/sys/pal/sgx/fd.rs | 2 +- library/std/src/sys/stdio/sgx.rs | 33 ++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs b/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs index 90b9b59471a52..cbdaf439b2847 100644 --- a/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs +++ b/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs @@ -1,5 +1,7 @@ use crate::cmp; -use crate::io::{Error as IoError, ErrorKind, IoSlice, IoSliceMut, Result as IoResult}; +use crate::io::{ + BorrowedCursor, Error as IoError, ErrorKind, IoSlice, IoSliceMut, Result as IoResult, +}; use crate::random::{DefaultRandomSource, Random}; use crate::time::{Duration, Instant}; @@ -36,6 +38,19 @@ pub fn read(fd: Fd, bufs: &mut [IoSliceMut<'_>]) -> IoResult { } } +/// Usercall `read` with an uninitialized buffer. See the ABI documentation for +/// more information. +#[unstable(feature = "sgx_platform", issue = "56975")] +pub fn read_buf(fd: Fd, mut buf: BorrowedCursor<'_>) -> IoResult<()> { + unsafe { + let mut userbuf = alloc::User::<[u8]>::uninitialized(buf.capacity()); + let len = raw::read(fd, userbuf.as_mut_ptr().cast(), userbuf.len()).from_sgx_result()?; + userbuf[..len].copy_to_enclave(&mut buf.as_mut()[..len]); + buf.advance_unchecked(len); + Ok(()) + } +} + /// Usercall `read_alloc`. See the ABI documentation for more information. #[unstable(feature = "sgx_platform", issue = "56975")] pub fn read_alloc(fd: Fd) -> IoResult> { diff --git a/library/std/src/sys/pal/sgx/fd.rs b/library/std/src/sys/pal/sgx/fd.rs index 3bb3189a1d127..399f6a1648984 100644 --- a/library/std/src/sys/pal/sgx/fd.rs +++ b/library/std/src/sys/pal/sgx/fd.rs @@ -29,7 +29,7 @@ impl FileDesc { } pub fn read_buf(&self, buf: BorrowedCursor<'_>) -> io::Result<()> { - crate::io::default_read_buf(|b| self.read(b), buf) + usercalls::read_buf(self.fd, buf) } pub fn read_vectored(&self, bufs: &mut [IoSliceMut<'_>]) -> io::Result { diff --git a/library/std/src/sys/stdio/sgx.rs b/library/std/src/sys/stdio/sgx.rs index 03d754cb2173f..1894c098d1851 100644 --- a/library/std/src/sys/stdio/sgx.rs +++ b/library/std/src/sys/stdio/sgx.rs @@ -1,6 +1,6 @@ use fortanix_sgx_abi as abi; -use crate::io; +use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; use crate::sys::fd::FileDesc; pub struct Stdin(()); @@ -24,6 +24,19 @@ impl io::Read for Stdin { fn read(&mut self, buf: &mut [u8]) -> io::Result { with_std_fd(abi::FD_STDIN, |fd| fd.read(buf)) } + + fn read_buf(&mut self, buf: BorrowedCursor<'_>) -> io::Result<()> { + with_std_fd(abi::FD_STDIN, |fd| fd.read_buf(buf)) + } + + fn read_vectored(&mut self, bufs: &mut [IoSliceMut<'_>]) -> io::Result { + with_std_fd(abi::FD_STDIN, |fd| fd.read_vectored(bufs)) + } + + #[inline] + fn is_read_vectored(&self) -> bool { + true + } } impl Stdout { @@ -40,6 +53,15 @@ impl io::Write for Stdout { fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDOUT, |fd| fd.flush()) } + + fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result { + with_std_fd(abi::FD_STDOUT, |fd| fd.write_vectored(bufs)) + } + + #[inline] + fn is_write_vectored(&self) -> bool { + true + } } impl Stderr { @@ -56,6 +78,15 @@ impl io::Write for Stderr { fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDERR, |fd| fd.flush()) } + + fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result { + with_std_fd(abi::FD_STDERR, |fd| fd.write_vectored(bufs)) + } + + #[inline] + fn is_write_vectored(&self) -> bool { + true + } } pub const STDIN_BUF_SIZE: usize = crate::sys::io::DEFAULT_BUF_SIZE; From c62aa0baa1a8228d5bfbb3e810db4c7ee77eb3a1 Mon Sep 17 00:00:00 2001 From: Thalia Archibald Date: Wed, 26 Feb 2025 11:51:28 -0800 Subject: [PATCH 3/3] Fix `UserRef<[T]>::copy_to_enclave_vec` It reinterprets uninitialized memory as initialized and does not drop existing elements of the Vec. Fix that. Additionally, make it more general by appending, instead of overwriting existing elements, and rename it to `append_to_enclave_vec`. A caller can simply call `.clear()` before, for the old behavior. --- .../src/sys/pal/sgx/abi/usercalls/alloc.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs b/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs index 0132c1bd3f2e4..3fe6dee3d6f4f 100644 --- a/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs +++ b/library/std/src/sys/pal/sgx/abi/usercalls/alloc.rs @@ -678,25 +678,18 @@ where unsafe { (*self.0.get()).len() } } - /// Copies the value from user memory and place it into `dest`. Afterwards, - /// `dest` will contain exactly `self.len()` elements. - /// - /// # Panics - /// 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) { - if let Some(missing) = self.len().checked_sub(dest.capacity()) { - dest.reserve(missing) - } + /// Copies the value from user memory and appends it to `dest`. + pub fn append_to_enclave_vec(&self, dest: &mut Vec) { + dest.reserve(self.len()); + self.copy_to_enclave(&mut dest.spare_capacity_mut()[..self.len()]); // SAFETY: We reserve enough space above. - unsafe { dest.set_len(self.len()) }; - self.copy_to_enclave(&mut dest[..]); + unsafe { dest.set_len(dest.len() + self.len()) }; } /// Copies the value from user memory into a vector in enclave memory. pub fn to_enclave(&self) -> Vec { let mut ret = Vec::with_capacity(self.len()); - self.copy_to_enclave_vec(&mut ret); + self.append_to_enclave_vec(&mut ret); ret }