pvmfw: Detect pvmfw overlap before REQUEST_MMIO
As pKVM maps (stage-2) any MMIO region that pvmfw passes in the
REQUEST_MMIO HVC, a malicious host could set up VFIO+KVM to map physical
pages of MMIO in pvmfw's address space. As long as pvmfw hasn't (yet)
touched those pages, they will be mapped by the hyp and then accessed
unintentionally by pvmfw in place of the expected memory pages (e.g. for
.text, .rodata, the appended config data, the stack, the payload, ...).
Bug: 316862665
Test: atest libpvmfw.device_assignment.test
Change-Id: I1dd2c16f3e5e407b14bfbf8b8481b43fbb059d1f
diff --git a/pvmfw/Android.bp b/pvmfw/Android.bp
index 6a6d199..cce0e73 100644
--- a/pvmfw/Android.bp
+++ b/pvmfw/Android.bp
@@ -83,6 +83,7 @@
":test_pvmfw_devices_vm_dtbo",
":test_pvmfw_devices_vm_dtbo_without_symbols",
":test_pvmfw_devices_vm_dtbo_with_duplicated_iommus",
+ ":test_pvmfw_devices_overlapping_pvmfw",
":test_pvmfw_devices_with_rng",
":test_pvmfw_devices_with_multiple_devices_iommus",
":test_pvmfw_devices_with_iommu_sharing",
@@ -142,6 +143,13 @@
}
genrule {
+ name: "test_pvmfw_devices_overlapping_pvmfw",
+ defaults: ["test_device_assignment_dts_to_dtb"],
+ srcs: ["testdata/test_pvmfw_devices_overlapping_pvmfw.dts"],
+ out: ["test_pvmfw_devices_overlapping_pvmfw.dtb"],
+}
+
+genrule {
name: "test_pvmfw_devices_with_rng",
defaults: ["test_device_assignment_dts_to_dtb"],
srcs: ["testdata/test_pvmfw_devices_with_rng.dts"],
diff --git a/pvmfw/src/device_assignment.rs b/pvmfw/src/device_assignment.rs
index edfe000..c3ccf96 100644
--- a/pvmfw/src/device_assignment.rs
+++ b/pvmfw/src/device_assignment.rs
@@ -27,6 +27,7 @@
use core::ffi::CStr;
use core::iter::Iterator;
use core::mem;
+use core::ops::Range;
use hyp::DeviceAssigningHypervisor;
use libfdt::{Fdt, FdtError, FdtNode, Phandle, Reg};
use log::error;
@@ -419,6 +420,12 @@
size: u64,
}
+impl DeviceReg {
+ pub fn overlaps(&self, range: &Range<u64>) -> bool {
+ self.addr < range.end && range.start < self.addr.checked_add(self.size).unwrap()
+ }
+}
+
impl TryFrom<Reg<u64>> for DeviceReg {
type Error = DeviceAssignmentError;
@@ -530,13 +537,19 @@
) -> Result<()> {
let mut virt_regs = device_reg.iter();
let mut phys_regs = physical_device_reg.iter();
+ // TODO(b/308694211): Move this constant to vmbase::layout once vmbase is std-compatible.
+ const PVMFW_RANGE: Range<u64> = 0x7fc0_0000..0x8000_0000;
// PV reg and physical reg should have 1:1 match in order.
for (reg, phys_reg) in virt_regs.by_ref().zip(phys_regs.by_ref()) {
+ if reg.overlaps(&PVMFW_RANGE) {
+ return Err(DeviceAssignmentError::InvalidReg(reg.addr, reg.size));
+ }
+ // If this call returns successfully, hyp has mapped the MMIO region at `reg`.
let addr = hypervisor.get_phys_mmio_token(reg.addr, reg.size).map_err(|e| {
error!("Hypervisor error while requesting MMIO token: {e}");
DeviceAssignmentError::InvalidReg(reg.addr, reg.size)
})?;
- // Only check address because hypervisor guaranatees size match when success.
+ // Only check address because hypervisor guarantees size match when success.
if phys_reg.addr != addr {
error!("Assigned device {reg:x?} has unexpected physical address");
return Err(DeviceAssignmentError::InvalidPhysReg(addr, reg.size));
@@ -857,6 +870,7 @@
const FDT_WITHOUT_IOMMUS_FILE_PATH: &str = "test_pvmfw_devices_without_iommus.dtb";
const FDT_WITHOUT_DEVICE_FILE_PATH: &str = "test_pvmfw_devices_without_device.dtb";
const FDT_FILE_PATH: &str = "test_pvmfw_devices_with_rng.dtb";
+ const FDT_WITH_DEVICE_OVERLAPPING_PVMFW: &str = "test_pvmfw_devices_overlapping_pvmfw.dtb";
const FDT_WITH_MULTIPLE_DEVICES_IOMMUS_FILE_PATH: &str =
"test_pvmfw_devices_with_multiple_devices_iommus.dtb";
const FDT_WITH_IOMMU_SHARING: &str = "test_pvmfw_devices_with_iommu_sharing.dtb";
@@ -1407,6 +1421,22 @@
}
#[test]
+ fn device_info_overlaps_pvmfw() {
+ let mut fdt_data = fs::read(FDT_WITH_DEVICE_OVERLAPPING_PVMFW).unwrap();
+ let mut vm_dtbo_data = fs::read(VM_DTBO_FILE_PATH).unwrap();
+ let fdt = Fdt::from_mut_slice(&mut fdt_data).unwrap();
+ let vm_dtbo = VmDtbo::from_mut_slice(&mut vm_dtbo_data).unwrap();
+
+ let hypervisor = MockHypervisor {
+ mmio_tokens: [((0x7fee0000, 0x1000), 0xF00000)].into(),
+ iommu_tokens: [((0xFF, 0xF), (0x40000, 0x4))].into(),
+ };
+ let device_info = DeviceAssignmentInfo::parse(fdt, vm_dtbo, &hypervisor);
+
+ assert_eq!(device_info, Err(DeviceAssignmentError::InvalidReg(0x7fee0000, 0x1000)));
+ }
+
+ #[test]
fn device_assignment_clean() {
let mut platform_dt_data = pvmfw_fdt_template::RAW.to_vec();
let platform_dt = Fdt::from_mut_slice(&mut platform_dt_data).unwrap();
diff --git a/pvmfw/testdata/test_pvmfw_devices_overlapping_pvmfw.dts b/pvmfw/testdata/test_pvmfw_devices_overlapping_pvmfw.dts
new file mode 100644
index 0000000..2743dd8
--- /dev/null
+++ b/pvmfw/testdata/test_pvmfw_devices_overlapping_pvmfw.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/include/ "test_crosvm_dt_base.dtsi"
+
+/ {
+ light {
+ reg = <0x0 0x7fee0000 0x0 0x1000>;
+ iommus = <&pviommu_0 0xF>;
+ };
+
+ pviommu_0: pviommu0 {
+ compatible = "pkvm,pviommu";
+ id = <0xFF>;
+ #iommu-cells = <1>;
+ };
+};