Skip to content
This repository has been archived by the owner on Sep 1, 2024. It is now read-only.

Commit

Permalink
Bug Fix: Crash due to the order of function calls
Browse files Browse the repository at this point in the history
- Noticed an incorrect parameter passed to `is_large_page` as well.

- Added conversation in `is_large_page`

- Note: We must map the large page to the pre-allocated page table before accessing it AND we must map the guest page to the shadow page before accessing it.
  • Loading branch information
memN0ps committed Jun 10, 2024
1 parent e2ef8c7 commit 140b6d2
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 13 deletions.
1 change: 1 addition & 0 deletions hypervisor/src/intel/ept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl Ept {
/// `true` if the guest physical address is part of a large 2MB page, otherwise `false`.
pub fn is_large_page(&self, guest_pa: u64) -> bool {
let guest_pa = VAddr::from(guest_pa);
let guest_pa = guest_pa.align_down_to_base_page();
let pdpt_index = pdpt_index(guest_pa);
let pd_index = pd_index(guest_pa);
let pde = &self.pd[pdpt_index].0.entries[pd_index];
Expand Down
23 changes: 13 additions & 10 deletions hypervisor/src/intel/hooks/hook_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,20 +209,14 @@ impl HookManager {
debug!("Guest large page PA: {:#x}", guest_large_page_pa.as_u64());

// 1. Map the large page to the pre-allocated page table, if it hasn't been mapped already.
// We must map the large page to the pre-allocated page table before accessing it.
debug!("Mapping large page");
vm.hook_manager.memory_manager.map_large_page_to_pt(guest_large_page_pa.as_u64())?;

let shadow_page_pa = {
let shadow_page_pa = vm
.hook_manager
.memory_manager
.get_shadow_page_as_ptr(guest_page_pa.as_u64())
.ok_or(HypervisorError::ShadowPageNotFound)?;
PAddr::from(shadow_page_pa)
};

// 2. Check if the large page has already been split. If not, split it into 4KB pages.
if vm.primary_ept.is_large_page(guest_large_page_pa.as_u64()) {
debug!("Checking if large page has already been split");
if vm.primary_ept.is_large_page(guest_page_pa.as_u64()) {
// We must map the large page to the pre-allocated page table before accessing it.
let pre_alloc_pt = vm
.hook_manager
.memory_manager
Expand All @@ -236,6 +230,7 @@ impl HookManager {
// 3. Check if the guest page is already processed. If not, map the guest page to the shadow page.
// Ensure the memory manager maintains a set of processed guest pages to track this mapping.
if !vm.hook_manager.memory_manager.is_guest_page_processed(guest_page_pa.as_u64()) {
// We must map the guest page to the shadow page before accessing it.
debug!("Mapping guest page and shadow page");
vm.hook_manager.memory_manager.map_guest_to_shadow_page(
guest_page_pa.as_u64(),
Expand All @@ -245,6 +240,14 @@ impl HookManager {
function_hash,
)?;

// We must map the guest page to the shadow page before accessing it.
let shadow_page_pa = PAddr::from(
vm.hook_manager
.memory_manager
.get_shadow_page_as_ptr(guest_page_pa.as_u64())
.ok_or(HypervisorError::ShadowPageNotFound)?,
);

// 4. Copy the guest page to the shadow page if it hasn't been copied already, ensuring the shadow page contains the original function code.
debug!("Copying guest page to shadow page: {:#x}", guest_page_pa.as_u64());
Self::unsafe_copy_guest_to_shadow(guest_page_pa, shadow_page_pa);
Expand Down
3 changes: 2 additions & 1 deletion hypervisor/src/intel/hooks/memory_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl MemoryManager {
self.active_mappings
.insert(guest_page_pa, mapping)
.map_err(|_| HypervisorError::ActiveMappingError)?;
trace!("Mapping added successfully");
trace!("Guest page mapped to shadow page successfully");
} else {
error!("No free pages available for mapping");
return Err(HypervisorError::OutOfMemory);
Expand All @@ -179,6 +179,7 @@ impl MemoryManager {
self.large_pt_mappings
.insert(guest_large_page_pa, pt)
.map_err(|_| HypervisorError::ActiveMappingError)?;
trace!("Large page mapped to page table successfully");
} else {
error!("No free page tables available for mapping");
return Err(HypervisorError::OutOfMemory);
Expand Down
2 changes: 0 additions & 2 deletions hypervisor/src/intel/vmexit/cpuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ pub fn handle_cpuid(vm: &mut Vm) -> Result<ExitType, HypervisorError> {
trace!("CPUID leaf 0x2 detected (Cache Information).");
if vm.hook_manager.has_cpuid_cache_info_been_called == false {
// Test UEFI boot-time hooks
/*
if let Some(mut kernel_hook) = vm.hook_manager.kernel_hook.take() {
info!("Hooking NtQuerySystemInformation with syscall number 0x36");
kernel_hook.enable_kernel_ept_hook(
Expand Down Expand Up @@ -159,7 +158,6 @@ pub fn handle_cpuid(vm: &mut Vm) -> Result<ExitType, HypervisorError> {
} else {
return Err(HypervisorError::KernelHookMissing);
}
*/
}
}
leaf if leaf == CpuidLeaf::ExtendedFeatureInformation as u32 => {
Expand Down

0 comments on commit 140b6d2

Please sign in to comment.