Validate that PCI MMIO regions are within the expected range.
Bug: 237249346
Test: Built and ran pVM firmware manually
Change-Id: Ic506486c8961f40fa6f08c602813bf17796be8e7
diff --git a/libs/fdtpci/src/lib.rs b/libs/fdtpci/src/lib.rs
index 1ddda9f..e32e16d 100644
--- a/libs/fdtpci/src/lib.rs
+++ b/libs/fdtpci/src/lib.rs
@@ -91,7 +91,7 @@
}
/// Information about the PCI bus parsed from the device tree.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
pub struct PciInfo {
/// The MMIO range used by the memory-mapped PCI CAM.
pub cam_range: Range<usize>,
diff --git a/pvmfw/Android.bp b/pvmfw/Android.bp
index f5e214e..21f84a5 100644
--- a/pvmfw/Android.bp
+++ b/pvmfw/Android.bp
@@ -18,6 +18,7 @@
"libfdtpci",
"liblibfdt",
"liblog_rust_nostd",
+ "libonce_cell_nostd",
"libpvmfw_avb_nostd",
"libpvmfw_embedded_key",
"libtinyvec_nostd",
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index a249e8d..24c36b3 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -45,7 +45,7 @@
helpers::flush,
helpers::GUEST_PAGE_SIZE,
memory::MemoryTracker,
- virtio::pci::{find_virtio_devices, map_mmio},
+ virtio::pci::{self, find_virtio_devices},
};
use ::dice::bcc;
use fdtpci::{PciError, PciInfo};
@@ -76,10 +76,7 @@
// Set up PCI bus for VirtIO devices.
let pci_info = PciInfo::from_fdt(fdt).map_err(handle_pci_error)?;
debug!("PCI: {:#x?}", pci_info);
- map_mmio(&pci_info, memory)?;
- // Safety: This is the only place where we call make_pci_root, and this main function is only
- // called once.
- let mut pci_root = unsafe { pci_info.make_pci_root() };
+ let mut pci_root = pci::initialise(pci_info, memory)?;
find_virtio_devices(&mut pci_root).map_err(handle_pci_error)?;
verify_payload(signed_kernel, ramdisk, PUBLIC_KEY).map_err(|e| {
diff --git a/pvmfw/src/virtio/hal.rs b/pvmfw/src/virtio/hal.rs
index c6c7a99..5f70b33 100644
--- a/pvmfw/src/virtio/hal.rs
+++ b/pvmfw/src/virtio/hal.rs
@@ -1,5 +1,9 @@
+use super::pci::PCI_INFO;
use crate::memory::{alloc_shared, dealloc_shared, phys_to_virt, virt_to_phys};
-use core::ptr::{copy_nonoverlapping, NonNull};
+use core::{
+ ops::Range,
+ ptr::{copy_nonoverlapping, NonNull},
+};
use log::debug;
use virtio_drivers::{BufferDirection, Hal, PhysAddr, PAGE_SIZE};
@@ -26,7 +30,21 @@
0
}
- fn mmio_phys_to_virt(paddr: PhysAddr, _size: usize) -> NonNull<u8> {
+ fn mmio_phys_to_virt(paddr: PhysAddr, size: usize) -> NonNull<u8> {
+ let pci_info = PCI_INFO.get().expect("VirtIO HAL used before PCI_INFO was initialised");
+ // Check that the region is within the PCI MMIO range that we read from the device tree. If
+ // not, the host is probably trying to do something malicious.
+ if !contains_range(
+ &pci_info.bar_range,
+ &(paddr.try_into().expect("PCI MMIO region start was outside of 32-bit address space")
+ ..paddr
+ .checked_add(size)
+ .expect("PCI MMIO region end overflowed")
+ .try_into()
+ .expect("PCI MMIO region end was outside of 32-bit address space")),
+ ) {
+ panic!("PCI MMIO region was outside of expected BAR range.");
+ }
phys_to_virt(paddr)
}
@@ -68,3 +86,8 @@
}
}
}
+
+/// Returns true if `inner` is entirely contained within `outer`.
+fn contains_range(outer: &Range<u32>, inner: &Range<u32>) -> bool {
+ inner.start >= outer.start && inner.end <= outer.end
+}
diff --git a/pvmfw/src/virtio/pci.rs b/pvmfw/src/virtio/pci.rs
index f9d36c6..d3b3124 100644
--- a/pvmfw/src/virtio/pci.rs
+++ b/pvmfw/src/virtio/pci.rs
@@ -16,8 +16,10 @@
use super::hal::HalImpl;
use crate::{entry::RebootReason, memory::MemoryTracker};
+use alloc::boxed::Box;
use fdtpci::{PciError, PciInfo};
use log::{debug, error, info};
+use once_cell::race::OnceBox;
use virtio_drivers::{
device::blk::VirtIOBlk,
transport::{
@@ -26,8 +28,29 @@
},
};
+pub(super) static PCI_INFO: OnceBox<PciInfo> = OnceBox::new();
+
+/// Prepares to use VirtIO PCI devices.
+///
+/// In particular:
+///
+/// 1. Maps the PCI CAM and BAR range in the page table and MMIO guard.
+/// 2. Stores the `PciInfo` for the VirtIO HAL to use later.
+/// 3. Creates and returns a `PciRoot`.
+///
+/// This must only be called once; it will panic if it is called a second time.
+pub fn initialise(pci_info: PciInfo, memory: &mut MemoryTracker) -> Result<PciRoot, RebootReason> {
+ map_mmio(&pci_info, memory)?;
+
+ PCI_INFO.set(Box::new(pci_info.clone())).expect("Tried to set PCI_INFO a second time");
+
+ // Safety: This is the only place where we call make_pci_root, and `PCI_INFO.set` above will
+ // panic if it is called a second time.
+ Ok(unsafe { pci_info.make_pci_root() })
+}
+
/// Maps the CAM and BAR range in the page table and MMIO guard.
-pub fn map_mmio(pci_info: &PciInfo, memory: &mut MemoryTracker) -> Result<(), RebootReason> {
+fn map_mmio(pci_info: &PciInfo, memory: &mut MemoryTracker) -> Result<(), RebootReason> {
memory.map_mmio_range(pci_info.cam_range.clone()).map_err(|e| {
error!("Failed to map PCI CAM: {}", e);
RebootReason::InternalError