Remove Ptr, RawPtr, GuestPtr, Offset, AddressSpace, GuestAddressSpace #767
Conversation
ludfjig
requested review from
danbugs,
dblnz,
devigned,
syntactically,
marosset,
jprendes and
simongdavies
as code owners
ludfjig
added
the
kind/enhancement
|
In preparation for making Hyperlight more portable, one small thing from a commit series near the beginning of my big guest-CoW branch incidentally adds the types |
Yes, I think that would make a lot of sense |
Would it be valuable to introduce the type in a small PR like this? then re-use them in the guest-COW PR? Or wait till we bring that in and rebase this? |
Certain drivers used RawPtr and others used u64. To unify these in preperation for a refactor, they are replaced them with u64 instead. The removed types were mostly unused and provided minimal benefit over a simple u64. In the future we should think about introducing some type to abstract over phys/virtual addresses etc instead. Signed-off-by: Ludvig Liljenberg <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR removes several pointer wrapper types (Ptr, RawPtr, GuestPtr, Offset, AddressSpace, GuestAddressSpace) and replaces them with simple u64 values to unify different driver implementations and reduce conversion overhead.
- Removes complex pointer abstraction types that provided minimal benefit over raw
u64 - Simplifies memory management by eliminating type conversions between pointer types
- Unifies driver implementations to consistently use
u64for addresses
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_host/src/sandbox/uninitialized_evolve.rs | Removes pointer type usage, simplifies address calculations and removes address validation |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Changes dispatch pointer from RawPtr to u64 |
| src/hyperlight_host/src/mem/ptr_offset.rs | Complete file removal - eliminates Offset type |
| src/hyperlight_host/src/mem/ptr_addr_space.rs | Complete file removal - eliminates address space types |
| src/hyperlight_host/src/mem/ptr.rs | Complete file removal - eliminates pointer wrapper types |
| src/hyperlight_host/src/mem/mod.rs | Removes module declarations for deleted pointer types |
| src/hyperlight_host/src/mem/mgr.rs | Converts address fields to u64 and simplifies memory operations |
| src/hyperlight_host/src/mem/exe.rs | Changes entrypoint return type from Offset to u64 |
| src/hyperlight_host/src/hypervisor/mod.rs | Updates trait signatures to use u64 instead of pointer types |
| src/hyperlight_host/src/hypervisor/kvm.rs | Simplifies register setup by removing pointer conversions |
| src/hyperlight_host/src/hypervisor/hyperv_windows.rs | Updates to use u64 addresses directly |
| src/hyperlight_host/src/hypervisor/hyperv_linux.rs | Removes pointer type usage in hypervisor operations |
| src/hyperlight_host/src/error.rs | Removes error variant for pointer validation |
| ); | ||
| } | ||
| let pml4_ptr = SandboxMemoryLayout::PML4_OFFSET as u64; | ||
| let entrypoint_ptr = mgr.load_addr + mgr.entrypoint_offset; |
There was a problem hiding this comment.
The removal of address validation checks between base_ptr, pml4_ptr, and entrypoint_ptr could allow invalid memory configurations to proceed undetected. The original code verified that base_ptr == pml4_ptr and entrypoint_ptr > pml4_ptr. Consider adding equivalent validation for the new u64 values to ensure memory layout correctness.
Copilot uses AI. Check for mistakes.
| ) -> Result<usize> { | ||
| let rsp = self.layout.get_top_of_user_stack_offset() | ||
| + SandboxMemoryLayout::BASE_ADDRESS | ||
| + self.layout.stack_size |
There was a problem hiding this comment.
The arithmetic operations are now performed on usize values without overflow checking, whereas the original code used checked arithmetic via u64::checked_add(). This could lead to silent overflow in stack pointer calculations. Consider adding overflow checks or using saturating arithmetic.
Copilot uses AI. Check for mistakes.
Certain drivers used RawPtr and others used u64. To unify these in preparation for a refactor, they are replaced them with u64 instead.
The removed types were mostly unused and provided minimal benefit over a simple u64. Instead they just introduced overhead of converting to and from the pointer types. In the future we should think about introducing some type to abstract over phys/virtual addresses etc, or provide new pointer types with some security benefit like bounds checking or something else useful.
Related to #429