kata-containers/cloud-hypervisor
1
0
Fork 0
mirror of https://github.com/cloud-hypervisor/cloud-hypervisor.git synced 2026-08-05 02:19:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

misc: Mark memory region APIs as unsafe

Browse Source 
To ensure that struct sizes are the same on 32-bit and 64-bit, various
kernel APIs use __u64 (Rust u64) to represent userspace pointers.
Userspace is expected to cast pointers to __u64 before passing them to
the kernel, and cast kernel-provided __u64 to a pointer before using
them.  However, various safe APIs in Cloud Hypervisor took
caller-provided u64 values and passed them to syscalls that treat them
as userspace addresses.  Therefore, passing bad u64 values would cause
memory disclosure or corruption.  The memory region APIs are one example
of this, so mark them as unsafe.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This commit is contained in:
Demi Marie Obenour
2025-06-13 14:27:00 -04:00
committed by Rob Bradford
parent 00f0b9e42c
commit fdc19ad85e
12 changed files with 489 additions and 407 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
vmm/src/api/dbus/mod.rs
View File

@@ -356,7 +356,7 @@ pub fn start_dbus_thread(
apply_filter(&api_seccomp_filter)
.map_err(VmmError::ApplySeccompFilter)
.map_err(|e| {
error!("Error applying seccomp filter: {:?}", e);
error!("Error applying seccomp filter: {e:?}");
exit_evt.write(1).ok();
e
})?;
@@ -383,7 +383,7 @@ pub fn start_dbus_thread(
}
}
}
})
});
}))
.map_err(|_| {
error!("dbus-api thread panicked");

2
vmm/src/config.rs
View File

@@ -2521,7 +2521,7 @@ impl VmConfig {
#[cfg(target_arch = "x86_64")]
if self.debug_console.mode == ConsoleOutputMode::Tty {
tty_consoles.push("debug-console");
};
}
if tty_consoles.len() > 1 {
warn!("Using TTY output for multiple consoles: {tty_consoles:?}");
}

156
vmm/src/device_manager.rs
View File

@@ -817,32 +817,43 @@ impl DeviceRelocation for AddressManager {
if let Some(mut shm_regions) = virtio_dev.get_shm_regions()
&& shm_regions.addr.raw_value() == old_base
{
let mem_region = self.vm.make_user_memory_region(
shm_regions.mem_slot,
old_base,
shm_regions.len,
shm_regions.host_addr,
false,
false,
);
// SAFETY: TODO what are the invariants here?
unsafe {
// Remove old mapping
self.vm
.remove_user_memory_region(
shm_regions.mem_slot,
old_base,
shm_regions.len,
shm_regions.host_addr,
false,
false,
)
.map_err(|e| {
io::Error::other(format!(
"failed to remove user memory region: {e:?}"
))
})?;
}
self.vm.remove_user_memory_region(mem_region).map_err(|e| {
io::Error::other(format!("failed to remove user memory region: {e:?}"))
})?;
// Create new mapping by inserting new region to KVM.
let mem_region = self.vm.make_user_memory_region(
shm_regions.mem_slot,
new_base,
shm_regions.len,
shm_regions.host_addr,
false,
false,
);
self.vm.create_user_memory_region(mem_region).map_err(|e| {
io::Error::other(format!("failed to create user memory regions: {e:?}"))
})?;
// SAFETY: TODO what are the invariants here?
unsafe {
// Create new mapping by inserting new region to KVM.
self.vm
.create_user_memory_region(
shm_regions.mem_slot,
new_base,
shm_regions.len,
shm_regions.host_addr,
false,
false,
)
.map_err(|e| {
io::Error::other(format!(
"failed to create user memory regions: {e:?}"
))
})?;
}
// Update shared memory regions to reflect the new mapping.
shm_regions.addr = GuestAddress(new_base);
@@ -3240,12 +3251,14 @@ impl DeviceManager {
.map_err(DeviceManagerError::NewMmapRegion)?;
let host_addr: u64 = mmap_region.as_ptr() as u64;
let mem_slot = self
.memory_manager
.lock()
.unwrap()
.create_userspace_mapping(region_base, region_size, host_addr, false, false, false)
.map_err(DeviceManagerError::MemoryManager)?;
// SAFETY: host_addr points to region_size bytes of mmap-allocated memory.
let mem_slot = unsafe {
self.memory_manager
.lock()
.unwrap()
.create_userspace_mapping(region_base, region_size, host_addr, false, false, false)
.map_err(DeviceManagerError::MemoryManager)
}?;
let mapping = UserspaceMapping {
host_addr,
@@ -3997,13 +4010,16 @@ impl DeviceManager {
resources,
)?;
// SAFETY: TODO
// Note it is required to call 'add_pci_device()' in advance to have the list of
// mmio regions provisioned correctly
vfio_user_pci_device
.lock()
.unwrap()
.map_mmio_regions()
.map_err(DeviceManagerError::VfioUserMapRegion)?;
unsafe {
vfio_user_pci_device
.lock()
.unwrap()
.map_mmio_regions()
.map_err(DeviceManagerError::VfioUserMapRegion)
}?;
let mut node = device_node!(vfio_user_name, vfio_user_pci_device);
@@ -4722,17 +4738,22 @@ impl DeviceManager {
// Shutdown and remove the underlying virtio-device if present
if let Some(virtio_device) = virtio_device {
for mapping in virtio_device.lock().unwrap().userspace_mappings() {
self.memory_manager
.lock()
.unwrap()
.remove_userspace_mapping(
mapping.addr.raw_value(),
mapping.len,
mapping.host_addr,
mapping.mergeable,
mapping.mem_slot,
)
.map_err(DeviceManagerError::MemoryManager)?;
// SAFETY: userspace_mappings only has valid mappings.
// TODO: do not rely on the correctness of all the code in this file
// for this to hold.
unsafe {
self.memory_manager
.lock()
.unwrap()
.remove_userspace_mapping(
mapping.addr.raw_value(),
mapping.len,
mapping.host_addr,
mapping.mergeable,
mapping.mem_slot,
)
.map_err(DeviceManagerError::MemoryManager)
}?;
}
virtio_device.lock().unwrap().shutdown();
@@ -4984,7 +5005,7 @@ impl IvshmemOps for IvshmemHandler {
) -> Result<(Arc<GuestRegionMmap>, UserspaceMapping), IvshmemError> {
info!("Creating ivshmem mem region at 0x{start_addr:x}");
let region = MemoryManager::create_ram_region(
let region: Arc<GuestRegionMmap> = MemoryManager::create_ram_region(
&backing_file,
0,
GuestAddress(start_addr),
@@ -4998,19 +5019,21 @@ impl IvshmemOps for IvshmemHandler {
false,
)
.map_err(|_| IvshmemError::CreateUserMemoryRegion)?;
let mem_slot = self
.memory_manager
.lock()
.unwrap()
.create_userspace_mapping(
region.start_addr().0,
region.len(),
region.as_ptr() as u64,
false,
false,
false,
)
.map_err(|_| IvshmemError::CreateUserspaceMapping)?;
let mem_slot = {
let mut manager = self.memory_manager.lock().unwrap();
// SAFETY: guaranteed by GuestRegionMmap invariants
unsafe {
manager.create_userspace_mapping(
region.start_addr().0,
region.len(),
region.as_ptr() as u64,
false,
false,
false,
)
}
}
.map_err(|_| IvshmemError::CreateUserspaceMapping)?;
let mapping = UserspaceMapping {
host_addr: region.as_ptr() as u64,
mem_slot,
@@ -5022,17 +5045,18 @@ impl IvshmemOps for IvshmemHandler {
}
fn unmap_ram_region(&mut self, mapping: UserspaceMapping) -> Result<(), IvshmemError> {
self.memory_manager
.lock()
.unwrap()
.remove_userspace_mapping(
let mut manager = self.memory_manager.lock().unwrap();
// SAFETY: UserspaceMapping is valid due to other code being correct
unsafe {
manager.remove_userspace_mapping(
mapping.addr.raw_value(),
mapping.len,
mapping.host_addr,
mapping.mergeable,
mapping.mem_slot,
)
.map_err(|_| IvshmemError::RemoveUserspaceMapping)?;
}
.map_err(|_| IvshmemError::RemoveUserspaceMapping)?;
Ok(())
}
}

4
vmm/src/lib.rs
View File

@@ -1128,7 +1128,7 @@ impl Vmm {
return Err(MigratableError::MigrateSend(anyhow!(
"Live Migration is not supported when TDX is enabled"
)));
};
}
let amx = vm_config.lock().unwrap().cpus.features.amx;
let phys_bits = vm::physical_bits(
@@ -1265,7 +1265,7 @@ impl Vmm {
return Err(MigratableError::MigrateReceive(anyhow!(
"Live Migration is not supported when TDX is enabled"
)));
};
}
// We check the `CPUID` compatibility of between the source vm and destination, which is
// mostly about feature compatibility.

129
vmm/src/memory_manager.rs
View File

@@ -900,14 +900,18 @@ impl MemoryManager {
for (zone_id, regions) in list {
for (region, virtio_mem) in regions {
let slot = self.create_userspace_mapping(
region.start_addr().raw_value(),
region.len(),
region.as_ptr() as u64,
self.mergeable,
false,
self.log_dirty,
)?;
// SAFETY: regions only holds valid addresses.
// TODO: encapsulate this unsafety in a small part of the file.
let slot = unsafe {
self.create_userspace_mapping(
region.start_addr().raw_value(),
region.len(),
region.as_ptr() as u64,
self.mergeable,
false,
self.log_dirty,
)
}?;
let file_offset = if let Some(file_offset) = region.file_offset() {
file_offset.start()
@@ -958,18 +962,18 @@ impl MemoryManager {
arch::layout::UEFI_START,
)
.unwrap();
let uefi_mem_region = self.vm.make_user_memory_region(
uefi_mem_slot,
uefi_region.start_addr().raw_value(),
uefi_region.len(),
uefi_region.as_ptr() as u64,
false,
false,
);
self.vm
.create_user_memory_region(uefi_mem_region)
.map_err(Error::CreateUefiFlash)?;
unsafe {
self.vm
.create_user_memory_region(
uefi_mem_slot,
uefi_region.start_addr().raw_value(),
uefi_region.len(),
uefi_region.as_ptr() as u64,
false,
false,
)
.map_err(Error::CreateUefiFlash)?;
}
let uefi_flash =
GuestMemoryAtomic::new(GuestMemoryMmap::from_regions(vec![uefi_region]).unwrap());
@@ -1608,14 +1612,17 @@ impl MemoryManager {
)?;
// Map it into the guest
let slot = self.create_userspace_mapping(
region.start_addr().0,
region.len(),
region.as_ptr() as u64,
self.mergeable,
false,
self.log_dirty,
)?;
// SAFETY: create_ram_region only produces valid mappings.
let slot = unsafe {
self.create_userspace_mapping(
region.start_addr().0,
region.len(),
region.as_ptr() as u64,
self.mergeable,
false,
self.log_dirty,
)
}?;
self.guest_ram_mappings.push(GuestRamMapping {
gpa: region.start_addr().raw_value(),
size: region.len(),
@@ -1708,7 +1715,11 @@ impl MemoryManager {
self.memory_slot_allocator().next_memory_slot()
}
pub fn create_userspace_mapping(
/// # Safety
///
/// `userspace_addr` and `memory_size` must be and remain valid
/// until `remove_userspace_mapping` is called.
pub unsafe fn create_userspace_mapping(
&mut self,
guest_phys_addr: u64,
memory_size: u64,
@@ -1718,22 +1729,24 @@ impl MemoryManager {
log_dirty: bool,
) -> Result<u32, Error> {
let slot = self.allocate_memory_slot();
let mem_region = self.vm.make_user_memory_region(
slot,
guest_phys_addr,
memory_size,
userspace_addr,
readonly,
log_dirty,
);
info!(
"Creating userspace mapping: {guest_phys_addr:x} -> {userspace_addr:x} {memory_size:x}, slot {slot}"
);
self.vm
.create_user_memory_region(mem_region)
.map_err(Error::CreateUserMemoryRegion)?;
// SAFETY: promised by caller
unsafe {
self.vm
.create_user_memory_region(
slot,
guest_phys_addr,
memory_size,
userspace_addr,
readonly,
log_dirty,
)
.map_err(Error::CreateUserMemoryRegion)?;
}
// SAFETY: the address and size are valid since the
// mmap succeeded.
@@ -1781,7 +1794,16 @@ impl MemoryManager {
Ok(slot)
}
pub fn remove_userspace_mapping(
/// # Safety
///
/// `userspace_addr` and `memory_size` must have previously been passed
/// to `create_userspace_mapping`.
///
/// # Errors
///
/// If this function fails there is no way to clean up resources and you
/// should probably crash the process.
pub unsafe fn remove_userspace_mapping(
&mut self,
guest_phys_addr: u64,
memory_size: u64,
@@ -1789,18 +1811,19 @@ impl MemoryManager {
mergeable: bool,
slot: u32,
) -> Result<(), Error> {
let mem_region = self.vm.make_user_memory_region(
slot,
guest_phys_addr,
memory_size,
userspace_addr,
false, /* readonly -- don't care */
false, /* log dirty */
);
self.vm
.remove_user_memory_region(mem_region)
.map_err(Error::RemoveUserMemoryRegion)?;
// SAFETY: The caller promises that the parameters are correct.
unsafe {
self.vm
.remove_user_memory_region(
slot,
guest_phys_addr,
memory_size,
userspace_addr,
false, /* readonly -- don't care */
false, /* log dirty */
)
.map_err(Error::RemoveUserMemoryRegion)?;
}
// Mark the pages as unmergeable if there were previously marked as
// mergeable.

89
vmm/src/vm.rs
View File

@@ -3464,17 +3464,18 @@ mod unit_tests {
.expect("new VM creation failed");
for (index, region) in mem.iter().enumerate() {
let mem_region = vm.make_user_memory_region(
index as u32,
region.start_addr().raw_value(),
region.len(),
region.as_ptr() as u64,
false,
false,
);
vm.create_user_memory_region(mem_region)
// SAFETY: inputs are valid
unsafe {
vm.create_user_memory_region(
index as u32,
region.start_addr().raw_value(),
region.len(),
region.as_ptr() as u64,
false,
false,
)
.expect("Cannot configure guest memory");
}
}
mem.write_slice(&code, load_addr)
.expect("Writing code to memory failed");
@@ -3574,3 +3575,71 @@ mod unit_tests {
.unwrap();
}
}
#[cfg(all(feature = "kvm", target_arch = "x86_64"))]
#[test]
pub fn test_vm() {
use hypervisor::VmExit;
use vm_memory::{Address, GuestMemory, GuestMemoryRegion};
// This example based on https://lwn.net/Articles/658511/
let code = [
0xba, 0xf8, 0x03, /* mov $0x3f8, %dx */
0x00, 0xd8, /* add %bl, %al */
0x04, b'0', /* add $'0', %al */
0xee, /* out %al, (%dx) */
0xb0, b'\n', /* mov $'\n', %al */
0xee, /* out %al, (%dx) */
0xf4, /* hlt */
];
let mem_size = 0x1000;
let load_addr = GuestAddress(0x1000);
let mem = GuestMemoryMmap::from_ranges(&[(load_addr, mem_size)]).unwrap();
let hv = hypervisor::new().unwrap();
let vm = hv
.create_vm(HypervisorVmConfig::default())
.expect("new VM creation failed");
for (index, region) in mem.iter().enumerate() {
// SAFETY: parameters are correct
unsafe {
vm.create_user_memory_region(
index as u32,
region.start_addr().raw_value(),
region.len(),
region.as_ptr() as u64,
false,
false,
)
.expect("Cannot configure guest memory");
}
}
mem.write_slice(&code, load_addr)
.expect("Writing code to memory failed");
let mut vcpu = vm.create_vcpu(0, None).expect("new Vcpu failed");
let mut vcpu_sregs = vcpu.get_sregs().expect("get sregs failed");
vcpu_sregs.cs.base = 0;
vcpu_sregs.cs.selector = 0;
vcpu.set_sregs(&vcpu_sregs).expect("set sregs failed");
let mut vcpu_regs = vcpu.get_regs().expect("get regs failed");
vcpu_regs.set_rip(0x1000);
vcpu_regs.set_rax(2);
vcpu_regs.set_rbx(3);
vcpu_regs.set_rflags(2);
vcpu.set_regs(&vcpu_regs).expect("set regs failed");
loop {
match vcpu.run().expect("run failed") {
VmExit::Reset => {
println!("HLT");
break;
}
VmExit::Ignore => {}
r => panic!("unexpected exit reason: {r:?}"),
}
}
}