Merge "[test] Update test to reflect failure to verify payload in pVM" into main
diff --git a/pvmfw/src/config.rs b/pvmfw/src/config.rs
index 3f78a88..39b7421 100644
--- a/pvmfw/src/config.rs
+++ b/pvmfw/src/config.rs
@@ -84,6 +84,7 @@
const MAGIC: u32 = u32::from_ne_bytes(*b"pvmf");
const VERSION_1_0: Version = Version { major: 1, minor: 0 };
const VERSION_1_1: Version = Version { major: 1, minor: 1 };
+ const VERSION_1_2: Version = Version { major: 1, minor: 2 };
pub fn total_size(&self) -> usize {
self.total_size as usize
@@ -105,8 +106,9 @@
let last_entry = match self.version {
Self::VERSION_1_0 => Entry::DebugPolicy,
Self::VERSION_1_1 => Entry::VmDtbo,
+ Self::VERSION_1_2 => Entry::VmBaseDtbo,
v @ Version { major: 1, .. } => {
- const LATEST: Version = Header::VERSION_1_1;
+ const LATEST: Version = Header::VERSION_1_2;
warn!("Parsing unknown config data version {v} as version {LATEST}");
return Ok(Entry::COUNT);
}
@@ -122,6 +124,7 @@
Bcc,
DebugPolicy,
VmDtbo,
+ VmBaseDtbo,
#[allow(non_camel_case_types)] // TODO: Use mem::variant_count once stable.
_VARIANT_COUNT,
}
@@ -129,7 +132,8 @@
impl Entry {
const COUNT: usize = Self::_VARIANT_COUNT as usize;
- const ALL_ENTRIES: [Entry; Self::COUNT] = [Self::Bcc, Self::DebugPolicy, Self::VmDtbo];
+ const ALL_ENTRIES: [Entry; Self::COUNT] =
+ [Self::Bcc, Self::DebugPolicy, Self::VmDtbo, Self::VmBaseDtbo];
}
#[derive(Default)]
@@ -137,6 +141,7 @@
pub bcc: &'a mut [u8],
pub debug_policy: Option<&'a [u8]>,
pub vm_dtbo: Option<&'a mut [u8]>,
+ pub vm_base_dtbo: Option<&'a mut [u8]>,
}
#[repr(packed)]
@@ -285,13 +290,13 @@
entries[i] = Some(chunk);
}
}
- let [bcc, debug_policy, vm_dtbo] = entries;
+ let [bcc, debug_policy, vm_dtbo, vm_base_dtbo] = entries;
// The platform BCC has always been required.
let bcc = bcc.unwrap();
// We have no reason to mutate so drop the `mut`.
let debug_policy = debug_policy.map(|x| &*x);
- Entries { bcc, debug_policy, vm_dtbo }
+ Entries { bcc, debug_policy, vm_dtbo, vm_base_dtbo }
}
}
diff --git a/pvmfw/src/entry.rs b/pvmfw/src/entry.rs
index 8c4396d..c740d1b 100644
--- a/pvmfw/src/entry.rs
+++ b/pvmfw/src/entry.rs
@@ -88,6 +88,7 @@
kernel: usize,
kernel_size: usize,
vm_dtbo: Option<&mut [u8]>,
+ vm_base_dtbo: Option<&mut [u8]>,
) -> Result<Self, RebootReason> {
let fdt_size = NonZeroUsize::new(crosvm::FDT_MAX_SIZE).unwrap();
// TODO - Only map the FDT as read-only, until we modify it right before jump_to_payload()
@@ -101,7 +102,7 @@
// SAFETY: The tracker validated the range to be in main memory, mapped, and not overlap.
let fdt = unsafe { slice::from_raw_parts_mut(range.start as *mut u8, range.len()) };
- let info = fdt::sanitize_device_tree(fdt, vm_dtbo)?;
+ let info = fdt::sanitize_device_tree(fdt, vm_dtbo, vm_base_dtbo)?;
let fdt = libfdt::Fdt::from_mut_slice(fdt).map_err(|e| {
error!("Failed to load sanitized FDT: {e}");
RebootReason::InvalidFdt
@@ -227,7 +228,13 @@
Some(memory::appended_payload_range()),
));
- let slices = MemorySlices::new(fdt, payload, payload_size, config_entries.vm_dtbo)?;
+ let slices = MemorySlices::new(
+ fdt,
+ payload,
+ payload_size,
+ config_entries.vm_dtbo,
+ config_entries.vm_base_dtbo,
+ )?;
// This wrapper allows main() to be blissfully ignorant of platform details.
let next_bcc = crate::main(
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index b53e452..44e55dd 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -19,6 +19,7 @@
use crate::helpers::GUEST_PAGE_SIZE;
use crate::Box;
use crate::RebootReason;
+use alloc::collections::BTreeMap;
use alloc::ffi::CString;
use alloc::vec::Vec;
use core::cmp::max;
@@ -200,27 +201,47 @@
Ok(())
}
-fn read_vendor_hashtree_descriptor_root_digest_from(fdt: &Fdt) -> libfdt::Result<Option<Vec<u8>>> {
+/// Read candidate properties' names from DT which could be overlaid
+fn parse_vm_base_dtbo(fdt: &Fdt) -> libfdt::Result<BTreeMap<CString, Vec<u8>>> {
+ let mut property_map = BTreeMap::new();
if let Some(avf_node) = fdt.node(cstr!("/avf"))? {
- if let Some(vendor_hashtree_descriptor_root_digest) =
- avf_node.getprop(cstr!("vendor_hashtree_descriptor_root_digest"))?
- {
- return Ok(Some(vendor_hashtree_descriptor_root_digest.to_vec()));
+ for property in avf_node.properties()? {
+ let name = property.name()?;
+ let value = property.value()?;
+ property_map.insert(
+ CString::new(name.to_bytes()).map_err(|_| FdtError::BadValue)?,
+ value.to_vec(),
+ );
}
}
- Ok(None)
+ Ok(property_map)
}
-fn patch_vendor_hashtree_descriptor_root_digest(
- fdt: &mut Fdt,
- vendor_hashtree_descriptor_root_digest: &[u8],
+/// Overlay VM base DTBO into VM DT based on the props_info. Property is overlaid in vm_dt only
+/// when it exists both in vm_base_dtbo and props_info. If the values mismatch, it returns error.
+fn apply_vm_base_dtbo(
+ vm_dt: &mut Fdt,
+ vm_base_dtbo: &Fdt,
+ props_info: &BTreeMap<CString, Vec<u8>>,
) -> libfdt::Result<()> {
- let mut root_node = fdt.root_mut()?;
- let mut avf_node = root_node.add_subnode(cstr!("/avf"))?;
- avf_node.setprop(
- cstr!("vendor_hashtree_descriptor_root_digest"),
- vendor_hashtree_descriptor_root_digest,
- )?;
+ let mut root_vm_dt = vm_dt.root_mut()?;
+ let mut avf_vm_dt = root_vm_dt.add_subnode(cstr!("avf"))?;
+ // TODO(b/318431677): Validate nodes beyond /fragment@0/__overlay__/avf and use apply_overlay.
+ let avf_vm_base_dtbo =
+ vm_base_dtbo.node(cstr!("/fragment@0/__overlay__/avf"))?.ok_or(FdtError::NotFound)?;
+ for (name, value) in props_info.iter() {
+ if let Some(value_in_vm_base_dtbo) = avf_vm_base_dtbo.getprop(name)? {
+ if value != value_in_vm_base_dtbo {
+ error!(
+ "Property mismatches while applying overlay VM base DTBO. \
+ Name:{:?}, Value from host as hex:{:x?}, Value from VM base DTBO as hex:{:x?}",
+ name, value, value_in_vm_base_dtbo
+ );
+ return Err(FdtError::BadValue);
+ }
+ avf_vm_dt.setprop(name, value_in_vm_base_dtbo)?;
+ }
+ }
Ok(())
}
@@ -616,7 +637,7 @@
serial_info: SerialInfo,
pub swiotlb_info: SwiotlbInfo,
device_assignment: Option<DeviceAssignmentInfo>,
- vendor_hashtree_descriptor_root_digest: Option<Vec<u8>>,
+ vm_base_dtbo_props_info: BTreeMap<CString, Vec<u8>>,
}
impl DeviceTreeInfo {
@@ -630,6 +651,7 @@
pub fn sanitize_device_tree(
fdt: &mut [u8],
vm_dtbo: Option<&mut [u8]>,
+ vm_base_dtbo: Option<&mut [u8]>,
) -> Result<DeviceTreeInfo, RebootReason> {
let fdt = Fdt::from_mut_slice(fdt).map_err(|e| {
error!("Failed to load FDT: {e}");
@@ -673,6 +695,18 @@
}
}
+ if let Some(vm_base_dtbo) = vm_base_dtbo {
+ let vm_base_dtbo = Fdt::from_mut_slice(vm_base_dtbo).map_err(|e| {
+ error!("Failed to load VM base DTBO: {e}");
+ RebootReason::InvalidFdt
+ })?;
+
+ apply_vm_base_dtbo(fdt, vm_base_dtbo, &info.vm_base_dtbo_props_info).map_err(|e| {
+ error!("Failed to apply VM base DTBO: {e}");
+ RebootReason::InvalidFdt
+ })?;
+ }
+
patch_device_tree(fdt, &info)?;
// TODO(b/317201360): Ensure no overlapping in <reg> among devices
@@ -746,19 +780,10 @@
None => None,
};
- // TODO(b/285854379) : A temporary solution lives. This is for enabling
- // microdroid vendor partition for non-protected VM as well. When passing
- // DT path containing vendor_hashtree_descriptor_root_digest via fstab, init
- // stage will check if vendor_hashtree_descriptor_root_digest exists in the
- // init stage, regardless the protection. Adding this temporary solution
- // will prevent fatal in init stage for protected VM. However, this data is
- // not trustable without validating root digest of vendor hashtree
- // descriptor comes from ABL.
- let vendor_hashtree_descriptor_root_digest =
- read_vendor_hashtree_descriptor_root_digest_from(fdt).map_err(|e| {
- error!("Failed to read vendor_hashtree_descriptor_root_digest from DT: {e}");
- RebootReason::InvalidFdt
- })?;
+ let vm_base_dtbo_props_info = parse_vm_base_dtbo(fdt).map_err(|e| {
+ error!("Failed to read names of properties under /avf from DT: {e}");
+ RebootReason::InvalidFdt
+ })?;
Ok(DeviceTreeInfo {
kernel_range,
@@ -770,7 +795,7 @@
serial_info,
swiotlb_info,
device_assignment,
- vendor_hashtree_descriptor_root_digest,
+ vm_base_dtbo_props_info,
})
}
@@ -823,15 +848,6 @@
RebootReason::InvalidFdt
})?;
}
- if let Some(vendor_hashtree_descriptor_root_digest) =
- &info.vendor_hashtree_descriptor_root_digest
- {
- patch_vendor_hashtree_descriptor_root_digest(fdt, vendor_hashtree_descriptor_root_digest)
- .map_err(|e| {
- error!("Failed to patch vendor_hashtree_descriptor_root_digest to DT: {e}");
- RebootReason::InvalidFdt
- })?;
- }
Ok(())
}
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index d775555..12b8f88 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -1392,17 +1392,12 @@
}
fn getSecretkeeper(&self) -> binder::Result<Option<Strong<dyn ISecretkeeper>>> {
- let sk = match binder::get_interface(SECRETKEEPER_IDENTIFIER) {
- Ok(sk) => {
- Some(BnSecretkeeper::new_binder(SecretkeeperProxy(sk), BinderFeatures::default()))
- }
- Err(StatusCode::NAME_NOT_FOUND) => None,
- Err(e) => {
- error!("unexpected error while fetching connection to Secretkeeper {:?}", e);
- return Err(e.into());
- }
+ let sk = if is_secretkeeper_present() {
+ Some(binder::wait_for_interface(SECRETKEEPER_IDENTIFIER)?)
+ } else {
+ None
};
- Ok(sk)
+ Ok(sk.map(|s| BnSecretkeeper::new_binder(SecretkeeperProxy(s), BinderFeatures::default())))
}
fn requestAttestation(&self, csr: &[u8]) -> binder::Result<Vec<Certificate>> {
@@ -1410,6 +1405,11 @@
}
}
+fn is_secretkeeper_present() -> bool {
+ binder::is_declared(SECRETKEEPER_IDENTIFIER)
+ .expect("Could not check for declared Secretkeeper interface")
+}
+
impl VirtualMachineService {
fn new_binder(state: Arc<Mutex<State>>, cid: Cid) -> Strong<dyn IVirtualMachineService> {
BnVirtualMachineService::new_binder(