Skip to content

Simplify GDB memory access by borrowing sandbox memory manager #1735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ludfjig wants to merge 1 commit into from
Closed

Simplify GDB memory access by borrowing sandbox memory manager #1735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 109 additions & 109 deletions src/hyperlight_host/src/hypervisor/gdb/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mod x86_64_target;

use std::io::{self, ErrorKind};
use std::net::TcpListener;
use std::sync::{Arc, Mutex};
use std::sync::Arc;
use std::thread;

use crossbeam_channel::{Receiver, Sender, TryRecvError};
Expand Down Expand Up @@ -78,28 +78,38 @@ impl From<GdbTargetError> for TargetError<GdbTargetError> {
}
}

/// This abstracts the memory access functions that debugging needs from a sandbox
pub(crate) struct DebugMemoryAccess {
/// Memory manager that provides access to the guest memory
pub(crate) dbg_mem_access_fn: Arc<Mutex<SandboxMemoryManager<HostSharedMemory>>>,
/// Guest mapped memory regions
pub(crate) guest_mmap_regions: Vec<MemoryRegion>,
/// A borrowed view of the sandbox memory visible to GDB.
pub(crate) struct DebugMemoryView<'a> {
mem_mgr: &'a SandboxMemoryManager<HostSharedMemory>,
guest_mmap_regions: Vec<MemoryRegion>,
}

/// Errors that can occur during debug memory access operations
#[derive(Debug, thiserror::Error)]
pub enum DebugMemoryAccessError {
#[error("Failed to copy memory: {0}")]
CopyFailed(Box<HyperlightError>),
#[error("Failed to acquire lock at {0}:{1} - {2}")]
LockFailed(&'static str, u32, String),
#[error("Failed to translate guest address {0:#x}")]
TranslateGuestAddress(u64),
#[error("Failed to write to read-only region")]
WriteToReadOnly,
}

impl DebugMemoryAccess {
impl<'a> DebugMemoryView<'a> {
pub(crate) fn new(
mem_mgr: &'a SandboxMemoryManager<HostSharedMemory>,
guest_mmap_regions: Vec<MemoryRegion>,
) -> Self {
Self {
mem_mgr,
guest_mmap_regions,
}
}

pub(crate) fn code_section_offset(&self) -> u64 {
self.mem_mgr.layout.get_guest_code_address() as u64
}

/// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE
///
/// # Arguments
Expand All @@ -113,15 +123,11 @@ impl DebugMemoryAccess {
data: &mut [u8],
gpa: u64,
) -> std::result::Result<(), DebugMemoryAccessError> {
let mgr = self
.dbg_mem_access_fn
.try_lock()
.map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?;

mgr.layout
self.mem_mgr
.layout
.resolve_gpa(gpa, &self.guest_mmap_regions)
.ok_or(DebugMemoryAccessError::TranslateGuestAddress(gpa))?
.with_memories(&mgr.shared_mem, &mgr.scratch_mem)
.with_memories(&self.mem_mgr.shared_mem, &self.mem_mgr.scratch_mem)
.copy_to_slice(data)
.map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))
}
Expand All @@ -139,12 +145,8 @@ impl DebugMemoryAccess {
data: &[u8],
gpa: u64,
) -> std::result::Result<(), DebugMemoryAccessError> {
let mgr = self
.dbg_mem_access_fn
.try_lock()
.map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?;

let resolved = mgr
let resolved = self
.mem_mgr
.layout
.resolve_gpa(gpa, &self.guest_mmap_regions)
.ok_or(DebugMemoryAccessError::TranslateGuestAddress(gpa))?;
Expand All @@ -153,11 +155,13 @@ impl DebugMemoryAccess {
// process) if the address is in the scratch region
match resolved.base {
#[cfg(unshared_snapshot_mem)]
BaseGpaRegion::Snapshot(()) => mgr
BaseGpaRegion::Snapshot(()) => self
.mem_mgr
.shared_mem
.copy_from_slice(data, resolved.offset)
.map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))),
BaseGpaRegion::Scratch(()) => mgr
BaseGpaRegion::Scratch(()) => self
.mem_mgr
.scratch_mem
.copy_from_slice(data, resolved.offset)
.map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))),
Expand Down Expand Up @@ -373,7 +377,6 @@ mod tests {
mod mem_access_tests {
use std::os::fd::AsRawFd;
use std::os::linux::fs::MetadataExt;
use std::sync::{Arc, Mutex};

use hyperlight_testing::dummy_guest_as_pathbuf;

Expand All @@ -387,168 +390,165 @@ mod tests {
#[cfg(target_os = "linux")]
const BASE_VIRT: usize = 0x10000000 + SandboxMemoryLayout::BASE_ADDRESS;

/// Dummy memory region to test memory access
/// This maps a file into memory and uses it as guest memory
fn get_mem_access() -> crate::Result<DebugMemoryAccess> {
let filename = dummy_guest_as_pathbuf();

let file = std::fs::File::options()
.read(true)
.write(true)
.open(&filename)?;
let file_size = file.metadata()?.st_size();
let page_size = page_size::get();
let size = (file_size as usize).div_ceil(page_size) * page_size;
let mapped_mem = unsafe {
libc::mmap(
std::ptr::null_mut(),
size,
libc::PROT_READ | libc::PROT_WRITE | libc::PROT_EXEC,
libc::MAP_PRIVATE,
file.as_raw_fd(),
0,
)
};
if mapped_mem == libc::MAP_FAILED {
log_then_return!("mmap error: {:?}", std::io::Error::last_os_error());
struct TestMemory {
mem_mgr: SandboxMemoryManager<HostSharedMemory>,
mmap_region: MemoryRegion,
}

impl TestMemory {
fn new() -> crate::Result<Self> {
let filename = dummy_guest_as_pathbuf();
let file = std::fs::File::options()
.read(true)
.write(true)
.open(&filename)?;
let file_size = file.metadata()?.st_size();
let page_size = page_size::get();
let size = (file_size as usize).div_ceil(page_size) * page_size;
let mapped_mem = unsafe {
libc::mmap(
std::ptr::null_mut(),
size,
libc::PROT_READ | libc::PROT_WRITE | libc::PROT_EXEC,
libc::MAP_PRIVATE,
file.as_raw_fd(),
0,
)
};
if mapped_mem == libc::MAP_FAILED {
log_then_return!("mmap error: {:?}", std::io::Error::last_os_error());
}

let sandbox = UninitializedSandbox::new(GuestBinary::FilePath(filename), None)
.inspect_err(|_| unsafe {
libc::munmap(mapped_mem, size);
})?;
let (mem_mgr, _) = sandbox.mgr.build()?;

Ok(Self {
mem_mgr,
mmap_region: MemoryRegion {
host_region: mapped_mem as usize..mapped_mem.wrapping_add(size) as usize,
guest_region: BASE_VIRT..BASE_VIRT + size,
flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE,
region_type: MemoryRegionType::Heap,
},
})
}

// Create a sandbox memory manager with the mapped memory region
let sandbox = UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), None)
.inspect_err(|_| unsafe {
libc::munmap(mapped_mem, size);
})?;
let (mem_mgr, _) = sandbox.mgr.build()?;

// Create the memory access struct
let mem_access = DebugMemoryAccess {
dbg_mem_access_fn: Arc::new(Mutex::new(mem_mgr)),
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,
flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE,
region_type: MemoryRegionType::Heap,
}],
};

Ok(mem_access)
}
fn access(&self) -> DebugMemoryView<'_> {
DebugMemoryView::new(&self.mem_mgr, vec![self.mmap_region.clone()])
}

/// Gets a slice to the mapped memory region to be able to modify it
///
/// NOTE: By returning a mutable slice from a mutable reference, we ensure
/// that the memory is not deallocated while the slice is in use.
unsafe fn get_mmap_slice(mem_access: &mut DebugMemoryAccess) -> &mut [u8] {
unsafe {
std::slice::from_raw_parts_mut(
mem_access.guest_mmap_regions[0].host_region.start as *mut u8,
mem_access.guest_mmap_regions[0].host_region.end
- mem_access.guest_mmap_regions[0].host_region.start,
)
unsafe fn mmap_slice(&mut self) -> &mut [u8] {
unsafe {
std::slice::from_raw_parts_mut(
self.mmap_region.host_region.start as *mut u8,
self.mmap_region.host_region.len(),
)
}
}
}

/// Drops the mapped memory region
fn drop_mem_access(mem_access: DebugMemoryAccess) {
let mapped_mem =
mem_access.guest_mmap_regions[0].host_region.start as *mut libc::c_void;
let size = mem_access.guest_mmap_regions[0].host_region.end
- mem_access.guest_mmap_regions[0].host_region.start;

unsafe {
libc::munmap(mapped_mem, size);
impl Drop for TestMemory {
fn drop(&mut self) {
unsafe {
libc::munmap(
self.mmap_region.host_region.start as *mut libc::c_void,
self.mmap_region.host_region.len(),
);
}
}
}

#[test]
fn test_mem_access_read_single_byte() -> crate::Result<()> {
let mut mem_access = get_mem_access()?;
let mut memory = TestMemory::new()?;
let offset = 2000;

// Modify the memory directly to have a known value to read
{
let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
slice[offset] = 0xAA;
}

let mut read_data = [0u8; 1];
mem_access
memory
.access()
.read(&mut read_data, (BASE_VIRT + offset) as u64)
.unwrap();

assert_eq!(read_data[0], 0xAA);

drop_mem_access(mem_access);

Ok(())
}

#[test]
fn test_mem_access_read_multiple_bytes() -> crate::Result<()> {
let mut mem_access = get_mem_access()?;
let mut memory = TestMemory::new()?;
let offset = 20;

// Modify the memory directly to have a known value to read
{
let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
for i in 0..16 {
slice[offset + i] = i as u8;
}
}

let mut read_data = [0u8; 16];
mem_access
memory
.access()
.read(&mut read_data, (BASE_VIRT + offset) as u64)
.unwrap();

assert_eq!(
read_data,
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
);
drop_mem_access(mem_access);
Ok(())
}

#[test]
fn test_mem_access_write_single_byte() -> crate::Result<()> {
let mut mem_access = get_mem_access()?;
let mut memory = TestMemory::new()?;
let offset = 3000;
{
let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
slice[offset] = 0xBB;
}

let write_data = [0xCCu8; 1];
mem_access
memory
.access()
.write(&write_data, (BASE_VIRT + offset) as u64)
.unwrap();

let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
assert_eq!(slice[offset], write_data[0]);
Comment on lines 521 to 528
drop_mem_access(mem_access);

Ok(())
}

#[test]
fn test_mem_access_write_multiple_bytes() -> crate::Result<()> {
let mut mem_access = get_mem_access()?;
let mut memory = TestMemory::new()?;
let offset = 56;
{
let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
for i in 0..16 {
slice[offset + i] = i as u8;
}
}

let write_data = [0xAAu8; 16];
mem_access
memory
.access()
.write(&write_data, (BASE_VIRT + offset) as u64)
.unwrap();

let slice = unsafe { get_mmap_slice(&mut mem_access) };
let slice = unsafe { memory.mmap_slice() };
assert_eq!(slice[offset..offset + 16], write_data);
drop_mem_access(mem_access);

Ok(())
}
Expand Down
Loading
Loading