Skip to content

Commit 46a4b51

Browse files
committed
Fix bug in virt_to_phys that affects multi-page translation
- The max virtual address was calculated based on the vmin (start of page), not the actual virtual address provided Signed-off-by: Doru Blânzeanu <dblnz@pm.me>
1 parent 44b770f commit 46a4b51

1 file changed

Lines changed: 53 additions & 3 deletions

File tree

  • src/hyperlight_common/src/arch/amd64

src/hyperlight_common/src/arch/amd64/vmem.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,9 +537,13 @@ pub unsafe fn virt_to_phys<'a, Op: TableReadOps + 'a>(
537537
address: u64,
538538
len: u64,
539539
) -> impl Iterator<Item = Mapping> + 'a {
540-
// Undo sign-extension, and mask off any sub-page bits
541-
let vmin = (address & ((1u64 << VA_BITS) - 1)) & !(PAGE_SIZE as u64 - 1);
542-
let vmax = core::cmp::min(vmin + len, 1u64 << VA_BITS);
540+
// Undo sign-extension
541+
let addr = address & ((1u64 << VA_BITS) - 1);
542+
// Mask off any sub-page bits
543+
let vmin = addr & !(PAGE_SIZE as u64 - 1);
544+
// Calculate the maximum virtual address we need to look at based on the starting
545+
// address and length ensuring we don't go past the end of the address space
546+
let vmax = core::cmp::min(addr + len, 1u64 << VA_BITS);
543547
modify_ptes::<47, 39, Op, _>(MapRequest {
544548
table_base: op.as_ref().root_table(),
545549
vmin,
@@ -920,6 +924,52 @@ mod tests {
920924
assert_eq!(mapping.phys_base, 0x1000);
921925
}
922926

927+
#[test]
928+
fn test_virt_to_phys_unaligned_virt_and_across_pages_len() {
929+
let ops = MockTableOps::new();
930+
let mapping = Mapping {
931+
phys_base: 0x1000,
932+
virt_base: 0x1000,
933+
len: 2 * PAGE_SIZE as u64, // 2 page
934+
kind: MappingKind::Basic(BasicMapping {
935+
readable: true,
936+
writable: true,
937+
executable: false,
938+
}),
939+
};
940+
941+
unsafe { map(&ops, mapping) };
942+
943+
let mappings = unsafe { virt_to_phys(&ops, 0x1F00, 0x300).collect::<Vec<_>>() };
944+
assert_eq!(mappings.len(), 2, "Should return 2 mappings for 2 pages");
945+
assert_eq!(mappings[0].phys_base, 0x1000);
946+
assert_eq!(mappings[1].phys_base, 0x2000);
947+
}
948+
949+
#[test]
950+
fn test_virt_to_phys_unaligned_virt_and_multiple_page_len() {
951+
let ops = MockTableOps::new();
952+
let mapping = Mapping {
953+
phys_base: 0x1000,
954+
virt_base: 0x1000,
955+
len: PAGE_SIZE as u64 * 2 + 0x200, // 2 page + 512 bytes
956+
kind: MappingKind::Basic(BasicMapping {
957+
readable: true,
958+
writable: true,
959+
executable: false,
960+
}),
961+
};
962+
963+
unsafe { map(&ops, mapping) };
964+
965+
let mappings =
966+
unsafe { virt_to_phys(&ops, 0x1234, PAGE_SIZE as u64 * 2 + 0x10).collect::<Vec<_>>() };
967+
assert_eq!(mappings.len(), 3, "Should return 3 mappings for 3 pages");
968+
assert_eq!(mappings[0].phys_base, 0x1000);
969+
assert_eq!(mappings[1].phys_base, 0x2000);
970+
assert_eq!(mappings[2].phys_base, 0x3000);
971+
}
972+
923973
#[test]
924974
fn test_virt_to_phys_perms() {
925975
let test = |kind| {

0 commit comments

Comments
 (0)