diff --git a/.github/workflows/dep_build_guests.yml b/.github/workflows/dep_build_guests.yml index 808ba179ae..c1409ffc25 100644 --- a/.github/workflows/dep_build_guests.yml +++ b/.github/workflows/dep_build_guests.yml @@ -54,7 +54,7 @@ jobs: run: | sudo chown -R $(id -u):$(id -g) /opt/cargo || true - # cargo-hyperlight builds a custom sysroot for x86_64-hyperlight-none target. + # cargo-hyperlight builds a custom sysroot for the Hyperlight guest target. # rust-cache cleans "anything not a dependency" from target dirs, removing the sysroot. # We cache sysroot separately to avoid rebuilding it (~10s) on every run. - name: Sysroot cache @@ -62,6 +62,7 @@ jobs: with: path: | src/tests/rust_guests/target/sysroot + src/tests/rust_guests/target-non-pie/sysroot key: sysroot-linux-${{ inputs.arch }}-${{ inputs.config }}-${{ hashFiles('rust-toolchain.toml') }} - name: Rust cache @@ -87,6 +88,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 }} @@ -108,4 +114,3 @@ jobs: path: src/tests/c_guests/bin/${{ inputs.config }}/ retention-days: 1 if-no-files-found: error - diff --git a/.gitignore b/.gitignore index 3aae7b792e..27126b8ded 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 1583900f04..c426140fe5 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,22 @@ 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. +# 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 {{ hyperlight-target }} --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/" + hyperlight-target + +@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/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 65b770e33d..c42d894d08 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -94,7 +94,7 @@ impl<'a> DebugMemoryView<'a> { } pub(crate) fn code_section_offset(&self) -> u64 { - self.mem_mgr.layout.get_guest_code_address() as u64 + self.mem_mgr.code_virt_base } /// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index 4ced7f9e9f..75d0870421 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -12,7 +12,7 @@ use super::{ #[cfg(hvf)] use crate::hypervisor::HvfInterruptHandle; use crate::hypervisor::InterruptHandleImpl; -#[cfg(any(kvm, mshv3))] +#[cfg(target_os = "linux")] use crate::hypervisor::LinuxInterruptHandle; #[cfg(gdb)] use crate::hypervisor::gdb::{DebugCommChannel, DebugMsg, DebugResponse}; 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 6fabdbfc57..7845b1ece6 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -863,7 +863,7 @@ mod tests { use crate::hypervisor::regs::{CommonSegmentRegister, CommonTableRegister, MXCSR_DEFAULT}; use crate::hypervisor::virtual_machine::VirtualMachine; use crate::mem::layout::SandboxMemoryLayout; - use crate::mem::memory_region::{GuestMemoryRegion, MemoryRegionFlags}; + use crate::mem::memory_region::MemoryRegionFlags; use crate::mem::mgr::{GuestPageTableBuffer, SandboxMemoryManager}; use crate::mem::ptr::RawPtr; use crate::mem::shared_mem::{ExclusiveSharedMemory, ReadonlySharedMemory}; @@ -1425,16 +1425,12 @@ mod tests { let pt_base_gpa = layout.get_pt_base_gpa(); let pt_buf = GuestPageTableBuffer::new(pt_base_gpa as usize); - for rgn in layout - .get_memory_regions_::(()) - .unwrap() - .iter() - { + for rgn in layout.get_memory_regions().unwrap().iter() { let readable = rgn.flags.contains(MemoryRegionFlags::READ); let writable = rgn.flags.contains(MemoryRegionFlags::WRITE); let executable = rgn.flags.contains(MemoryRegionFlags::EXECUTE); let mapping = Mapping { - phys_base: rgn.guest_region.start as u64, + phys_base: rgn.host_region.start as u64, virt_base: rgn.guest_region.start as u64, len: rgn.guest_region.len() as u64, kind: MappingKind::Basic(BasicMapping { @@ -1480,7 +1476,7 @@ mod tests { layout, ro_mem.to_mgr_snapshot_mem().unwrap(), scratch_mem, - NextAction::Initialise(layout.get_guest_code_address() as u64), + NextAction::Initialise(layout.get_guest_code_gva() as u64), ); let (mut hshm, gshm) = mem_mgr.build().unwrap(); @@ -2185,7 +2181,7 @@ mod tests { a.fxsave(ptr(rax)).unwrap(); // Return dispatch ptr - a.mov(rax, layout.get_guest_code_address() as u64).unwrap(); + a.mov(rax, layout.get_guest_code_gva() as u64).unwrap(); a.hlt().unwrap(); diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index aee561b477..cc7dc981ce 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -508,8 +508,8 @@ pub(crate) mod tests { )?; // Set up required parameters for initialise - let peb_addr = RawPtr::from(0x1000u64); // Dummy PEB address - let seed = 12345u64; // Random seed + let peb_addr = RawPtr::from(0x230000u64); + let seed = 1234567890u64; let host_funcs = Arc::new(Mutex::new(FunctionRegistry::default())); let guest_max_log_level = Some(tracing_core::LevelFilter::ERROR); diff --git a/src/hyperlight_host/src/mem/elf.rs b/src/hyperlight_host/src/mem/elf.rs index f6cde29068..7b2fa0a2fe 100644 --- a/src/hyperlight_host/src/mem/elf.rs +++ b/src/hyperlight_host/src/mem/elf.rs @@ -4,6 +4,7 @@ #[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")] @@ -14,6 +15,53 @@ use goblin::elf64::program_header::PT_LOAD; use super::exe::LoadInfo; use crate::{Result, log_then_return, new_error}; +fn apply_relative_relocation( + name: &str, + relocation_va: u64, + addend: i64, + base_va: u64, + load_gva: u64, + target: &mut [u8], +) -> Result<()> { + let offset = relocation_va.checked_sub(base_va).ok_or_else(|| { + new_error!( + "{} target VA ({:#x}) is below ELF base VA ({:#x})", + name, + relocation_va, + base_va + ) + })?; + let offset: usize = offset.try_into()?; + let end = offset + .checked_add(size_of::()) + .ok_or_else(|| new_error!("{} target offset overflow", name))?; + let target_len = target.len(); + let destination = target.get_mut(offset..end).ok_or_else(|| { + new_error!( + "{} target range [{:#x}, {:#x}) exceeds loaded image size ({:#x})", + name, + offset, + end, + target_len + ) + })?; + + let load_bias = i128::from(load_gva) - i128::from(base_va); + let value = i128::from(addend) + .checked_add(load_bias) + .and_then(|value| u64::try_from(value).ok()) + .ok_or_else(|| { + new_error!( + "{} result does not fit in u64: addend ({:#x}) + load bias ({:#x})", + name, + addend, + load_bias + ) + })?; + destination.copy_from_slice(&value.to_le_bytes()); + Ok(()) +} + #[cfg(feature = "mem_profile")] struct ResolvedSectionHeader { name: String, @@ -29,6 +77,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, @@ -130,6 +180,7 @@ impl ElfInfo { .collect(), entry: elf.entry, relocs, + is_pie: elf.header.e_type == ET_DYN, guest_bin_version, }) } @@ -155,6 +206,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> { @@ -180,7 +236,7 @@ impl ElfInfo { .unwrap(); (max_phdr.p_vaddr + max_phdr.p_memsz - self.get_base_va()) as usize } - pub(crate) fn load_at(self, load_addr: usize, target: &mut [u8]) -> Result { + pub(crate) fn load_at(self, load_gva: u64, target: &mut [u8]) -> Result { let base_va = self.get_base_va(); for phdr in self.phdrs.iter().filter(|phdr| phdr.p_type == PT_LOAD) { let start_va = (phdr.p_vaddr - base_va) as usize; @@ -199,8 +255,14 @@ impl ElfInfo { match r.r_type { R_AARCH64_RELATIVE => { let addend = get_addend("R_AARCH64_RELATIVE", r)?; - target[r.r_offset as usize..r.r_offset as usize + 8] - .copy_from_slice(&(load_addr as i64 + addend).to_le_bytes()); + apply_relative_relocation( + "R_AARCH64_RELATIVE", + r.r_offset, + addend, + base_va, + load_gva, + target, + )?; } R_AARCH64_NONE => {} _ => { @@ -211,8 +273,14 @@ impl ElfInfo { match r.r_type { R_X86_64_RELATIVE => { let addend = get_addend("R_X86_64_RELATIVE", r)?; - target[r.r_offset as usize..r.r_offset as usize + 8] - .copy_from_slice(&(load_addr as i64 + addend).to_le_bytes()); + apply_relative_relocation( + "R_X86_64_RELATIVE", + r.r_offset, + addend, + base_va, + load_gva, + target, + )?; } R_X86_64_NONE => {} _ => { @@ -227,7 +295,7 @@ impl ElfInfo { Ok(LoadInfo { info: Arc::new(UnwindInfo { payload: self.payload, - load_addr: load_addr as u64, + load_addr: load_gva, va_size, base_svma, shdrs: self.shdrs, @@ -239,3 +307,28 @@ impl ElfInfo { } } } + +#[cfg(test)] +mod tests { + use super::apply_relative_relocation; + + #[test] + fn relative_relocation_uses_link_base() { + let mut target = [0u8; 16]; + + apply_relative_relocation("R_RELATIVE", 0x1008, 0x1010, 0x1000, 0x3000, &mut target) + .unwrap(); + + assert_eq!(u64::from_le_bytes(target[8..].try_into().unwrap()), 0x3010); + } + + #[test] + fn relative_relocation_supports_negative_load_bias() { + let mut target = [0u8; 8]; + + apply_relative_relocation("R_RELATIVE", 0x1000, 0x1010, 0x1000, 0x800, &mut target) + .unwrap(); + + assert_eq!(u64::from_le_bytes(target), 0x810); + } +} diff --git a/src/hyperlight_host/src/mem/exe.rs b/src/hyperlight_host/src/mem/exe.rs index 7bf3a446d4..e2818f70bc 100644 --- a/src/hyperlight_host/src/mem/exe.rs +++ b/src/hyperlight_host/src/mem/exe.rs @@ -76,6 +76,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 { @@ -100,9 +106,9 @@ impl ExeInfo { // copying into target, but the PE loader chooses to apply // relocations in its owned representation of the PE contents, // which requires it to be &mut. - pub fn load(self, load_addr: usize, target: &mut [u8]) -> Result { + pub fn load(self, load_gva: u64, target: &mut [u8]) -> Result { match self { - ExeInfo::Elf(elf) => elf.load_at(load_addr, target), + ExeInfo::Elf(elf) => elf.load_at(load_gva, target), } } } diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 9e7e10b242..e5bc4da1da 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -56,8 +56,8 @@ use tracing::{Span, instrument}; use super::memory_region::MemoryRegionType::{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, MemoryRegionVecBuilder, }; #[cfg(readable_shared_mem)] use super::shared_mem::HostSharedMemory; @@ -244,6 +244,8 @@ pub(crate) struct SandboxMemoryLayout { heap_size: usize, /// The size of the guest code section. code_size: usize, + /// Guest virtual address of the code section. + code_gva: usize, /// The size of the init data section (guest blob). init_data_size: usize, /// Permission flags for the init data region. @@ -270,6 +272,7 @@ impl Debug for SandboxMemoryLayout { &format_args!("{:#x}", self.get_memory_size().unwrap_or(0)), ) .field("Code Size", &format_args!("{:#x}", self.code_size)) + .field("Code GVA", &format_args!("{:#x}", self.code_gva)) .field("Heap Size", &format_args!("{:#x}", self.heap_size)) .field( "Init Data Size", @@ -324,6 +327,7 @@ impl SandboxMemoryLayout { output_data_size, heap_size, code_size, + code_gva, init_data_size, init_data_permissions, scratch_size, @@ -334,6 +338,7 @@ impl SandboxMemoryLayout { && *output_data_size == other.output_data_size && *heap_size == other.heap_size && *code_size == other.code_size + && *code_gva == other.code_gva && *init_data_size == other.init_data_size && *init_data_permissions == other.init_data_permissions && *scratch_size == other.scratch_size @@ -379,6 +384,7 @@ impl SandboxMemoryLayout { output_data_size, heap_size, code_size, + code_gva: Self::BASE_ADDRESS, init_data_size, init_data_permissions, pt_size: None, @@ -457,11 +463,8 @@ impl SandboxMemoryLayout { /// Returns the memory regions associated with this memory layout, /// suitable for passing to a hypervisor for mapping into memory - pub(crate) fn get_memory_regions_( - &self, - host_base: K::HostBaseType, - ) -> Result>> { - let mut builder = MemoryRegionVecBuilder::new(Self::BASE_ADDRESS, host_base); + pub(crate) fn get_memory_regions(&self) -> Result>> { + let mut builder = MemoryRegionVecBuilder::new(Self::BASE_ADDRESS, Self::BASE_ADDRESS); // code let peb_offset = builder.push_page_aligned( @@ -551,7 +554,114 @@ impl SandboxMemoryLayout { )); } - Ok(builder.build()) + let mut regions = builder.build(); + for region in &mut regions { + if region.region_type == Code { + let end = self + .code_gva + .checked_add(region.guest_region.len()) + .ok_or_else(|| { + new_error!( + "code mapping overflow: base {:#x} + size {:#x}", + self.code_gva, + region.guest_region.len() + ) + })?; + region.guest_region = self.code_gva..end; + } + } + Ok(regions) + } + + /// 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 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 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, + )); + } + } + + // Override the Code region's GVA (guest_region) to code_virt_base. + // host_region retains the GPA from the builder. + for rgn in regions.iter_mut() { + if rgn.region_type == MemoryRegionType::Code { + let len = rgn.guest_region.len(); + rgn.guest_region = code_virt_base as usize..(code_virt_base as usize + len); + } + } + + 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")] @@ -676,11 +786,17 @@ impl SandboxMemoryLayout { 0 } - /// Guest address of the code section in the sandbox. - pub(crate) fn get_guest_code_address(&self) -> usize { + /// Guest physical address of the code section. + #[allow(dead_code)] + pub(crate) fn get_guest_code_gpa(&self) -> usize { Self::BASE_ADDRESS + self.guest_code_offset() } + /// Guest virtual address of the code section. + pub(crate) fn get_guest_code_gva(&self) -> usize { + self.code_gva + } + /// Guest virtual address of the start of output data. pub(crate) fn get_output_data_buffer_gva(&self) -> u64 { hyperlight_common::layout::scratch_base_gva(self.scratch_size) + self.input_data_size as u64 @@ -774,6 +890,35 @@ mod tests { ); } + #[test] + fn code_gva_updates_code_region() { + let mut layout = + SandboxMemoryLayout::new(SandboxConfiguration::default(), PAGE_SIZE, 0, None).unwrap(); + let code_gva = 0x100_0000; + layout.set_code_gva(code_gva).unwrap(); + + assert_eq!( + layout.get_guest_code_gpa(), + SandboxMemoryLayout::BASE_ADDRESS + ); + assert_eq!(layout.get_guest_code_gva(), code_gva as usize); + let code = layout + .get_memory_regions() + .unwrap() + .into_iter() + .find(|region| region.region_type == Code) + .unwrap(); + assert_eq!(code.host_region.start, SandboxMemoryLayout::BASE_ADDRESS); + assert_eq!(code.guest_region.start, code_gva as usize); + } + + #[test] + fn code_gva_rejects_overlap() { + let mut layout = + SandboxMemoryLayout::new(SandboxConfiguration::default(), PAGE_SIZE, 0, None).unwrap(); + assert!(layout.set_code_gva(layout.peb_address() as u64).is_err()); + } + #[test] fn test_max_memory_sandbox() { let mut cfg = SandboxConfiguration::default(); @@ -818,6 +963,7 @@ mod tests { |l| l.output_data_size += PAGE_SIZE, |l| l.heap_size += PAGE_SIZE, |l| l.code_size += PAGE_SIZE, + |l| l.code_gva += PAGE_SIZE, |l| l.init_data_size += PAGE_SIZE, |l| l.scratch_size += PAGE_SIZE, |l| { diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 26a5331437..8f6e978c56 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -261,26 +261,33 @@ impl MemoryRegionKind for HostGuestMemoryRegion { } } -/// Type for memory regions that only track guest addresses. +/// Marker for GPA-to-GVA mappings used to construct guest page tables. /// +/// `host_region` contains GPAs and `guest_region` contains GVAs. #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub(crate) struct GuestMemoryRegion {} impl MemoryRegionKind for GuestMemoryRegion { - type HostBaseType = (); + type HostBaseType = usize; - fn add(_base: Self::HostBaseType, _size: usize) -> Self::HostBaseType {} + fn add(base: Self::HostBaseType, size: usize) -> Self::HostBaseType { + base + size + } } /// represents a single memory region inside the guest. All memory within a region has /// the same memory permissions #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct MemoryRegion_ { - /// the range of guest memory addresses + /// The destination range of the mapping. + /// + /// This is a GVA for `GuestMemoryRegion`, a GPA for + /// `HostGuestMemoryRegion`, and a GVA for `CrashDumpMemoryRegion`. pub guest_region: Range, - /// the range of host memory addresses + /// The source range of the mapping. /// - /// Note that Range<()> = () x () = (). + /// This is a GPA for `GuestMemoryRegion` and an HVA for + /// `HostGuestMemoryRegion` and `CrashDumpMemoryRegion`. pub host_region: Range, /// memory access flags for the given region pub flags: MemoryRegionFlags, diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index e2c4da395b..4d56c92033 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -513,7 +513,6 @@ 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.update_scratch_bookkeeping()?; Ok((gsnapshot, gscratch)) } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 8613546dc1..0a03fe264d 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -2298,9 +2298,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); @@ -2324,7 +2327,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); } @@ -2334,7 +2337,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); } @@ -2344,7 +2347,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; @@ -2360,7 +2363,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); } @@ -2371,7 +2374,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 e9b49ef823..2844a3a3a6 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -210,6 +210,10 @@ pub(super) struct MemoryLayout { pub(super) output_data_size: usize, pub(super) heap_size: usize, pub(super) code_size: usize, + /// Virtual base address of the code region. A value of zero means the + /// code region is identity mapped. + #[serde(default)] + pub(super) code_virt_base: u64, pub(super) init_data_size: usize, /// Memory region flag bits. `None` means default permissions. pub(super) init_data_permissions: Option, @@ -477,13 +481,19 @@ impl OciSnapshotConfig { } // The saved dispatch entrypoint must be in the executable code - // region. Code occupies the page-rounded prefix of the snapshot. - let code_lo = SandboxMemoryLayout::BASE_ADDRESS as u64; + // region. For non-PIE or ASLR guests the code region's virtual + // base differs from the physical load address. + let code_lo = if self.layout.code_virt_base != 0 { + self.layout.code_virt_base + } else { + SandboxMemoryLayout::BASE_ADDRESS as u64 + }; let code_hi = code_lo .checked_add(self.layout.code_size.next_multiple_of(PAGE_SIZE) as u64) .ok_or_else(|| { crate::new_error!( - "snapshot layout overflow: BASE_ADDRESS + code_size ({}) does not fit in u64", + "snapshot layout overflow: code_virt_base ({:#x}) + code_size ({}) does not fit in u64", + code_lo, self.layout.code_size ) })?; @@ -515,10 +525,10 @@ impl OciSnapshotConfig { })?; if self.original_entrypoint_addr < code_lo || self.original_entrypoint_addr >= snapshot_hi { 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 code region [{:#x}, {:#x})", self.original_entrypoint_addr, code_lo, - snapshot_hi + code_hi )); } @@ -771,6 +781,7 @@ mod tests { output_data_size: 0, heap_size: 0, code_size: 0, + code_virt_base: 0, init_data_size: 0, init_data_permissions: None, scratch_size: 0, @@ -1003,6 +1014,7 @@ mod schema_pin { "output_data_size": 2, "heap_size": 3, "code_size": 4, + "code_virt_base": 0, "init_data_size": 5, "init_data_permissions": null, "scratch_size": 8, @@ -1045,6 +1057,7 @@ mod schema_pin { "output_data_size": 2, "heap_size": 3, "code_size": 4, + "code_virt_base": 0, "init_data_size": 5, "init_data_permissions": null, "scratch_size": 8, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 678d1d1626..ffa6e7c27d 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -606,6 +606,7 @@ impl Snapshot { output_data_size: l.output_data_size(), heap_size: l.heap_size(), code_size: l.code_size(), + code_virt_base: l.get_guest_code_gva() as u64, init_data_size: l.init_data_size(), init_data_permissions: l.init_data_permissions().map(|f| f.bits()), scratch_size: l.get_scratch_size(), @@ -815,6 +816,12 @@ impl Snapshot { cfg.layout.init_data_size, init_data_perms, )?; + let code_gva = if cfg.layout.code_virt_base == 0 { + layout.get_guest_code_gpa() as u64 + } else { + cfg.layout.code_virt_base + }; + layout.set_code_gva(code_gva)?; // `snapshot_size` and `pt_size` are independent fields. if let Some(pt) = cfg.layout.pt_size { layout.set_pt_size(pt)?; diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 7e200a50e4..018a327743 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -12,8 +12,7 @@ use serde_json::Value; use sha2::{Digest as _, Sha256}; use crate::func::Registerable; -use crate::mem::layout::SandboxMemoryLayout; -use crate::mem::shared_mem::SharedMemory as _; +use crate::mem::shared_mem::SharedMemory; use crate::sandbox::snapshot::{OciDigest, OciReference, OciTag, Snapshot}; use crate::{GuestBinary, HostFunctions, MultiUseSandbox, SandboxBuilder}; @@ -2047,11 +2046,11 @@ fn original_entrypoint_addr_zero_rejected() { fn entrypoint_addr_outside_code_rejected() { let (_dir, path) = save_for_mutation(); rewrite_config(&path, |cfg| { + let code_virt_base = cfg["layout"]["code_virt_base"].as_u64().unwrap(); let code_size = cfg["layout"]["code_size"].as_u64().unwrap(); let page_size = hyperlight_common::vmem::PAGE_SIZE as u64; - let peb_addr = - SandboxMemoryLayout::BASE_ADDRESS as u64 + code_size.next_multiple_of(page_size); - cfg["entrypoint_addr"] = Value::from(peb_addr); + let beyond_code = code_virt_base + code_size.next_multiple_of(page_size); + cfg["entrypoint_addr"] = Value::from(beyond_code); }); let err = unwrap_err_snapshot(Snapshot::checked_load( &path, @@ -2959,6 +2958,22 @@ fn save_returns_manifest_digest_that_loads() { assert_eq!(loaded.snapshot_generation(), expected_gen); } +/// The code GVA must survive a save/load round-trip so GDB and +/// tracing can resolve symbols for non-PIE (or ASLR) guests after +/// restoring from a file snapshot. +#[test] +fn round_trip_preserves_code_gva() { + let snap = create_snapshot(); + let original = snap.layout().get_guest_code_gva(); + assert_ne!(original, 0, "fixture must have a non-zero code GVA"); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("layout"); + snap.save(&path, &OciTag::new("latest").unwrap()).unwrap(); + let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); + assert_eq!(loaded.layout().get_guest_code_gva(), original); +} + /// The returned digest is the sha256 of the manifest blob, matching the /// digest recorded for that tag's manifest descriptor in `index.json`. #[test] diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index a4def5b7af..be7fac50ba 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -23,7 +23,7 @@ use crate::hypervisor::regs::CommonSpecialRegisters; use crate::hypervisor::regs::MsrEntry; 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; @@ -89,7 +89,7 @@ pub struct Snapshot { next_action: NextAction, /// Guest virtual address of the guest binary's ELF entry point - /// (`load_addr + e_entry - base_va`). Unlike `next_action`, which + /// (`code GVA + 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 @@ -321,14 +321,20 @@ 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 is_pie = exe_info.is_pie(); + + let (code_virt_base, regions) = layout.get_guest_regions_with_code_va( + is_pie, + base_va, + exe_info.loaded_size() as u64, + )?; 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()..], )?; @@ -341,7 +347,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); @@ -357,8 +363,9 @@ impl Snapshot { executable, }) }; + let mapping = Mapping { - phys_base: rgn.guest_region.start as u64, + phys_base: rgn.host_region.start as u64, virt_base: rgn.guest_region.start as u64, len: rgn.guest_region.len() as u64, kind, @@ -377,7 +384,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())?, @@ -637,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 diff --git a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs index f89b83bc43..f4c92a6def 100644 --- a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs +++ b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs @@ -105,7 +105,7 @@ impl MemTraceInfo { &mut *cache, &mut read_stack, ); - iter.map(|f| Ok(f.address() - mem_mgr.layout.get_guest_code_address() as u64)) + iter.map(|f| Ok(f.address() - mem_mgr.layout.get_guest_code_gva() as u64)) .collect() .map_err(|e| new_error!("couldn't unwind: {}", e)) } diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index 165d753fd4..2d05b48067 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -7,7 +7,7 @@ use std::time::Duration; use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use hyperlight_common::log_level::GuestLogFilter; -use hyperlight_host::{HyperlightError, MultiUseSandbox, SandboxBuilder}; +use hyperlight_host::{HyperlightError, MultiUseSandbox, SandboxBuilder, UninitializedSandbox}; use hyperlight_testing::simplelogger::{LOGGER, SimpleLogger}; use serial_test::serial; use tracing_core::LevelFilter; @@ -1850,3 +1850,17 @@ 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.into()), 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 46b0b8c48c..a9a058bf90 100644 --- a/src/hyperlight_testing/src/lib.rs +++ b/src/hyperlight_testing/src/lib.rs @@ -90,6 +90,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"