From b975546c3ffa05a65e6b725d39baad593cbd3052 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 3 Nov 2021 09:52:21 +0000 Subject: [PATCH] vm-memory: mlock2(MLOCK_ONFAULT) guest memory for protected VMs By default, the memory of a protected VM is inaccessible to the host and crosvm. Consequently, attempts to access guest memory are fatal and must be avoided in order for the guest to run. Mlock guest pages as they are faulted in for protected VMs, ensuring that the host doesn't try to age or swap them out as a result of memory pressure. Bug: b:204298056 Test: cargo test on x86 and arm64 Cc: Quentin Perret Cc: Andrew Walbran Signed-off-by: Will Deacon Change-Id: I618ec1e8b1136a47a8b3ef563e45bc41d75ab517 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3257689 Tested-by: kokoro Reviewed-by: Chirantan Ekbote --- common/base/src/mmap.rs | 4 ++++ common/sys_util/src/mmap.rs | 19 +++++++++++++++++++ src/linux.rs | 7 ++++++- src/plugin/mod.rs | 7 +++++-- vm_memory/src/guest_memory.rs | 18 +++++++++++++++--- 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/common/base/src/mmap.rs b/common/base/src/mmap.rs index 16444fb054..00e8a7c6a6 100644 --- a/common/base/src/mmap.rs +++ b/common/base/src/mmap.rs @@ -42,6 +42,10 @@ impl MemoryMapping { self.mapping.use_hugepages() } + pub fn mlock_on_fault(&self) -> Result<()> { + self.mapping.mlock_on_fault() + } + pub fn read_to_memory( &self, mem_offset: usize, diff --git a/common/sys_util/src/mmap.rs b/common/sys_util/src/mmap.rs index a3ecbbde63..2cf6dea9df 100644 --- a/common/sys_util/src/mmap.rs +++ b/common/sys_util/src/mmap.rs @@ -19,6 +19,8 @@ use data_model::DataInit; use crate::{errno, pagesize}; +const MLOCK_ONFAULT: libc::c_int = 1; + #[sorted] #[derive(Debug, thiserror::Error)] pub enum Error { @@ -415,6 +417,23 @@ impl MemoryMapping { } } + /// Mlock the guest pages as they are faulted in + pub fn mlock_on_fault(&self) -> Result<()> { + let ret = unsafe { + // TODO: Switch to libc::mlock2 once https://github.com/rust-lang/libc/pull/2525 lands + libc::syscall( + libc::SYS_mlock2, + self.as_ptr() as *mut libc::c_void, + self.size(), + MLOCK_ONFAULT, + ) + }; + if ret == -1 { + return Err(Error::SystemCallFailed(errno::Error::last())); + } + Ok(()) + } + /// Calls msync with MS_SYNC on the mapping. pub fn msync(&self) -> Result<()> { // This is safe since we use the exact address and length of a known diff --git a/src/linux.rs b/src/linux.rs index cc4a77850f..cc47f6064a 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -2419,7 +2419,12 @@ pub fn run_config(cfg: Config) -> Result<()> { if components.hugepages { mem_policy |= MemoryPolicy::USE_HUGEPAGES; } - guest_mem.set_memory_policy(mem_policy); + if components.protected_vm == ProtectionType::Protected { + mem_policy |= MemoryPolicy::MLOCK_ON_FAULT; + } + guest_mem + .set_memory_policy(mem_policy) + .context("failed to set guest memory policy")?; let kvm = Kvm::new_with_path(&cfg.kvm_device_path).context("failed to create kvm")?; let vm = KvmVm::new(&kvm, guest_mem).context("failed to create vm")?; let vm_clone = vm.try_clone().context("failed to clone vm")?; diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 79880baa45..de43141062 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -35,7 +35,7 @@ use base::{ use kvm::{Cap, Datamatch, IoeventAddress, Kvm, Vcpu, VcpuExit, Vm}; use minijail::{self, Minijail}; use net_util::{Error as TapError, Tap, TapT}; -use vm_memory::{GuestMemory, MemoryPolicy}; +use vm_memory::{GuestMemory, GuestMemoryError, MemoryPolicy}; use self::process::*; use self::vcpu::*; @@ -138,6 +138,8 @@ pub enum Error { RootNotDir, #[error("failed to set gidmap for jail: {0}")] SetGidMap(minijail::Error), + #[error("failed to set guest memory policy: {0}")] + SetMemoryPolicy(GuestMemoryError), #[error("failed to set uidmap for jail: {0}")] SetUidMap(minijail::Error), #[error("process {pid} died with signal {signo}, status {status}, and code {code}")] @@ -695,7 +697,8 @@ pub fn run_config(cfg: Config) -> Result<()> { if cfg.hugepages { mem_policy |= MemoryPolicy::USE_HUGEPAGES; } - mem.set_memory_policy(mem_policy); + mem.set_memory_policy(mem_policy) + .map_err(Error::SetMemoryPolicy)?; let kvm = Kvm::new_with_path(&cfg.kvm_device_path).map_err(Error::CreateKvm)?; let mut vm = Vm::new(&kvm, mem).map_err(Error::CreateVm)?; vm.create_irq_chip().map_err(Error::CreateIrqChip)?; diff --git a/vm_memory/src/guest_memory.rs b/vm_memory/src/guest_memory.rs index b3477258a2..9052b21a55 100644 --- a/vm_memory/src/guest_memory.rs +++ b/vm_memory/src/guest_memory.rs @@ -40,6 +40,8 @@ pub enum Error { MemoryAddSealsFailed(SysError), #[error("failed to create shm region")] MemoryCreationFailed(SysError), + #[error("failed to lock {0} bytes of guest memory: {1}")] + MemoryLockingFailed(usize, MmapError), #[error("failed to map guest memory: {0}")] MemoryMappingFailed(MmapError), #[error("shm regions must be page aligned")] @@ -63,6 +65,7 @@ pub type Result = result::Result; bitflags! { pub struct MemoryPolicy: u32 { const USE_HUGEPAGES = 1; + const MLOCK_ON_FAULT = 2; } } @@ -293,16 +296,25 @@ impl GuestMemory { } /// Handles guest memory policy hints/advices. - pub fn set_memory_policy(&self, mem_policy: MemoryPolicy) { - if mem_policy.contains(MemoryPolicy::USE_HUGEPAGES) { - for (_, region) in self.regions.iter().enumerate() { + pub fn set_memory_policy(&self, mem_policy: MemoryPolicy) -> Result<()> { + for (_, region) in self.regions.iter().enumerate() { + if mem_policy.contains(MemoryPolicy::USE_HUGEPAGES) { let ret = region.mapping.use_hugepages(); if let Err(err) = ret { println!("Failed to enable HUGEPAGE for mapping {}", err); } } + + if mem_policy.contains(MemoryPolicy::MLOCK_ON_FAULT) { + region + .mapping + .mlock_on_fault() + .map_err(|e| Error::MemoryLockingFailed(region.mapping.size(), e))?; + } } + + Ok(()) } /// Perform the specified action on each region's addresses.