diff --git a/.github/workflows/dep_build_guests.yml b/.github/workflows/dep_build_guests.yml index d26ea63d7..40662ede0 100644 --- a/.github/workflows/dep_build_guests.yml +++ b/.github/workflows/dep_build_guests.yml @@ -87,6 +87,11 @@ jobs: just build-rust-guests ${{ inputs.config }} just move-rust-guests ${{ inputs.config }} + - name: Build non-PIE Rust guests + run: | + just build-rust-guests-non-pie ${{ inputs.config }} + just move-rust-guests-non-pie ${{ inputs.config }} + - name: Build C guests run: | just build-c-guests ${{ inputs.config }} diff --git a/.gitignore b/.gitignore index 3aae7b792..27126b8de 100644 --- a/.gitignore +++ b/.gitignore @@ -454,6 +454,7 @@ $RECYCLE.BIN/ # Rust build artifacts **/**target +**/**target-non-pie libhyperlight_host.so libhyperlight_host.d hyperlight_host.dll diff --git a/Justfile b/Justfile index 1eb4dd3a0..798df0ca2 100644 --- a/Justfile +++ b/Justfile @@ -50,7 +50,7 @@ build target=default-target: {{ cargo-cmd }} build --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} # build testing guest binaries -guests: build-and-move-rust-guests build-and-move-c-guests +guests: build-and-move-rust-guests build-and-move-rust-guests-non-pie build-and-move-c-guests # Ensure the pinned cargo-hyperlight is installed. We compare the *actual* # installed binary's reported version instead of relying on `cargo install` @@ -75,6 +75,23 @@ build-rust-guests target=default-target features="": (ensure-cargo-hyperlight) build-and-move-rust-guests: (build-rust-guests "debug") (move-rust-guests "debug") (build-rust-guests "release") (move-rust-guests "release") build-and-move-c-guests: (build-c-guests "debug") (move-c-guests "debug") (build-c-guests "release") (move-c-guests "release") +# Build non-PIE variants of rust guests for testing ELF VA mapping. +# NOTE: non-PIE guests are x86_64-only; aarch64 is not yet supported. +# Phase 1 builds the sysroot without RUSTFLAGS (avoids RUSTFLAGS leaking +# into the sysroot wrapper build in cargo-hyperlight). +# Phase 2 uses plain cargo with --sysroot and non-PIE link flags. +build-rust-guests-non-pie target=default-target: (ensure-cargo-hyperlight) + cd src/tests/rust_guests/simpleguest && cargo hyperlight build --target-dir ../target-non-pie --profile={{ if target == "debug" { "dev" } else { target } }} + {{ if os() == "windows" { "$env:RUSTC_BOOTSTRAP=1; $env:RUSTFLAGS='--sysroot=' + (Resolve-Path src/tests/rust_guests/target-non-pie/sysroot).Path + ' -C relocation-model=static -C link-args=--no-pie -C link-args=--image-base=0x1000000 --cfg=hyperlight --check-cfg=cfg(hyperlight) -Clink-args=-eentrypoint';" } else { "" } }} cd src/tests/rust_guests/simpleguest && {{ if os() == "windows" { "" } else { "RUSTC_BOOTSTRAP=1 RUSTFLAGS=\"--sysroot=$(cd .. && pwd)/target-non-pie/sysroot -C relocation-model=static -C link-args=--no-pie -C link-args=--image-base=0x1000000 --cfg=hyperlight --check-cfg=cfg(hyperlight) -Clink-args=-eentrypoint\"" } }} cargo build --target x86_64-hyperlight-none --target-dir ../target-non-pie/build --profile={{ if target == "debug" { "dev" } else { target } }} + +non_pie_guests_target := "src/tests/rust_guests/target-non-pie/build/x86_64-hyperlight-none" + +@move-rust-guests-non-pie target=default-target: + {{ if os() == "windows" { "New-Item -ItemType Directory -Path " + rust_guests_bin_dir + "/" + target + "/non_pie -Force | Out-Null" } else { "mkdir -p " + rust_guests_bin_dir + "/" + target + "/non_pie" } }} + cp {{ non_pie_guests_target }}/{{ target }}/simpleguest {{ rust_guests_bin_dir }}/{{ target }}/non_pie/ + +build-and-move-rust-guests-non-pie: (build-rust-guests-non-pie "debug") (move-rust-guests-non-pie "debug") (build-rust-guests-non-pie "release") (move-rust-guests-non-pie "release") + clean: clean-rust clean-rust: diff --git a/src/hyperlight_host/src/hypervisor/crashdump.rs b/src/hyperlight_host/src/hypervisor/crashdump.rs index b821fba5f..1dbcdbb85 100644 --- a/src/hyperlight_host/src/hypervisor/crashdump.rs +++ b/src/hyperlight_host/src/hypervisor/crashdump.rs @@ -476,6 +476,7 @@ mod test { let ptr = dummy_vec.as_ptr() as usize; let regions = vec![CrashDumpRegion { guest_region: 0x1000..0x2000, + guest_virt_addr: 0x1000, host_region: ptr..ptr + dummy_vec.len(), flags: MemoryRegionFlags::READ | MemoryRegionFlags::WRITE, region_type: crate::mem::memory_region::MemoryRegionType::Code, diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 0a0685f71..84bbc94c7 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -426,6 +426,7 @@ mod tests { guest_mmap_regions: vec![MemoryRegion { host_region: mapped_mem as usize..mapped_mem.wrapping_add(size) as usize, guest_region: BASE_VIRT..BASE_VIRT + size, + guest_virt_addr: BASE_VIRT, flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE, region_type: MemoryRegionType::Heap, }], diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index ae17e100b..e0033c002 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -675,10 +675,9 @@ pub(super) mod debug { .dbg_mem_access_fn .try_lock() .map_err(|_| ProcessDebugRequestError::TryLockError(file!(), line!()))? - .layout - .get_guest_code_address(); + .code_virt_base; - Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) + Ok(DebugResponse::GetCodeSectionOffset(offset)) } DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; diff --git a/src/hyperlight_host/src/mem/elf.rs b/src/hyperlight_host/src/mem/elf.rs index 7309412f1..aba4e6cb2 100644 --- a/src/hyperlight_host/src/mem/elf.rs +++ b/src/hyperlight_host/src/mem/elf.rs @@ -17,6 +17,7 @@ limitations under the License. #[cfg(feature = "mem_profile")] use std::sync::Arc; +use goblin::elf::header::ET_DYN; #[cfg(target_arch = "aarch64")] use goblin::elf::reloc::{R_AARCH64_NONE, R_AARCH64_RELATIVE}; #[cfg(target_arch = "x86_64")] @@ -42,6 +43,8 @@ pub(crate) struct ElfInfo { shdrs: Vec, entry: u64, relocs: Vec, + /// Whether this is a position-independent executable (ET_DYN). + is_pie: bool, /// The hyperlight version string embedded by `hyperlight-guest-bin`, if /// present. Used to detect version/ABI mismatches between guest and host. guest_bin_version: Option, @@ -143,6 +146,7 @@ impl ElfInfo { .collect(), entry: elf.entry, relocs, + is_pie: elf.header.e_type == ET_DYN, guest_bin_version, }) } @@ -168,6 +172,11 @@ impl ElfInfo { self.entry } + /// Returns whether this is a position-independent executable (ET_DYN). + pub(crate) fn is_pie(&self) -> bool { + self.is_pie + } + /// Returns the hyperlight version string embedded in the guest binary, if /// present. Used to detect version/ABI mismatches between guest and host. pub(crate) fn guest_bin_version(&self) -> Option<&str> { diff --git a/src/hyperlight_host/src/mem/exe.rs b/src/hyperlight_host/src/mem/exe.rs index 97874ae6e..7649846ce 100644 --- a/src/hyperlight_host/src/mem/exe.rs +++ b/src/hyperlight_host/src/mem/exe.rs @@ -88,6 +88,12 @@ impl ExeInfo { ExeInfo::Elf(elf) => Offset::from(elf.entrypoint_va()), } } + /// Returns whether this is a position-independent executable (ET_DYN). + pub fn is_pie(&self) -> bool { + match self { + ExeInfo::Elf(elf) => elf.is_pie(), + } + } /// Returns the base virtual address of the loaded binary (lowest PT_LOAD p_vaddr). pub fn base_va(&self) -> u64 { match self { diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 6422b9b11..d54be1bfd 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -66,10 +66,10 @@ use std::mem::size_of; use hyperlight_common::mem::{HyperlightPEB, PAGE_SIZE_USIZE}; use tracing::{Span, instrument}; -use super::memory_region::MemoryRegionType::{Code, Heap, InitData, Peb}; +use super::memory_region::MemoryRegionType::{self, Code, Heap, InitData, Peb}; use super::memory_region::{ - DEFAULT_GUEST_BLOB_MEM_FLAGS, MemoryRegion, MemoryRegion_, MemoryRegionFlags, MemoryRegionKind, - MemoryRegionVecBuilder, + DEFAULT_GUEST_BLOB_MEM_FLAGS, GuestMemoryRegion, MemoryRegion, MemoryRegion_, + MemoryRegionFlags, MemoryRegionKind, MemoryRegionVecBuilder, }; #[cfg(readable_shared_mem)] use super::shared_mem::HostSharedMemory; @@ -555,6 +555,95 @@ impl SandboxMemoryLayout { Ok(builder.build()) } + /// Compute the virtual base address for the code region, validate + /// that it does not overlap any other memory region, and return the + /// guest memory regions with the Code region's `guest_virt_addr` + /// already set to the computed virtual base. + /// + /// For PIE binaries, a random page-aligned address is chosen within + /// 47-bit canonical user space (ASLR). For non-PIE binaries, the + /// code appears at the ELF's declared virtual address (`elf_base_va`). + /// + /// In both cases the resulting virtual range is validated against all + /// non-Code memory regions to prevent overlap. + /// + /// Returns `(code_virt_base, regions)`. + pub(crate) fn get_guest_regions_with_code_va( + &self, + is_pie: bool, + elf_base_va: u64, + loaded_size: u64, + ) -> Result<(u64, Vec>)> { + let code_size_pages = loaded_size.div_ceil(PAGE_SIZE_USIZE as u64); + let code_virt_base = if !is_pie { + elf_base_va + } else { + // Pick a random page-aligned address within 47-bit canonical user space. + // Lower bound: 0x1000000 (16 MiB, above all identity-mapped layout regions) + // Upper bound: accounts for code region size so it doesn't overflow + use rand::RngExt; + let mut rng = rand::rng(); + let min_page = 0x1000_u64; // 0x1000 * PAGE_SIZE = 0x1000000 + let max_page = 0x7_FFFF_FFFF_u64 + .checked_sub(code_size_pages) + .ok_or_else(|| { + new_error!( + "PIE code region too large ({} pages) for ASLR randomization", + code_size_pages + ) + })?; + let page_number = rng.random_range(min_page..max_page); + page_number + .checked_mul(PAGE_SIZE_USIZE as u64) + .ok_or_else(|| new_error!("ASLR page number overflow"))? + }; + + let mut regions = self.get_memory_regions_::(())?; + + // Verify the code mapping does not conflict with other mappings + // (both non-PIE with declared VA and PIE with randomized ASLR base). + let code_virt_end = code_virt_base.checked_add(loaded_size).ok_or_else(|| { + new_error!( + "Code mapping overflow: base {:#x} + size {:#x}", + code_virt_base, + loaded_size + ) + })?; + for rgn in regions.iter() { + if rgn.region_type == MemoryRegionType::Code { + continue; + } + let rgn_start = rgn.guest_region.start as u64; + let rgn_end = rgn_start.saturating_add(rgn.guest_region.len() as u64); + if code_virt_base < rgn_end && rgn_start < code_virt_end { + return Err(new_error!( + "Code mapping [{:#x}, {:#x}) conflicts with {:?} region [{:#x}, {:#x})", + code_virt_base, + code_virt_end, + rgn.region_type, + rgn_start, + rgn_end, + )); + } + } + + // Set the Code region's guest_virt_addr to code_virt_base. + for rgn in regions.iter_mut() { + if rgn.region_type == MemoryRegionType::Code { + rgn.guest_virt_addr = code_virt_base as usize; + } + } + + tracing::debug!( + code_virt_base = format_args!("{:#x}", code_virt_base), + elf_base_va = format_args!("{:#x}", elf_base_va), + is_pie, + "code region virtual base address" + ); + + Ok((code_virt_base, regions)) + } + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn write_init_data(&self, out: &mut [u8], bytes: &[u8]) -> Result<()> { out[self.init_data_offset()..self.init_data_offset() + self.init_data_size] @@ -678,6 +767,9 @@ impl SandboxMemoryLayout { } /// Guest address of the code section in the sandbox. + /// Used by WHP (Windows) and mem_profile feature; not called on + /// minimal Linux feature sets, hence the allow. + #[allow(dead_code)] pub(crate) fn get_guest_code_address(&self) -> usize { Self::BASE_ADDRESS + self.guest_code_offset() } diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 5d839647a..a17dffd27 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -291,8 +291,12 @@ impl MemoryRegionKind for GuestMemoryRegion { /// the same memory permissions #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct MemoryRegion_ { - /// the range of guest memory addresses + /// the range of guest physical addresses pub guest_region: Range, + /// the guest virtual address at which this region should be mapped. + /// For identity-mapped regions this equals `guest_region.start`. + /// For non-PIE code it is the ELF's declared virtual address. + pub guest_virt_addr: usize, /// the range of host memory addresses /// /// Note that Range<()> = () x () = (). @@ -356,6 +360,7 @@ impl MemoryRegionVecBuilder { let host_end = ::add(self.host_base_virt_addr, size); self.regions.push(MemoryRegion_ { guest_region: self.guest_base_phys_addr..guest_end, + guest_virt_addr: self.guest_base_phys_addr, host_region: self.host_base_virt_addr..host_end, flags, region_type, @@ -367,8 +372,10 @@ impl MemoryRegionVecBuilder { // we know this is safe because we check if the regions are empty above let last_region = self.regions.last().unwrap(); let host_end = ::add(last_region.host_region.end, size); + let guest_start = last_region.guest_region.end; let new_region = MemoryRegion_ { - guest_region: last_region.guest_region.end..last_region.guest_region.end + size, + guest_region: guest_start..guest_start + size, + guest_virt_addr: guest_start, host_region: last_region.host_region.end..host_end, flags, region_type, diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 7c74fea91..e0f0d44d5 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -147,6 +147,10 @@ pub(crate) struct SandboxMemoryManager { /// preserved across the `Initialise` -> `Call` transition so it /// can fill `AT_ENTRY` in guest core dumps. 0 if unknown. pub(crate) original_entrypoint: u64, + /// Virtual base address of the code region. + /// For PIE binaries this equals the physical load address (identity-mapped). + /// For non-PIE binaries this is the ELF-declared base VA. + pub(crate) code_virt_base: u64, /// Buffer for accumulating guest abort messages pub(crate) abort_buffer: Vec, /// Generation counter: how many snapshots have been taken from @@ -288,6 +292,7 @@ where scratch_mem, next_action, original_entrypoint: 0, + code_virt_base: 0, abort_buffer: Vec::new(), snapshot_count: 0, } @@ -320,6 +325,7 @@ where rsp_gva, sregs, next_action, + self.code_virt_base, self.original_entrypoint, self.snapshot_count, host_functions, @@ -335,6 +341,7 @@ impl SandboxMemoryManager { let next_action = s.next_action(); let mut mgr = Self::new(layout, shared_mem, scratch_mem, next_action); mgr.original_entrypoint = s.original_entrypoint(); + mgr.code_virt_base = s.code_virt_base; // Inherit the snapshot's generation number for the same // reason `restore_snapshot` does: the guest-visible counter // reflects "which snapshot is the sandbox currently a clone @@ -367,6 +374,7 @@ impl SandboxMemoryManager { layout: self.layout, next_action: self.next_action, original_entrypoint: self.original_entrypoint, + code_virt_base: self.code_virt_base, abort_buffer: self.abort_buffer, snapshot_count: self.snapshot_count, }; @@ -376,6 +384,7 @@ impl SandboxMemoryManager { layout: self.layout, next_action: self.next_action, original_entrypoint: self.original_entrypoint, + code_virt_base: self.code_virt_base, abort_buffer: Vec::new(), // Guest doesn't need abort buffer snapshot_count: self.snapshot_count, }; @@ -516,6 +525,7 @@ impl SandboxMemoryManager { // Carry the guest ELF entry point across restore so crashdumps // report the restored image's entry. self.original_entrypoint = snapshot.original_entrypoint(); + self.code_virt_base = snapshot.code_virt_base; self.update_scratch_bookkeeping()?; Ok((gsnapshot, gscratch)) @@ -637,6 +647,7 @@ impl SandboxMemoryManager { regions.push(CrashDumpRegion { guest_region: virt_base..virt_end, + guest_virt_addr: virt_base, host_region: host_base..host_base + host_len, flags, region_type, diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index 4b706cc41..e47c4380a 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -514,6 +514,7 @@ fn mapping_at( MemoryRegion { guest_region: guest_base..(guest_base + size), + guest_virt_addr: guest_base, host_region: s.host_region_base() ..::add(s.host_region_base(), size), region_type, diff --git a/src/hyperlight_host/src/sandbox/file_mapping.rs b/src/hyperlight_host/src/sandbox/file_mapping.rs index 4f3ed2a4d..d0e367074 100644 --- a/src/hyperlight_host/src/sandbox/file_mapping.rs +++ b/src/hyperlight_host/src/sandbox/file_mapping.rs @@ -164,6 +164,7 @@ impl PreparedFileMapping { Ok(MemoryRegion { host_region: host_base..host_end, guest_region: guest_start..guest_end, + guest_virt_addr: guest_start, flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE, region_type: MemoryRegionType::MappedFile, }) @@ -184,6 +185,7 @@ impl PreparedFileMapping { host_region: *mmap_base as usize ..(*mmap_base as usize).wrapping_add(*mmap_size), guest_region: guest_start..guest_end, + guest_virt_addr: guest_start, flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE, region_type: MemoryRegionType::MappedFile, }) diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 8e60c68e7..4ab8ae3d0 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -1553,6 +1553,7 @@ mod tests { MemoryRegion { host_region: mem.host_region_base()..mem.host_region_end(), guest_region: guest_base..(guest_base + len), + guest_virt_addr: guest_base, flags, region_type: MemoryRegionType::Heap, } @@ -1976,9 +1977,12 @@ mod tests { /// `read_guest_memory_by_gva`, then assert both views are identical. #[cfg(feature = "trace_guest")] fn assert_gva_read_matches(sbox: &mut MultiUseSandbox, gva: u64, len: usize) { - // Guest reads via its own page tables + // Guest reads via its own page tables. + // do_map = false: the code region is already mapped (identity-mapped + // or ASLR-mapped), so we must not remap it with an identity mapping + // that would use the GVA as a physical address. let expected: Vec = sbox - .call("ReadMappedBuffer", (gva, len as u64, true)) + .call("ReadMappedBuffer", (gva, len as u64, false)) .unwrap(); assert_eq!(expected.len(), len); @@ -2002,7 +2006,7 @@ mod tests { #[cfg(feature = "trace_guest")] fn read_guest_memory_by_gva_single_page() { let mut sbox = sandbox_for_gva_tests(); - let code_gva = sbox.mem_mgr.layout.get_guest_code_address() as u64; + let code_gva = sbox.mem_mgr.code_virt_base; assert_gva_read_matches(&mut sbox, code_gva, 128); } @@ -2012,7 +2016,7 @@ mod tests { #[cfg(feature = "trace_guest")] fn read_guest_memory_by_gva_full_page() { let mut sbox = sandbox_for_gva_tests(); - let code_gva = sbox.mem_mgr.layout.get_guest_code_address() as u64; + let code_gva = sbox.mem_mgr.code_virt_base; assert_gva_read_matches(&mut sbox, code_gva, 4096); } @@ -2022,7 +2026,7 @@ mod tests { #[cfg(feature = "trace_guest")] fn read_guest_memory_by_gva_unaligned_cross_page() { let mut sbox = sandbox_for_gva_tests(); - let code_gva = sbox.mem_mgr.layout.get_guest_code_address() as u64; + let code_gva = sbox.mem_mgr.code_virt_base; // Start 1 byte before the second page boundary and read 4097 bytes // (spans 2 full page boundaries). let start = code_gva + 4096 - 1; @@ -2038,7 +2042,7 @@ mod tests { #[cfg(feature = "trace_guest")] fn read_guest_memory_by_gva_two_full_pages() { let mut sbox = sandbox_for_gva_tests(); - let code_gva = sbox.mem_mgr.layout.get_guest_code_address() as u64; + let code_gva = sbox.mem_mgr.code_virt_base; assert_gva_read_matches(&mut sbox, code_gva, 4096 * 2); } @@ -2049,7 +2053,7 @@ mod tests { #[cfg(feature = "trace_guest")] fn read_guest_memory_by_gva_cross_page_boundary() { let mut sbox = sandbox_for_gva_tests(); - let code_gva = sbox.mem_mgr.layout.get_guest_code_address() as u64; + let code_gva = sbox.mem_mgr.code_virt_base; // Start 100 bytes before the first page boundary, read across it. let start = code_gva + 4096 - 100; assert_gva_read_matches(&mut sbox, start, 200); diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 5e9fe6c02..2af1845af 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -464,41 +464,31 @@ impl OciSnapshotConfig { } } - // Entrypoint address must point inside the guest snapshot - // region `[BASE_ADDRESS, BASE_ADDRESS + snapshot_size)`. The - // address is a GVA, bounded by the same range because guests - // identity-map the snapshot region at low VAs. A guest - // dispatching from a non-identity-mapped VA must relax this - // check. - let snap_lo = SandboxMemoryLayout::BASE_ADDRESS as u64; - let snap_hi = snap_lo - .checked_add(self.layout.snapshot_size as u64) - .ok_or_else(|| { - crate::new_error!( - "snapshot layout overflow: BASE_ADDRESS + snapshot_size ({}) does not fit in u64", - self.layout.snapshot_size - ) - })?; - if self.entrypoint_addr < snap_lo || self.entrypoint_addr >= snap_hi { + // Validate the entrypoint GVA. + // `entrypoint_addr` is a GVA loaded into RIP. For ASLR or non-PIE + // guests the code may be mapped at a non-identity VA, so we only + // require the address is non-zero and within the 47-bit canonical + // user-space range. + let max_gva_entrypoint = 0x7FFF_FFFF_FFFFu64; // 47-bit canonical + if self.entrypoint_addr == 0 || self.entrypoint_addr > max_gva_entrypoint { return Err(crate::new_error!( - "snapshot entrypoint addr {:#x} is outside the snapshot region [{:#x}, {:#x})", + "snapshot entrypoint addr {:#x} is outside the valid GVA range (0, {:#x}]", self.entrypoint_addr, - snap_lo, - snap_hi + max_gva_entrypoint )); } // ELF entry point GVA for `AT_ENTRY` in core dumps. 0 means - // unknown. Any other value must point inside the snapshot - // region, like `entrypoint_addr`. - if self.original_entrypoint_addr != 0 - && (self.original_entrypoint_addr < snap_lo || self.original_entrypoint_addr >= snap_hi) + // unknown. Any other value must be within the 47-bit canonical + // user-space range (same as entrypoint_addr), since with ASLR + // or non-PIE the entry point may not be inside the snapshot + // physical region. + if self.original_entrypoint_addr != 0 && self.original_entrypoint_addr > max_gva_entrypoint { return Err(crate::new_error!( - "snapshot original entrypoint addr {:#x} is outside the snapshot region [{:#x}, {:#x})", + "snapshot original entrypoint addr {:#x} is outside the valid GVA range (0, {:#x}]", self.original_entrypoint_addr, - snap_lo, - snap_hi + max_gva_entrypoint )); } diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 6e13085da..8c19948db 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -874,6 +874,12 @@ impl Snapshot { sregs: Some(cfg.sregs), next_action, original_entrypoint: cfg.original_entrypoint_addr, + // TODO: File-snapshot format doesn't persist code_virt_base. + // This is acceptable because file snapshots currently pre-date + // ASLR and always use identity-mapped code (VA == GPA). If file + // snapshots ever need to round-trip through an ASLR sandbox, the + // format must be extended to include code_virt_base. + code_virt_base: 0, snapshot_generation, host_functions, }) diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index c062efad7..65d7d4131 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -34,7 +34,7 @@ use crate::Result; use crate::hypervisor::regs::CommonSpecialRegisters; use crate::mem::exe::{ExeInfo, LoadInfo}; use crate::mem::layout::SandboxMemoryLayout; -use crate::mem::memory_region::{GuestMemoryRegion, MemoryRegion, MemoryRegionFlags}; +use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::mgr::{GuestPageTableBuffer, SnapshotSharedMemory}; use crate::mem::shared_mem::{ReadonlySharedMemory, SharedMemory}; use crate::sandbox::SandboxConfiguration; @@ -95,8 +95,13 @@ pub struct Snapshot { /// The next action that should be performed on this snapshot next_action: NextAction, + /// Virtual base address of the code region. + /// For PIE binaries this equals the physical load address (identity-mapped). + /// For non-PIE binaries this is the ELF-declared base VA. + pub(crate) code_virt_base: u64, + /// Guest virtual address of the guest binary's ELF entry point - /// (`load_addr + e_entry - base_va`). Unlike `next_action`, which + /// (`code_virt_base + e_entry - base_va`). Unlike `next_action`, which /// transitions to `Call(dispatch_addr)` once the guest has run, /// this preserves the original entry across that transition. Used /// to fill `AT_ENTRY` in guest core dumps so a debugger can @@ -328,14 +333,21 @@ impl Snapshot { guest_blob_mem_flags, )?; - let load_addr = layout.get_guest_code_address() as u64; let base_va = exe_info.base_va(); let entrypoint_va: u64 = exe_info.entrypoint().into(); + let loaded_size = exe_info.loaded_size() as u64; + let is_pie = exe_info.is_pie(); + + // Get the memory regions with the Code region's guest_virt_addr + // already set to the correct virtual base (identity-mapped for PIE, + // ELF-declared VA for non-PIE), and validate no overlap conflicts. + let (code_virt_base, regions) = + layout.get_guest_regions_with_code_va(is_pie, base_va, loaded_size)?; let mut memory = vec![0; layout.get_memory_size()?]; let load_info = exe_info.load( - load_addr.try_into()?, + code_virt_base.try_into()?, &mut memory[layout.guest_code_offset()..], )?; @@ -348,7 +360,7 @@ impl Snapshot { let pt_buf = GuestPageTableBuffer::new(layout.get_pt_base_gpa() as usize); // 1. Map the (ideally readonly) pages of snapshot data - for rgn in layout.get_memory_regions_::(())?.iter() { + for rgn in regions.iter() { let readable = rgn.flags.contains(MemoryRegionFlags::READ); let executable = rgn.flags.contains(MemoryRegionFlags::EXECUTE); let writable = rgn.flags.contains(MemoryRegionFlags::WRITE); @@ -364,9 +376,10 @@ impl Snapshot { executable, }) }; + let mapping = Mapping { phys_base: rgn.guest_region.start as u64, - virt_base: rgn.guest_region.start as u64, + virt_base: rgn.guest_virt_addr as u64, len: rgn.guest_region.len() as u64, kind, }; @@ -384,7 +397,23 @@ impl Snapshot { - hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET + 1; - let entrypoint_gva = load_addr + entrypoint_va - base_va; + let entrypoint_offset = entrypoint_va.checked_sub(base_va).ok_or_else(|| { + crate::new_error!( + "ELF entrypoint VA ({:#x}) is below base VA ({:#x})", + entrypoint_va, + base_va + ) + })?; + + let entrypoint_gva = code_virt_base + .checked_add(entrypoint_offset) + .ok_or_else(|| { + crate::new_error!( + "Entrypoint overflow: code_virt_base {:#x} + offset {:#x}", + code_virt_base, + entrypoint_offset + ) + })?; Ok(Self { memory: ReadonlySharedMemory::from_bytes(&memory, layout.snapshot_size())?, @@ -393,6 +422,7 @@ impl Snapshot { stack_top_gva: exn_stack_top_gva, sregs: None, next_action: NextAction::Initialise(entrypoint_gva), + code_virt_base, original_entrypoint: entrypoint_gva, snapshot_generation: 0, host_functions: HostFunctionDetails { @@ -420,6 +450,7 @@ impl Snapshot { stack_top_gva: u64, sregs: CommonSpecialRegisters, next_action: NextAction, + code_virt_base: u64, original_entrypoint: u64, snapshot_generation: u64, host_functions: HostFunctionDetails, @@ -574,6 +605,7 @@ impl Snapshot { stack_top_gva, sregs: Some(sregs), next_action, + code_virt_base, original_entrypoint, snapshot_generation, host_functions, @@ -628,6 +660,12 @@ impl Snapshot { self.original_entrypoint } + /// Returns the virtual base address of the code region in guest space. + #[allow(dead_code)] + pub(crate) fn code_virt_base(&self) -> u64 { + self.code_virt_base + } + /// Validate that `provided` is a superset of the host functions /// recorded in this snapshot: every function that was registered /// at snapshot time must also be present in `provided` with a @@ -783,6 +821,7 @@ mod tests { 0, default_sregs(), super::NextAction::None, + 0, // code_virt_base 0, 1, HostFunctionDetails::default(), @@ -801,6 +840,7 @@ mod tests { 0, default_sregs(), super::NextAction::None, + 0, // code_virt_base 0, 2, HostFunctionDetails::default(), diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index b449ea68d..052dc9882 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -21,7 +21,7 @@ use std::time::Duration; use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use hyperlight_common::log_level::GuestLogFilter; use hyperlight_host::sandbox::SandboxConfiguration; -use hyperlight_host::{HyperlightError, MultiUseSandbox}; +use hyperlight_host::{HyperlightError, MultiUseSandbox, UninitializedSandbox}; use hyperlight_testing::simplelogger::{LOGGER, SimpleLogger}; use serial_test::serial; use tracing_core::LevelFilter; @@ -1838,6 +1838,7 @@ fn memory_region_types_are_publicly_accessible() { let base: ::HostBaseType = 0x1000; let _region = MemoryRegion_:: { guest_region: 0x1000..0x2000, + guest_virt_addr: 0x1000, host_region: base..::add(base, 0x1000), flags: MemoryRegionFlags::READ, region_type: MemoryRegionType::Code, @@ -1858,6 +1859,7 @@ fn memory_region_types_are_publicly_accessible() { }; let _region = MemoryRegion_:: { guest_region: 0x1000..0x2000, + guest_virt_addr: 0x1000, host_region: host_base ..::add(host_base, 0x1000), flags: MemoryRegionFlags::READ, @@ -1890,3 +1892,16 @@ fn hw_timer_interrupts() { ); }); } + +#[test] +fn non_pie_guest_hello_world() { + let path = + hyperlight_testing::simple_guest_non_pie_as_string().expect("non-PIE guest not found"); + let sandbox = + UninitializedSandbox::new(hyperlight_host::GuestBinary::FilePath(path), None).unwrap(); + let mut multi_use_sandbox: MultiUseSandbox = sandbox.evolve().unwrap(); + let result: i32 = multi_use_sandbox + .call("PrintOutput", "Hello from non-PIE guest!\n".to_string()) + .unwrap(); + assert_eq!(result, 26); +} diff --git a/src/hyperlight_testing/src/lib.rs b/src/hyperlight_testing/src/lib.rs index aa3af3237..b2d1d160d 100644 --- a/src/hyperlight_testing/src/lib.rs +++ b/src/hyperlight_testing/src/lib.rs @@ -88,6 +88,37 @@ pub fn dummy_guest_as_string() -> Result { .ok_or_else(|| anyhow!("couldn't convert dummy guest PathBuf to string")) } +/// Get a fully qualified OS-specific path to the non-PIE simpleguest elf binary +pub fn simple_guest_non_pie_as_string() -> Result { + let buf = rust_guest_non_pie_as_pathbuf("simpleguest"); + buf.to_str() + .map(|s| s.to_string()) + .ok_or_else(|| anyhow!("couldn't convert non-PIE simple guest PathBuf to string")) +} + +/// Get a new `PathBuf` to a specified non-PIE Rust guest +/// $REPO_ROOT/src/tests/rust_guests/bin/${profile}/non_pie/ +fn rust_guest_non_pie_as_pathbuf(guest: &str) -> PathBuf { + let build_dir_selector = if cfg!(debug_assertions) { + "debug" + } else { + "release" + }; + + join_to_path( + MANIFEST_DIR, + vec![ + "..", + "tests", + "rust_guests", + "bin", + build_dir_selector, + "non_pie", + guest, + ], + ) +} + pub fn c_guest_as_pathbuf(guest: &str) -> PathBuf { let build_dir_selector = if cfg!(debug_assertions) { "debug"