diff --git a/src/libos/src/exception/mod.rs b/src/libos/src/exception/mod.rs index f931783b..e14ec0f0 100644 --- a/src/libos/src/exception/mod.rs +++ b/src/libos/src/exception/mod.rs @@ -5,7 +5,7 @@ use self::rdtsc::{handle_rdtsc_exception, RDTSC_OPCODE}; use self::syscall::{handle_syscall_exception, SYSCALL_OPCODE}; use super::*; use crate::signal::{FaultSignal, SigSet}; -use crate::syscall::{CpuContext, SyscallNum}; +use crate::syscall::{CpuContext, FpRegs, SyscallNum}; use aligned::{Aligned, A16}; use core::arch::x86_64::_fxsave; use sgx_types::*; @@ -26,19 +26,26 @@ pub fn register_exception_handlers() { #[no_mangle] extern "C" fn handle_exception(info: *mut sgx_exception_info_t) -> i32 { + // Rust compiler would complain about passing to external C functions a CpuContext + // pointer, which includes a FpRegs pointer that is not safe to use by external + // modules. In our case, the FpRegs pointer will not be used actually. So the + // Rust warning is a false alarm. We suppress it here. + #[allow(improper_ctypes)] extern "C" { fn __occlum_syscall_c_abi( num: u32, info: *mut sgx_exception_info_t, - fpregs: *mut u8, + fpregs: *mut FpRegs, ) -> u32; } - let mut fpregs: Aligned = Aligned([0u8; 512]); - let mut fpregs = fpregs.as_mut_ptr(); + let mut fpregs = FpRegs::save(); unsafe { - _fxsave(fpregs); - __occlum_syscall_c_abi(SyscallNum::HandleException as u32, info, fpregs) + __occlum_syscall_c_abi( + SyscallNum::HandleException as u32, + info, + &mut fpregs as *mut FpRegs, + ) }; unreachable!(); } @@ -46,7 +53,7 @@ extern "C" fn handle_exception(info: *mut sgx_exception_info_t) -> i32 { /// Exceptions are handled as a special kind of system calls. pub fn do_handle_exception( info: *mut sgx_exception_info_t, - fpregs: *mut u8, + fpregs: *mut FpRegs, user_context: *mut CpuContext, ) -> Result { let info = unsafe { &mut *info }; @@ -54,6 +61,7 @@ pub fn do_handle_exception( let user_context = unsafe { &mut *user_context }; *user_context = CpuContext::from_sgx(&info.cpu_context); + user_context.fpregs = fpregs; // Try to do instruction emulation first if info.exception_vector == sgx_exception_vector_t::SGX_EXCEPTION_VECTOR_UD { @@ -78,7 +86,6 @@ pub fn do_handle_exception( // // As the thread cannot proceed without handling the exception, we choose to force // delivering the signal regardless of the current signal mask. - user_context.fpregs = fpregs; let signal = Box::new(FaultSignal::new(info)); crate::signal::force_signal(signal, user_context); diff --git a/src/libos/src/interrupt/mod.rs b/src/libos/src/interrupt/mod.rs index 4853f91e..c532faef 100644 --- a/src/libos/src/interrupt/mod.rs +++ b/src/libos/src/interrupt/mod.rs @@ -1,7 +1,7 @@ pub use self::sgx::sgx_interrupt_info_t; use crate::prelude::*; use crate::process::ThreadRef; -use crate::syscall::{CpuContext, SyscallNum}; +use crate::syscall::{CpuContext, FpRegs, SyscallNum}; use aligned::{Aligned, A16}; use core::arch::x86_64::_fxsave; @@ -15,26 +15,33 @@ pub fn init() { } extern "C" fn handle_interrupt(info: *mut sgx_interrupt_info_t) -> i32 { + // Rust compiler would complain about passing to external C functions a CpuContext + // pointer, which includes a FpRegs pointer that is not safe to use by external + // modules. In our case, the FpRegs pointer will not be used actually. So the + // Rust warning is a false alarm. We suppress it here. + #[allow(improper_ctypes)] extern "C" { fn __occlum_syscall_c_abi( num: u32, info: *mut sgx_interrupt_info_t, - fpregs: *mut u8, + fpregs: *mut FpRegs, ) -> u32; } - let mut fpregs: Aligned = Aligned([0u8; 512]); - let mut fpregs = fpregs.as_mut_ptr(); + let mut fpregs = FpRegs::save(); unsafe { - _fxsave(fpregs); - __occlum_syscall_c_abi(SyscallNum::HandleInterrupt as u32, info, fpregs) + __occlum_syscall_c_abi( + SyscallNum::HandleInterrupt as u32, + info, + &mut fpregs as *mut FpRegs, + ) }; unreachable!(); } pub fn do_handle_interrupt( info: *mut sgx_interrupt_info_t, - fpregs: *mut u8, + fpregs: *mut FpRegs, cpu_context: *mut CpuContext, ) -> Result { let info = unsafe { &*info }; diff --git a/src/libos/src/signal/do_sigreturn.rs b/src/libos/src/signal/do_sigreturn.rs index 0683b879..dbf65e30 100644 --- a/src/libos/src/signal/do_sigreturn.rs +++ b/src/libos/src/signal/do_sigreturn.rs @@ -5,7 +5,7 @@ use super::{SigAction, SigActionFlags, SigDefaultAction, SigSet, Signal}; use crate::lazy_static::__Deref; use crate::prelude::*; use crate::process::{ProcessRef, TermStatus, ThreadRef}; -use crate::syscall::CpuContext; +use crate::syscall::{CpuContext, FpRegs}; use aligned::{Aligned, A16}; use core::arch::x86_64::{_fxrstor, _fxsave}; use std::{ptr, slice}; @@ -36,9 +36,8 @@ pub fn do_rt_sigreturn(curr_user_ctxt: &mut CpuContext) -> Result<()> { // Restore the floating point registers to a temp area // The floating point registers would be recoved just // before return to user's code - let mut fpregs: Box> = Box::new(Aligned([0u8; 512])); - fpregs.copy_from_slice(&last_ucontext.fpregs); - curr_user_ctxt.fpregs = Box::into_raw(fpregs) as *mut u8; + let mut fpregs = Box::new(unsafe { FpRegs::from_slice(&last_ucontext.fpregs) }); + curr_user_ctxt.fpregs = Box::into_raw(fpregs); curr_user_ctxt.fpregs_on_heap = 1; // indicates the fpregs is on heap Ok(()) } @@ -266,18 +265,17 @@ fn handle_signals_by_user( // Save the floating point registers if curr_user_ctxt.fpregs != ptr::null_mut() { - let fpregs = - unsafe { slice::from_raw_parts(curr_user_ctxt.fpregs, ucontext.fpregs.len()) }; - ucontext.fpregs.copy_from_slice(fpregs); + ucontext + .fpregs + .copy_from_slice(unsafe { curr_user_ctxt.fpregs.as_ref().unwrap().as_slice() }); // Clear the floating point registers, since we do not need to recover is when this syscall return curr_user_ctxt.fpregs = ptr::null_mut(); } else { // We need a correct fxsave structure in the buffer, // because the app may modify part of it to update the // floating point after the signal handler finished. - let mut fpregs: Aligned = Aligned([0u8; 512]); - unsafe { _fxsave(fpregs.as_mut_ptr()) }; - ucontext.fpregs.copy_from_slice(fpregs.deref()); + let fpregs = FpRegs::save(); + ucontext.fpregs.copy_from_slice(fpregs.as_slice()); } ucontext as *mut ucontext_t diff --git a/src/libos/src/syscall/mod.rs b/src/libos/src/syscall/mod.rs index b05ac6f9..326b6fb6 100644 --- a/src/libos/src/syscall/mod.rs +++ b/src/libos/src/syscall/mod.rs @@ -8,11 +8,13 @@ //! 4. Call `do_*` to process the system call (in other modules) use aligned::{Aligned, A16}; -use core::arch::x86_64::_fxrstor; +use core::arch::x86_64::{_fxrstor, _fxsave}; use std::any::Any; use std::convert::TryFrom; +use std::default::Default; use std::ffi::{CStr, CString}; use std::io::{Read, Seek, SeekFrom, Write}; +use std::mem::MaybeUninit; use std::ptr; use time::{clockid_t, timespec_t, timeval_t}; use util::log::{self, LevelFilter}; @@ -410,8 +412,8 @@ macro_rules! process_syscall_table_with_callback { // Occlum-specific system calls (Spawn = 360) => do_spawn(child_pid_ptr: *mut u32, path: *const i8, argv: *const *const i8, envp: *const *const i8, fdop_list: *const FdOp), - (HandleException = 361) => do_handle_exception(info: *mut sgx_exception_info_t, fpregs: *mut u8, context: *mut CpuContext), - (HandleInterrupt = 362) => do_handle_interrupt(info: *mut sgx_interrupt_info_t, fpregs: *mut u8, context: *mut CpuContext), + (HandleException = 361) => do_handle_exception(info: *mut sgx_exception_info_t, fpregs: *mut FpRegs, context: *mut CpuContext), + (HandleInterrupt = 362) => do_handle_interrupt(info: *mut sgx_interrupt_info_t, fpregs: *mut FpRegs, context: *mut CpuContext), } }; } @@ -714,6 +716,11 @@ fn do_syscall(user_context: &mut CpuContext) { /// Return to the user space according to the given CPU context fn do_sysret(user_context: &mut CpuContext) -> ! { + // Rust compiler would complain about passing to external C functions a CpuContext + // pointer, which includes a FpRegs pointer that is not safe to use by external + // modules. In our case, the FpRegs pointer will not be used actually. So the + // Rust warning is a false alarm. We suppress it here. + #[allow(improper_ctypes)] extern "C" { fn __occlum_sysret(user_context: *mut CpuContext) -> !; fn do_exit_task() -> !; @@ -721,20 +728,20 @@ fn do_sysret(user_context: &mut CpuContext) -> ! { if current!().status() != ThreadStatus::Exited { // Restore the floating point registers // Todo: Is it correct to do fxstor in kernel? - let fpregs: *const u8 = user_context.fpregs; - if (fpregs != ptr::null()) { - unsafe { _fxrstor(fpregs) }; - + let fpregs = user_context.fpregs; + if (fpregs != ptr::null_mut()) { if user_context.fpregs_on_heap == 1 { - // Converting the raw pointer back into a Box with Box::from_raw for automatic cleanup - unsafe { - let buf: &mut [u8] = core::slice::from_raw_parts_mut(user_context.fpregs, 512); - let _ = Box::from_raw(buf); - } + let fpregs = unsafe { Box::from_raw(user_context.fpregs as *mut FpRegs) }; + fpregs.restore(); + } else { + unsafe { fpregs.as_ref().unwrap().restore() }; } } unsafe { __occlum_sysret(user_context) } // jump to user space } else { + if user_context.fpregs != ptr::null_mut() && user_context.fpregs_on_heap == 1 { + drop(unsafe { Box::from_raw(user_context.fpregs as *mut FpRegs) }); + } unsafe { do_exit_task() } // exit enclave } unreachable!("__occlum_sysret never returns!"); @@ -888,6 +895,49 @@ fn handle_unsupported() -> Result { return_errno!(ENOSYS, "Unimplemented or unknown syscall") } +/// Floating point registers +/// +/// Note. The area is used to save fxsave result +//#[derive(Clone, Copy)] +#[repr(C)] +pub struct FpRegs { + inner: Aligned, +} + +impl FpRegs { + /// Save the current CPU floating pointer states to an instance of FpRegs + pub fn save() -> Self { + let mut fpregs = FpRegs { + inner: Aligned([0u8; 512]), + }; + unsafe { _fxsave(fpregs.inner.as_mut_ptr()) }; + fpregs + } + + /// Restore the current CPU floating pointer states from this FpRegs instance + pub fn restore(&self) { + unsafe { _fxrstor(self.inner.as_ptr()) }; + } + + /// Construct a FpRegs from a slice of u8. + /// + /// It is up to the caller to ensure that the src slice contains data that + /// is the xsave/xrstor format. + pub unsafe fn from_slice(src: &[u8]) -> Self { + let mut uninit = MaybeUninit::::uninit(); + let dst_buf: &mut [u8] = std::slice::from_raw_parts_mut( + uninit.as_mut_ptr() as *mut u8, + std::mem::size_of::(), + ); + dst_buf.copy_from_slice(&src); + uninit.assume_init() + } + + pub fn as_slice(&self) -> &[u8] { + self.inner.as_ref() + } +} + /// Cpu context. /// /// Note. The definition of this struct must be kept in sync with the assembly @@ -914,7 +964,7 @@ pub struct CpuContext { pub rip: u64, pub rflags: u64, pub fpregs_on_heap: u64, - pub fpregs: *mut u8, + pub fpregs: *mut FpRegs, } impl CpuContext { diff --git a/src/libos/src/syscall/syscall_entry_x86-64.S b/src/libos/src/syscall/syscall_entry_x86-64.S index 54fb770e..c3551248 100644 --- a/src/libos/src/syscall/syscall_entry_x86-64.S +++ b/src/libos/src/syscall/syscall_entry_x86-64.S @@ -28,8 +28,8 @@ __occlum_syscall_linux_abi: // Save the target CPU state when `call __occlum_syscall` is returned in // a CpuContext struct. The registers are saved in the reverse order of // the fields in CpuContext. - push $0 // default fpregs is NULL - push $0 // default fpregs is allocated on stack + pushq $0 // default fpregs is NULL + pushq $0 // default fpregs is allocated on stack pushfq push %rcx // save %rip push %r11 // save %rsp diff --git a/src/libos/src/util/sgx/sgx_attestation_agent.rs b/src/libos/src/util/sgx/sgx_attestation_agent.rs index e11cd01c..387fae25 100644 --- a/src/libos/src/util/sgx/sgx_attestation_agent.rs +++ b/src/libos/src/util/sgx/sgx_attestation_agent.rs @@ -142,7 +142,12 @@ impl InnerAgent { let mut quote_len: u32 = 0; let mut rt = Default::default(); let status = unsafe { - occlum_ocall_sgx_calc_quote_size(&mut rt as _, sigrl_ptr, sigrl_size, &mut quote_len as _) + occlum_ocall_sgx_calc_quote_size( + &mut rt as _, + sigrl_ptr, + sigrl_size, + &mut quote_len as _, + ) }; assert!(status == sgx_status_t::SGX_SUCCESS); if rt != sgx_status_t::SGX_SUCCESS {