ASLR: Lay down the foundations #725

n0toose · 2024-07-06T17:35:02Z

Move x86_64-related paging code to src/arch/x86_64/paging
Tests: x86_64-related paging tests should use a guest_address that is
not 0
Tests: Move them in separate files, use appropriate 'use' directives
Tests: Use init_guest_mem wrapper in test_virt_to_phys
Fix kernel memory loading
Add guest_address getter in UhyveVm
Change names of constants to clarify their purpose
Remove pagetable_l0 from virt_to_phys function
Various cargo fmt-related changes
aarch64: Blindly replace constant names and similar RAM_START change

We currently rely on guest_address in MmapMemory to calculate the
offsets during the initialization of the VM and when converting
virtual addresses to physical addresses. The latter case is intended
to be temporary - we should read the value from the CR3 register at
a later point.

Although this current revision does work with relocatable binaries, it
is not making use of this functionality just yet.

Fixes #719.
Fixes #713.

n0toose · 2024-07-06T17:44:49Z

This change was not tested on macOS (x86_64, aarch64) because of a lack of hardware, and, in the case of macOS (x86_64), there's no CI - I may have to bring up a VM for this, but it will take at least an hour to prepare it. This involved some guess work in both aarch64 and x86_64 - because of this, I will explicitly disable the upcoming ASLR feature in both of these cases.

The change looks artificially bigger than it is because of the x86_64 paging code being moved to a separate file. This is easy for me to undo.

This is a rebased and somewhat "cleaned up" version of https://github.com/n0toose/uhyve/tree/make-it-boot (might help with the review), plus lots of tiny changes:
- I had to fix lots of things in aarch64 (very often, formatting mistakes caught by cargo fmt).
- Everything aarch64 and macOS-related is not part of the tree
Despite the refactoring, the guest_address should always be zero for the time being. The work to change this was done here: ASLR: PoC for generating random address before Uhyve launches #711
These two changes were kept separate because of the very big scope of this change - and because some further thought has to be poured into the case of the hello_c case.

codecov · 2024-07-06T19:21:47Z

Codecov Report

Attention: Patch coverage is 96.98492% with 6 lines in your changes missing coverage. Please review.

Project coverage is 68.86%. Comparing base (e6ffffa) to head (08bfcfd).
Report is 10 commits behind head on main.

Files	Patch %	Lines
src/hypercall.rs	42.85%	4 Missing ⚠️
src/arch/x86_64/paging/mod.rs	99.15%	1 Missing ⚠️
src/vm.rs	95.65%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #725      +/-   ##
==========================================
+ Coverage   68.17%   68.86%   +0.68%     
==========================================
  Files          20       21       +1     
  Lines        2319     2370      +51     
==========================================
+ Hits         1581     1632      +51     
  Misses        738      738

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

n0toose

I spent 15 minutes trying to give more context behind the macOS changes on x86_64 and aarch64 to help the review process - where I tried to change the least amount of things possible until the compiler stopped being mad at me (and as a way of double-checking the changes).

The goal is to have this changed merged directly into main or in a feature branch, so that I can continue working on top of these foundations. Hope this helps.

(EDIT: I marked all of these comments as "resolved" to keep the PR log cleaner.)

src/arch/aarch64/mod.rs

src/arch/x86_64/mod.rs

src/arch/aarch64/mod.rs

src/macos/aarch64/vcpu.rs

src/macos/x86_64/vcpu.rs

src/vm.rs

src/arch/x86_64/mod.rs

n0toose · 2024-07-10T20:20:39Z

src/vm.rs

+		// TODO: guest_address is only taken into account on Linux platforms.
+		// TODO: Before changing this, fix init_guest_mem in `src/arch/aarch64/mod.rs`
+		#[cfg(target_os = "linux")]
+		#[cfg(not(target_arch = "x86_64"))]


TODO: Remove this #[cfg... mess and replace it with a feature as soon as

https://github.com/hermit-os/uhyve/pull/725/files#r1672908739 is answered.

src/linux/x86_64/kvm_cpu.rs

jounathaen · 2024-07-16T12:56:17Z

src/linux/x86_64/kvm_cpu.rs

+		entry_point: u64,
+		stack_address: u64,
+		guest_address: u64,


This could be GuestPhysicalAddresses, whilst we are on it, right?

Partially resolved, stack_address and entry_point possibly unnecessary?

jounathaen · 2024-07-16T12:57:22Z

src/macos/x86_64/vcpu.rs

@@ -233,7 +233,7 @@ impl XhyveCpu {
 		Ok(())
 	}

-	fn setup_system_64bit(&mut self) -> Result<(), xhypervisor::Error> {
+	fn setup_system_64bit(&mut self, guest_address: u64) -> Result<(), xhypervisor::Error> {


Please add a doc comment on what guest_address is

n0toose · 2024-07-22T18:19:09Z

src/vm.rs

@@ -219,7 +263,7 @@ impl<VCpuType: VirtualCPU> UhyveVm<VCpuType> {
 		};
 		unsafe {
 			let raw_boot_info_ptr =
-				self.mem.host_address.add(BOOT_INFO_ADDR.as_u64() as usize) as *mut RawBootInfo;
+				self.mem.host_address.add(BOOT_INFO_ADDR_OFFSET as usize) as *mut RawBootInfo;
 			*raw_boot_info_ptr = RawBootInfo::from(boot_info);
 			self.boot_info = raw_boot_info_ptr;
 		}


In the following line (270-271, GitHub won't let me comment) the self.stack_address = ... check is possibly unsound?

n0toose

Some notes on the latest revision which does not fully work...

src/vm.rs

n0toose · 2024-07-22T20:12:39Z

src/vm.rs

@@ -152,6 +231,10 @@ impl<VCpuType: VirtualCPU> UhyveVm<VCpuType> {
 		self.stack_address
 	}

+	pub fn guest_address(&self) -> u64 {
+		self.guest_address.as_u64()


This should be fixed, and start_address should be appended.

src/vm.rs

n0toose · 2024-11-19T20:03:46Z

btw, last time i checked, the function i created to generate a random address was pretty trash - this may have to be revised before a merge (or merged with a feature)

- Tests: x86_64-related paging tests should use a guest_address that is not 0 - Tests: Move them in separate files, use appropriate 'use' directives - Tests: Use init_guest_mem wrapper in test_virt_to_phys - Fix kernel memory loading - Add guest_address getter in UhyveVm - Change names of constants to clarify their purpose - Use u64 for arch::RAM_START instead of GuestVirtAddr - Remove pagetable_l0 from virt_to_phys function - Various `cargo fmt`-related changes - aarch64: Blindly replace constant names and similar RAM_START change We currently rely on guest_address in MmapMemory to calculate the offsets during the initialization of the VM and when converting virtual addresses to physical addresses. The latter case is intended to be temporary - we should read the value from the CR3 register at a later point, but this is too complex for the time being because of the different architectures. Although this current revision does work with relocatable binaries, it is not making use of this functionality _just_ yet. Fixes hermit-os#719. Co-authored-by: Jonathan <[email protected]>

This will be used by other functions for now, instead of relying on mem.guest_address. guest_address should only be a GuestPhysAddr internally for now, as other functions in places like src/linux/x86_64/kvm_cpu.rs use it.

Removes the start_address entry and uses the existing offset entry instead. Plus some `cargo fmt` changes

…ry is not mapped

…memory

This comment was marked as outdated.

Sign in to view

n0toose mentioned this pull request Jul 6, 2024

cargo test --test fs-test fails on main #726

Closed

n0toose force-pushed the aslr branch 3 times, most recently from 80d7f57 to 28b8dcf Compare July 6, 2024 18:55

n0toose commented Jul 8, 2024

View reviewed changes

n0toose force-pushed the aslr branch 2 times, most recently from 3589fe8 to 08bfcfd Compare July 8, 2024 12:47

n0toose commented Jul 10, 2024

View reviewed changes

src/arch/x86_64/mod.rs Outdated Show resolved Hide resolved

n0toose commented Jul 10, 2024

View reviewed changes

jounathaen reviewed Jul 16, 2024

View reviewed changes

src/linux/x86_64/kvm_cpu.rs Outdated Show resolved Hide resolved

jounathaen reviewed Jul 16, 2024

View reviewed changes

n0toose force-pushed the aslr branch from 8077f6d to 3775efa Compare July 22, 2024 17:50

n0toose commented Jul 22, 2024

View reviewed changes

n0toose commented Jul 23, 2024

View reviewed changes

src/vm.rs Outdated Show resolved Hide resolved

jounathaen mentioned this pull request Jul 23, 2024

ASLR: PoC for generating random address before Uhyve launches #711

Closed

n0toose mentioned this pull request Jul 24, 2024

Implement FromStr / something that allows storing byte slices / yoke for KernelObject hermit-os/hermit-entry#36

Open

n0toose marked this pull request as draft July 28, 2024 14:03

jounathaen self-assigned this Aug 5, 2024

jounathaen force-pushed the aslr branch from 5f8e495 to d88aabd Compare August 5, 2024 16:22

n0toose mentioned this pull request Aug 17, 2024

Implement ASLR #707

Open

jounathaen force-pushed the aslr branch 2 times, most recently from dc70cc4 to 258784a Compare September 18, 2024 17:01

jounathaen mentioned this pull request Sep 18, 2024

Uhyve Interface V2 #754

Draft

n0toose mentioned this pull request Sep 18, 2024

feat(vm): change UhyveVm struct parameters #755

Closed

jounathaen mentioned this pull request Sep 19, 2024

Enhanced uhyve memory detection hermit-os/kernel#1391

Open

jounathaen force-pushed the aslr branch from 258784a to 45c207f Compare November 19, 2024 19:54

jounathaen force-pushed the aslr branch 2 times, most recently from 3159f15 to d4eb1e1 Compare November 20, 2024 10:56

jounathaen force-pushed the aslr branch from d4eb1e1 to 38021d6 Compare December 2, 2024 13:57

jounathaen mentioned this pull request Dec 2, 2024

Use of GuestPhysAddr & added HugePageAlignedMem struct #826

Merged

jounathaen force-pushed the aslr branch 2 times, most recently from ae580da to 4922db9 Compare December 3, 2024 17:11

n0toose and others added 14 commits December 10, 2024 19:51

ASLR: Use OnceLock for storing guest_address

f0ded87

This will be used by other functions for now, instead of relying on mem.guest_address. guest_address should only be a GuestPhysAddr internally for now, as other functions in places like src/linux/x86_64/kvm_cpu.rs use it.

WIP ASLR work

d1d8f26

ASLR: Remove superfluous start_address entry

75b717a

Removes the start_address entry and uses the existing offset entry instead. Plus some `cargo fmt` changes

x86_64: allow MMIO reads for MP floating table search when lower memo…

97bf0f8

…ry is not mapped

x86_64: reworked pagetable creation: Only map precisely the required …

700425d

…memory

gdb: don't panic on invalid memory reads

72c1a52

gdb: reduced debug interrupt print from info to trace

a05ba1f

MmapMemory: fixed incorrect memory bounds check

a3825b2

Fixed ASLR: Use random address and fixes some mistakes

4ca91d5

Minor: Stack size constant is now in hex

b54615e

Minor: Nicer expect print on incorrect SerialWriteBuffer hypercall

9f9063b

Minor: Added doc comment to UhyveVm.entry_point

5215dd8

removed GUEST_ADDRESS global static

e203338

jounathaen force-pushed the aslr branch from 4922db9 to e203338 Compare December 10, 2024 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASLR: Lay down the foundations #725

ASLR: Lay down the foundations #725

n0toose commented Jul 6, 2024 •

edited

Loading

n0toose commented Jul 6, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Jul 6, 2024 •

edited

Loading

n0toose left a comment •

edited

Loading

n0toose Jul 10, 2024 •

edited

Loading

jounathaen Jul 16, 2024

n0toose Jul 22, 2024

jounathaen Jul 16, 2024

n0toose Jul 22, 2024 •

edited

Loading

n0toose left a comment

n0toose Jul 22, 2024

n0toose commented Nov 19, 2024

ASLR: Lay down the foundations #725

Are you sure you want to change the base?

ASLR: Lay down the foundations #725

Conversation

n0toose commented Jul 6, 2024 • edited Loading

n0toose commented Jul 6, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Jul 6, 2024 • edited Loading

Codecov Report

n0toose left a comment • edited Loading

Choose a reason for hiding this comment

n0toose Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

jounathaen Jul 16, 2024

Choose a reason for hiding this comment

n0toose Jul 22, 2024

Choose a reason for hiding this comment

jounathaen Jul 16, 2024

Choose a reason for hiding this comment

n0toose Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

n0toose left a comment

Choose a reason for hiding this comment

n0toose Jul 22, 2024

Choose a reason for hiding this comment

n0toose commented Nov 19, 2024

n0toose commented Jul 6, 2024 •

edited

Loading

n0toose commented Jul 6, 2024 •

edited

Loading

codecov bot commented Jul 6, 2024 •

edited

Loading

n0toose left a comment •

edited

Loading

n0toose Jul 10, 2024 •

edited

Loading

n0toose Jul 22, 2024 •

edited

Loading