From ca4bcbf8fe8e997a7b7e7acfdb45752edc9836f7 Mon Sep 17 00:00:00 2001 From: "Hui, Chunyang" Date: Fri, 20 Oct 2023 08:39:41 +0000 Subject: [PATCH] Use low level API to replace sgx_mm_(commit/commit_data/modify_permissions) Reduce the EMA management overhead and the global lock of emm module of Intel SGX SDK --- src/libos/Cargo.toml | 1 + src/libos/Makefile | 14 +- src/libos/src/vm/shm_manager.rs | 2 +- src/libos/src/vm/vm_area.rs | 66 +++----- src/libos/src/vm/vm_chunk_manager.rs | 13 +- src/libos/src/vm/vm_epc.rs | 245 +++++++++++++++++++++++---- src/libos/src/vm/vm_manager.rs | 13 +- 7 files changed, 269 insertions(+), 85 deletions(-) diff --git a/src/libos/Cargo.toml b/src/libos/Cargo.toml index fa7954b2..f5464f26 100644 --- a/src/libos/Cargo.toml +++ b/src/libos/Cargo.toml @@ -48,6 +48,7 @@ dcap = [] # DCAP support. The compilation relies on DCAP package. cov = ["sgx_cov"] # Enable coverage colletcion. hyper_mode = [] # For running in hyper mode. pku = [] # PKU Support +sim_mode = [] # For running in SGX simulation mode [target.'cfg(not(target_env = "sgx"))'.dependencies] sgx_types = { path = "../../deps/rust-sgx-sdk/sgx_types" } diff --git a/src/libos/Makefile b/src/libos/Makefile index 82fb32bd..d0f2cc9d 100644 --- a/src/libos/Makefile +++ b/src/libos/Makefile @@ -61,7 +61,12 @@ else endif LIBOS_CORE_A := $(OBJ_DIR)/libos/lib/lib$(LIBOS_CORE_LIB_NAME).a -LIBOS_CORE_RS_A := $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs.a + +ifeq ($(SGX_MODE), SIM) + LIBOS_CORE_RS_A := $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs_sim.a +else + LIBOS_CORE_RS_A := $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs.a +endif # All source code RUST_SRCS := $(wildcard src/*.rs src/*/*.rs src/*/*/*.rs src/*/*/*/*.rs src/*/*/*/*/*.rs) @@ -140,20 +145,27 @@ ifeq ($(SGX_MODE), HYPER) LIBOS_FEATURES += hyper_mode endif +ifeq ($(SGX_MODE), SIM) + LIBOS_FEATURES += sim_mode +endif + # Release build is for production use. We enable code coverage only for debug # build. It also simplifies the implementation as the release and debug build # have different output paths. ifeq ($(OCCLUM_RELEASE_BUILD), 1) $(LIBOS_CORE_RS_A): $(RUST_SRCS) @RUSTC_BOOTSTRAP=1 RUSTC_WRAPPER=$(RUSTC_WRAPPER) cargo build --release --target-dir=$(RUST_TARGET_DIR) -Z unstable-options --out-dir=$(RUST_OUT_DIR) --features "$(LIBOS_FEATURES)" + @mv $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs.a $@ || true @echo "CARGO (release) => $@" else ifneq ($(OCCLUM_COV),) $(LIBOS_CORE_RS_A): $(RUST_SRCS) @CARGO_INCREMENTAL=0 RUSTC_BOOTSTRAP=1 RUSTFLAGS=$(COV_FLAGS) cargo build --target-dir=$(RUST_TARGET_DIR) -Z unstable-options --out-dir=$(RUST_OUT_DIR) --features "$(LIBOS_FEATURES)" + @mv $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs.a $@ || true @echo "CARGO (debug + cov) => $@" else $(LIBOS_CORE_RS_A): $(RUST_SRCS) @RUSTC_BOOTSTRAP=1 RUSTC_WRAPPER=$(RUSTC_WRAPPER) cargo build --target-dir=$(RUST_TARGET_DIR) -Z unstable-options --out-dir=$(RUST_OUT_DIR) --features "$(LIBOS_FEATURES)" + @mv $(OBJ_DIR)/libos/lib/libocclum_libos_core_rs.a $@ || true @echo "CARGO (debug) => $@" endif diff --git a/src/libos/src/vm/shm_manager.rs b/src/libos/src/vm/shm_manager.rs index 5e0bb723..37a88efd 100644 --- a/src/libos/src/vm/shm_manager.rs +++ b/src/libos/src/vm/shm_manager.rs @@ -282,6 +282,6 @@ impl ShmManager { return; } vma.set_perms(perms); - vma.modify_permissions_for_committed_pages(perms); + vma.modify_permissions_for_committed_pages(old_perms, perms); } } diff --git a/src/libos/src/vm/vm_area.rs b/src/libos/src/vm/vm_area.rs index d1fc7032..2f720b7b 100644 --- a/src/libos/src/vm/vm_area.rs +++ b/src/libos/src/vm/vm_area.rs @@ -231,7 +231,7 @@ impl VMArea { // Set memory permissions if !options.perms().is_default() { - vm_area.modify_protection_force(None, vm_area.perms()); + vm_area.modify_protection_force(None, VMPerms::DEFAULT, vm_area.perms()); } } // Do nothing if this vma has no committed memory @@ -274,7 +274,7 @@ impl VMArea { debug_assert!(self.range().is_superset_of(target_range)); let buf = unsafe { target_range.as_slice_mut() }; if !self.perms().is_default() { - self.modify_protection_force(Some(&target_range), VMPerms::default()); + self.modify_protection_force(Some(&target_range), self.perms(), VMPerms::default()); } if need_flush { @@ -296,13 +296,17 @@ impl VMArea { } } - pub fn modify_permissions_for_committed_pages(&self, new_perms: VMPerms) { + pub fn modify_permissions_for_committed_pages( + &self, + current_perms: VMPerms, + new_perms: VMPerms, + ) { if self.is_fully_committed() { - self.modify_protection_force(None, new_perms); + self.modify_protection_force(None, current_perms, new_perms); } else if self.is_partially_committed() { let committed = true; for range in self.pages().get_ranges(committed) { - self.modify_protection_force(Some(&range), new_perms); + self.modify_protection_force(Some(&range), current_perms, new_perms); } } } @@ -638,11 +642,21 @@ impl VMArea { // Current implementation with "unwrap()" can help us find the error quickly by panicing directly. Also, restoring VM state // when this function fails will require some work and is not that simple. // TODO: Return with Result instead of "unwrap()"" in this function. - fn modify_protection_force(&self, protect_range: Option<&VMRange>, new_perms: VMPerms) { + fn modify_protection_force( + &self, + protect_range: Option<&VMRange>, + current_perms: VMPerms, + new_perms: VMPerms, + ) { let protect_range = protect_range.unwrap_or_else(|| self.range()); self.epc_type - .modify_protection(protect_range.start(), protect_range.size(), new_perms) + .modify_protection( + protect_range.start(), + protect_range.size(), + current_perms, + new_perms, + ) .unwrap() } @@ -668,7 +682,7 @@ impl VMArea { } VMInitializer::DoNothing() => { if !self.perms().is_default() { - self.modify_protection_force(Some(target_range), perms); + self.modify_protection_force(Some(target_range), VMPerms::DEFAULT, perms); } } VMInitializer::FillZeros() => { @@ -677,7 +691,7 @@ impl VMArea { buf.iter_mut().for_each(|b| *b = 0); } if !perms.is_default() { - self.modify_protection_force(Some(target_range), perms); + self.modify_protection_force(Some(target_range), VMPerms::DEFAULT, perms); } } _ => todo!(), @@ -732,7 +746,7 @@ impl VMArea { .map_err(|_| errno!(EACCES, "failed to init memory from file"))?; if !new_perm.is_default() { - self.modify_protection_force(Some(target_range), new_perm); + self.modify_protection_force(Some(target_range), VMPerms::DEFAULT, new_perm); } Ok(()) @@ -766,8 +780,6 @@ impl VMArea { range.resize(commit_once_size - total_commit_size); } - // We don't take care the file-backed memory here - debug_assert!(self.backed_file().is_none()); self.init_memory_internal(&range, None)?; total_commit_size += range.size(); @@ -787,7 +799,6 @@ impl VMArea { // Only used to handle PF triggered by the kernel fn commit_current_vma_whole(&mut self) -> Result<()> { debug_assert!(!self.is_fully_committed()); - debug_assert!(self.backed_file().is_none()); let mut uncommitted_ranges = self.pages.as_ref().unwrap().get_ranges(false); for range in uncommitted_ranges { @@ -797,35 +808,6 @@ impl VMArea { Ok(()) } - - // TODO: We can re-enable this when we support lazy extend permissions. - #[allow(dead_code)] - fn page_fault_handler_extend_permission(&mut self, pf_addr: usize) -> Result<()> { - let permission = self.perms(); - - // This is intended by the application. - if permission == VMPerms::NONE { - return_errno!(EPERM, "trying to access PROT_NONE memory"); - } - - if self.is_fully_committed() { - self.modify_protection_force(None, permission); - return Ok(()); - } - - let committed = true; - let committed_ranges = self.pages().get_ranges(committed); - for range in committed_ranges.iter() { - if !range.contains(pf_addr) { - continue; - } - - self.epc_type - .modify_protection(range.start(), range.size(), permission)?; - } - - Ok(()) - } } impl Deref for VMArea { diff --git a/src/libos/src/vm/vm_chunk_manager.rs b/src/libos/src/vm/vm_chunk_manager.rs index c8a32024..e140e7cb 100644 --- a/src/libos/src/vm/vm_chunk_manager.rs +++ b/src/libos/src/vm/vm_chunk_manager.rs @@ -293,7 +293,8 @@ impl ChunkManager { if intersection_vma.range() == containing_vma.range() { // The whole containing_vma is mprotected containing_vma.set_perms(new_perms); - containing_vma.modify_permissions_for_committed_pages(containing_vma.perms()); + containing_vma + .modify_permissions_for_committed_pages(old_perms, containing_vma.perms()); containing_vmas.replace_with(VMAObj::new_vma_obj(containing_vma)); containing_vmas.move_next(); continue; @@ -317,7 +318,10 @@ impl ChunkManager { new_perms, VMAccess::Private(current_pid), ); - new_vma.modify_permissions_for_committed_pages(new_vma.perms()); + new_vma.modify_permissions_for_committed_pages( + containing_vma.perms(), + new_vma.perms(), + ); let new_vma = VMAObj::new_vma_obj(new_vma); // Another new VMA @@ -351,7 +355,10 @@ impl ChunkManager { VMAccess::Private(current_pid), ); - new_vma.modify_permissions_for_committed_pages(new_vma.perms()); + new_vma.modify_permissions_for_committed_pages( + containing_vma.perms(), + new_vma.perms(), + ); if remain_vma.start() == containing_vma.start() { // mprotect right side of the vma diff --git a/src/libos/src/vm/vm_epc.rs b/src/libos/src/vm/vm_epc.rs index 2aa47739..3d3e06cd 100644 --- a/src/libos/src/vm/vm_epc.rs +++ b/src/libos/src/vm/vm_epc.rs @@ -69,7 +69,12 @@ pub trait EPCAllocator { return_errno!(ENOSYS, "operation not supported"); } - fn modify_protection(addr: usize, length: usize, protection: VMPerms) -> Result<()> { + fn modify_protection( + addr: usize, + length: usize, + current_protection: VMPerms, + new_protection: VMPerms, + ) -> Result<()> { return_errno!(ENOSYS, "operation not supported"); } @@ -99,11 +104,16 @@ impl EPCAllocator for ReservedMem { Ok(()) } - fn modify_protection(addr: usize, length: usize, protection: VMPerms) -> Result<()> { + fn modify_protection( + addr: usize, + length: usize, + current_protection: VMPerms, + new_protection: VMPerms, + ) -> Result<()> { let mut ret_val = 0; let ret = if rsgx_is_supported_EDMM() { unsafe { - sgx_tprotect_rsrv_mem(addr as *const c_void, length, protection.bits() as i32) + sgx_tprotect_rsrv_mem(addr as *const c_void, length, new_protection.bits() as i32) } } else { // For platforms without EDMM, sgx_tprotect_rsrv_mem is actually useless. @@ -113,12 +123,13 @@ impl EPCAllocator for ReservedMem { &mut ret_val as *mut i32, addr as *const c_void, length, - protection.bits() as i32, + new_protection.bits() as i32, ) } }; if ret != sgx_status_t::SGX_SUCCESS || ret_val != 0 { + error!("ocall ret = {:?}, ret_val = {:?}", ret, ret_val); return_errno!(ENOMEM, "reserved memory modify protection failure"); } @@ -147,17 +158,26 @@ impl EPCAllocator for UserRegionMem { Ok(()) } - fn modify_protection(addr: usize, length: usize, protection: VMPerms) -> Result<()> { + fn modify_protection( + addr: usize, + length: usize, + current_protection: VMPerms, + new_protection: VMPerms, + ) -> Result<()> { trace!( "user region modify protection, protection = {:?}, range = {:?}", - protection, + new_protection, VMRange::new_with_size(addr, length).unwrap() ); - let ptr = NonNull::::new(addr as *mut u8).unwrap(); - unsafe { - EmmAlloc.modify_permissions(ptr, length, Perm::from_bits(protection.bits()).unwrap()) + + // Simulation mode doesn't have the symbol used here + #[cfg(not(feature = "sim_mode"))] + { + EDMMLocalApi::modify_permissions(addr, length, current_protection, new_protection)?; } - .map_err(|e| errno!(Errno::from(e as u32)))?; + + #[cfg(feature = "sim_mode")] + unreachable!(); Ok(()) } @@ -169,8 +189,12 @@ impl EPCAllocator for UserRegionMem { impl UserRegionMem { fn commit_memory(start_addr: usize, size: usize) -> Result<()> { - let ptr = NonNull::::new(start_addr as *mut u8).unwrap(); - unsafe { EmmAlloc.commit(ptr, size) }.map_err(|e| errno!(Errno::from(e as u32)))?; + #[cfg(not(feature = "sim_mode"))] + EDMMLocalApi::commit_memory(start_addr, size)?; + + #[cfg(feature = "sim_mode")] + unreachable!(); + Ok(()) } @@ -179,16 +203,19 @@ impl UserRegionMem { size: usize, new_perms: VMPerms, ) -> Result<()> { - let ptr = NonNull::::new(start_addr as *mut u8).unwrap(); - let perm = Perm::from_bits(new_perms.bits()).unwrap(); - if size == PAGE_SIZE { - unsafe { EmmAlloc::commit_with_data(ptr, ZERO_PAGE.as_slice(), perm) } - .map_err(|e| errno!(Errno::from(e as u32)))?; - } else { - let data = ZeroPage::new_page_aligned_vec(size); - unsafe { EmmAlloc::commit_with_data(ptr, data.as_slice(), perm) } - .map_err(|e| errno!(Errno::from(e as u32)))?; + #[cfg(not(feature = "sim_mode"))] + { + if size == PAGE_SIZE { + EDMMLocalApi::commit_with_data(start_addr, ZERO_PAGE.as_slice(), new_perms)?; + } else { + let data = ZeroPage::new_page_aligned_vec(size); + EDMMLocalApi::commit_with_data(start_addr, data.as_slice(), new_perms)?; + } } + + #[cfg(feature = "sim_mode")] + unreachable!(); + Ok(()) } @@ -199,16 +226,19 @@ impl UserRegionMem { file_offset: usize, new_perms: VMPerms, ) -> Result<()> { - let mut data = ZeroPage::new_page_aligned_vec(size); - let len = file - .read_at(file_offset, data.as_mut_slice()) - .map_err(|_| errno!(EACCES, "failed to init memory from file"))?; + #[cfg(not(feature = "sim_mode"))] + { + let mut data = ZeroPage::new_page_aligned_vec(size); + let len = file + .read_at(file_offset, data.as_mut_slice()) + .map_err(|_| errno!(EACCES, "failed to init memory from file"))?; - let ptr = NonNull::::new(start_addr as *mut u8).unwrap(); - let perm = Perm::from_bits(new_perms.bits()).unwrap(); + EDMMLocalApi::commit_with_data(start_addr, data.as_slice(), new_perms)?; + } + + #[cfg(feature = "sim_mode")] + unreachable!(); - unsafe { EmmAlloc::commit_with_data(ptr, data.as_slice(), perm) } - .map_err(|e| errno!(Errno::from(e as u32)))?; Ok(()) } } @@ -324,21 +354,33 @@ impl EPCMemType { } } - pub fn modify_protection(&self, addr: usize, length: usize, protection: VMPerms) -> Result<()> { + pub fn modify_protection( + &self, + addr: usize, + length: usize, + current_protection: VMPerms, + new_protection: VMPerms, + ) -> Result<()> { // PT_GROWSDOWN should only be applied to stack segment or a segment mapped with the MAP_GROWSDOWN flag set. // Since the memory are managed by our own, mprotect ocall shouldn't use this flag. Otherwise, EINVAL will be thrown. - let mut prot = protection.clone(); + let mut prot = new_protection; + let mut current_prot = current_protection; prot.remove(VMPerms::GROWSDOWN); + current_prot.remove(VMPerms::GROWSDOWN); match self { - EPCMemType::Reserved => ReservedMem::modify_protection(addr, length, prot), - EPCMemType::UserRegion => UserRegionMem::modify_protection(addr, length, prot), + EPCMemType::Reserved => { + ReservedMem::modify_protection(addr, length, current_prot, prot) + } + EPCMemType::UserRegion => { + UserRegionMem::modify_protection(addr, length, current_prot, prot) + } } } } pub fn commit_memory(start_addr: usize, size: usize, new_perms: Option) -> Result<()> { - trace!( + debug!( "commit epc: {:?}, new permission: {:?}", VMRange::new_with_size(start_addr, size).unwrap(), new_perms @@ -397,4 +439,139 @@ extern "C" { len: usize, prot: i32, ) -> sgx_status_t; + + fn sgx_mm_modify_ocall(addr: usize, size: usize, flags_from: i32, flags_to: i32) -> i32; + + // EACCEPT + fn do_eaccept(si: *const sec_info_t, addr: usize) -> i32; + + // EMODPE + fn do_emodpe(si: *const sec_info_t, addr: usize) -> i32; + + // EACCEPTCOPY + fn do_eacceptcopy(si: *const sec_info_t, dest: usize, src: usize) -> i32; +} + +#[allow(non_camel_case_types)] +#[repr(C, align(512))] +struct sec_info_t { + flags: u64, + reserved: [u64; 7], +} + +impl sec_info_t { + const SGX_EMA_STATE_PENDING: u64 = 0x08; // pending state + const SGX_EMA_STATE_PR: u64 = 0x20; // permission restriction state + + fn new_for_modify_permission(new_protection: &VMPerms) -> Self { + Self { + flags: ((new_protection.bits() | SGX_EMA_PAGE_TYPE_REG) as u64) + | Self::SGX_EMA_STATE_PR, + reserved: [0; 7], + } + } + + fn new_for_commit_memory() -> Self { + Self { + flags: ((VMPerms::DEFAULT.bits() | SGX_EMA_PAGE_TYPE_REG) as u64) + | Self::SGX_EMA_STATE_PENDING, + reserved: [0; 7], + } + } + + fn new_for_commit_with_data(protection: &VMPerms) -> Self { + Self { + flags: (protection.bits() | SGX_EMA_PAGE_TYPE_REG) as u64, + reserved: [0; 7], + } + } +} + +#[cfg(not(feature = "sim_mode"))] +struct EDMMLocalApi; + +#[cfg(not(feature = "sim_mode"))] +impl EDMMLocalApi { + // To replace sgx_mm_commit + fn commit_memory(start_addr: usize, size: usize) -> Result<()> { + let si = sec_info_t::new_for_commit_memory(); + for page in (start_addr..start_addr + size).step_by(PAGE_SIZE) { + let ret = unsafe { do_eaccept(&si as *const sec_info_t, page) }; + if ret != 0 { + return_errno!(EFAULT, "do_eaccept failure"); + } + } + Ok(()) + } + + // To replace sgx_mm_commit_data + fn commit_with_data(addr: usize, data: &[u8], perm: VMPerms) -> Result<()> { + let si = sec_info_t::new_for_commit_with_data(&perm); + let size = data.len(); + let mut src_raw_ptr = data.as_ptr() as usize; + for dest_page in (addr..addr + size).step_by(PAGE_SIZE) { + let ret = unsafe { do_eacceptcopy(&si as *const sec_info_t, dest_page, src_raw_ptr) }; + if ret != 0 { + return_errno!(EFAULT, "do_eacceptcopy failure"); + } + + Self::modify_permissions(dest_page, PAGE_SIZE, VMPerms::DEFAULT, perm)?; + src_raw_ptr += PAGE_SIZE; + } + + Ok(()) + } + + // To replace sgx_mm_modify_permissions + fn modify_permissions( + addr: usize, + length: usize, + current_protection: VMPerms, + new_protection: VMPerms, + ) -> Result<()> { + if current_protection == new_protection { + return Ok(()); + } + + let flags_from = current_protection.bits() | SGX_EMA_PAGE_TYPE_REG; + let flags_to = new_protection.bits() | SGX_EMA_PAGE_TYPE_REG; + let ret = unsafe { sgx_mm_modify_ocall(addr, length, flags_from as i32, flags_to as i32) }; + if ret != 0 { + return_errno!(EFAULT, "sgx_mm_modify_ocall failure"); + } + + let si = sec_info_t::new_for_modify_permission(&new_protection); + for page in (addr..addr + length).step_by(PAGE_SIZE) { + debug_assert!(page % PAGE_SIZE == 0); + + if new_protection.bits() | current_protection.bits() != current_protection.bits() { + unsafe { do_emodpe(&si as *const sec_info_t, page) }; + // Check this return value is useless. RAX is set to SE_EMODPE which is 6 defined in SDK. + } + // If new permission is RWX, no EMODPR needed in untrusted part, hence no EACCEPT + if new_protection != VMPerms::ALL { + let ret = unsafe { do_eaccept(&si, page) }; + if ret != 0 { + return_errno!(EFAULT, "do_eaccept failure"); + } + } + } + + // ??? + if new_protection == VMPerms::NONE { + let ret = unsafe { + sgx_mm_modify_ocall( + addr, + length, + (SGX_EMA_PAGE_TYPE_REG | SGX_EMA_PROT_NONE) as i32, + (SGX_EMA_PAGE_TYPE_REG | SGX_EMA_PROT_NONE) as i32, + ) + }; + if ret != 0 { + return_errno!(EFAULT, "sgx_mm_modify_ocall failure for permission None"); + } + } + + Ok(()) + } } diff --git a/src/libos/src/vm/vm_manager.rs b/src/libos/src/vm/vm_manager.rs index 8342a50e..eace3c71 100644 --- a/src/libos/src/vm/vm_manager.rs +++ b/src/libos/src/vm/vm_manager.rs @@ -778,7 +778,10 @@ impl InternalVMManager { let mut vma = new_chunk.get_vma_for_single_vma_chunk(); // Reset memory permissions if !vma.perms().is_default() { - vma.modify_permissions_for_committed_pages(VMPerms::default()) + vma.modify_permissions_for_committed_pages( + vma.perms(), + VMPerms::default(), + ) } // Reset memory contents unsafe { @@ -926,7 +929,7 @@ impl InternalVMManager { (true, true) => { // Exact the same vma containing_vma.set_perms(new_perms); - containing_vma.modify_permissions_for_committed_pages(new_perms); + containing_vma.modify_permissions_for_committed_pages(old_perms, new_perms); return Ok(()); } (false, false) => { @@ -942,7 +945,8 @@ impl InternalVMManager { new_perms, VMAccess::Private(current_pid), ); - new_vma.modify_permissions_for_committed_pages(new_perms); + new_vma + .modify_permissions_for_committed_pages(containing_vma.perms(), new_perms); let remaining_old_vma = { let range = VMRange::new(protect_range.end(), old_end).unwrap(); @@ -966,7 +970,8 @@ impl InternalVMManager { new_perms, VMAccess::Private(current_pid), ); - new_vma.modify_permissions_for_committed_pages(new_perms); + new_vma + .modify_permissions_for_committed_pages(containing_vma.perms(), new_perms); if same_start { // Protect range is at left side of the containing vma