Validate Guest Address Ranges for Overlapping Regions in map_region

[Richard-Durkee]

Add overlap validation to HyperlightVm::map_region to enforce the safety contract documented on VirtualMachine::map_memory, which requires non-overlapping regions.

Checks the new region against existing dynamically mapped regions (mmap_regions), the snapshot region (starting at BASE_ADDRESS), and the scratch region (at the top of the guest physical address space).

Adds Overlapping variant to MapRegionError with a descriptive message showing both the new and conflicting ranges.

Also adds early validation at the MultiUseSandbox::map_region level to reject invalid input before side effects (snapshot reset) occur.

Tested on KVM (c8i.2xlarge with nested virtualization enabled).

Closes #1289

[ludfjig]

thanks for your contribution!

I think we can drop the validation in MultiUseSandbox::map_region entirely and rely on the check inside HyperlightVm::map_region, then reorder so the call happens before the snapshot reset:

unsafe { self.vm.map_region(rgn) }.map_err(HyperlightVmError::MapRegion)?;
self.snapshot = None;
self.mem_mgr.mapped_rgns += 1;

The same pattern can probably applies to map_file_cow too I think. What do you think about this?

[Richard-Durkee]

thanks for your contribution!

I think we can drop the validation in MultiUseSandbox::map_region entirely and rely on the check inside HyperlightVm::map_region, then reorder so the call happens before the snapshot reset:

unsafe { self.vm.map_region(rgn) }.map_err(HyperlightVmError::MapRegion)?;
self.snapshot = None;
self.mem_mgr.mapped_rgns += 1;

The same pattern can probably applies to map_file_cow too I think. What do you think about this?

Thanks for the feedback!

This makes sense to me. I just pushed again and included these suggestions. Please let me know if there are any other changes you'd like.

[ludfjig]

thanks for your contribution!
I think we can drop the validation in MultiUseSandbox::map_region entirely and rely on the check inside HyperlightVm::map_region, then reorder so the call happens before the snapshot reset:

unsafe { self.vm.map_region(rgn) }.map_err(HyperlightVmError::MapRegion)?;
self.snapshot = None;
self.mem_mgr.mapped_rgns += 1;

The same pattern can probably applies to map_file_cow too I think. What do you think about this?

Thanks for the feedback!

This makes sense to me. I just pushed again and included these suggestions. Please let me know if there are any other changes you'd like.

Looks great! Just one minor thing I just realized the page-table "tail" of compacted snapshots are not mapped into the vm, so the could be less than mem_size(). I think switching to

#[cfg(not(unshared_snapshot_mem))]
let snap_end = snap_start + snapshot.guest_mapped_size();
#[cfg(unshared_snapshot_mem)]
let snap_end = snap_start + snapshot.mem_size();

should do it. Thanks!

[Richard-Durkee]

Ah, okay -- I just learned about compacted snapshots. Pushed the change. Thanks again for the help.