general/occlum
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a potential memory issue in fpregs' free

Browse Source
This commit is contained in:
zongmin.gu 2020-08-19 13:22:39 +08:00 committed by tate.thl
parent 89c292e2df
commit 85f5bc7ccc
6 changed files with 108 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/libos/src/exception/mod.rs
View File

@ -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<A16, _> = 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<isize> {
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);

21
src/libos/src/interrupt/mod.rs
View File

@ -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<A16, _> = 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<isize> {
let info = unsafe { &*info };

18
src/libos/src/signal/do_sigreturn.rs
View File

@ -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<Aligned<A16, [u8]>> = 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<A16, _> = 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

76
src/libos/src/syscall/mod.rs
View File

@ -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<isize> {
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<A16, [u8; 512]>,
}
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::<Self>::uninit();
let dst_buf: &mut [u8] = std::slice::from_raw_parts_mut(
uninit.as_mut_ptr() as *mut u8,
std::mem::size_of::<FpRegs>(),
);
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 {

4
src/libos/src/syscall/syscall_entry_x86-64.S
View File

@ -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

7
src/libos/src/util/sgx/sgx_attestation_agent.rs
View File

@ -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 {