Merge "pvmfw: Detect and log large number of PCI devices" into main
diff --git a/libs/fdtpci/src/lib.rs b/libs/fdtpci/src/lib.rs
index 96d98d6..602f736 100644
--- a/libs/fdtpci/src/lib.rs
+++ b/libs/fdtpci/src/lib.rs
@@ -119,7 +119,9 @@
/// method must only be called once, and there must be no other `PciRoot` constructed using the
/// same CAM.
pub unsafe fn make_pci_root(&self) -> PciRoot {
- PciRoot::new(self.cam_range.start as *mut u8, Cam::MmioCam)
+ // SAFETY: We trust that the FDT gave us a valid MMIO base address for the CAM. The caller
+ // guarantees to only call us once, so there are no other references to it.
+ unsafe { PciRoot::new(self.cam_range.start as *mut u8, Cam::MmioCam) }
}
}
diff --git a/libs/hyp/src/error.rs b/libs/hyp/src/error.rs
index b8498ca..3fdad70 100644
--- a/libs/hyp/src/error.rs
+++ b/libs/hyp/src/error.rs
@@ -26,7 +26,7 @@
#[derive(Debug, Clone)]
pub enum Error {
/// MMIO guard is not supported.
- MmioGuardNotsupported,
+ MmioGuardNotSupported,
/// Failed to invoke a certain KVM HVC function.
KvmError(KvmError, u32),
/// Failed to invoke GenieZone HVC function.
@@ -40,7 +40,7 @@
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
- Self::MmioGuardNotsupported => write!(f, "MMIO guard is not supported"),
+ Self::MmioGuardNotSupported => write!(f, "MMIO guard is not supported"),
Self::KvmError(e, function_id) => {
write!(f, "Failed to invoke the HVC function with function ID {function_id}: {e}")
}
diff --git a/libs/hyp/src/hypervisor/common.rs b/libs/hyp/src/hypervisor/common.rs
index 7c030a1..70fdd0a 100644
--- a/libs/hyp/src/hypervisor/common.rs
+++ b/libs/hyp/src/hypervisor/common.rs
@@ -14,7 +14,7 @@
//! This module regroups some common traits shared by all the hypervisors.
-use crate::error::Result;
+use crate::error::{Error, Result};
use crate::util::SIZE_4KB;
/// Expected MMIO guard granule size, validated during MMIO guard initialization.
@@ -34,10 +34,9 @@
}
pub trait MmioGuardedHypervisor {
- /// Initializes the hypervisor by enrolling a MMIO guard and checking the memory granule size.
- /// By enrolling, all MMIO will be blocked unless allow-listed with `mmio_guard_map`.
- /// Protected VMs are auto-enrolled.
- fn init(&self) -> Result<()>;
+ /// Enrolls with the MMIO guard so that all MMIO will be blocked unless allow-listed with
+ /// `MmioGuardedHypervisor::map`.
+ fn enroll(&self) -> Result<()>;
/// Maps a page containing the given memory address to the hypervisor MMIO guard.
/// The page size corresponds to the MMIO guard granule size.
@@ -46,6 +45,18 @@
/// Unmaps a page containing the given memory address from the hypervisor MMIO guard.
/// The page size corresponds to the MMIO guard granule size.
fn unmap(&self, addr: usize) -> Result<()>;
+
+ /// Returns the MMIO guard granule size in bytes.
+ fn granule(&self) -> Result<usize>;
+
+ // TODO(ptosi): Fully move granule validation to client code.
+ /// Validates the MMIO guard granule size.
+ fn validate_granule(&self) -> Result<()> {
+ match self.granule()? {
+ MMIO_GUARD_GRANULE_SIZE => Ok(()),
+ granule => Err(Error::UnsupportedMmioGuardGranule(granule)),
+ }
+ }
}
pub trait MemSharingHypervisor {
diff --git a/libs/hyp/src/hypervisor/geniezone.rs b/libs/hyp/src/hypervisor/geniezone.rs
index 24eb89e..ad18e17 100644
--- a/libs/hyp/src/hypervisor/geniezone.rs
+++ b/libs/hyp/src/hypervisor/geniezone.rs
@@ -14,9 +14,7 @@
//! Wrappers around calls to the GenieZone hypervisor.
-use super::common::{
- Hypervisor, MemSharingHypervisor, MmioGuardedHypervisor, MMIO_GUARD_GRANULE_SIZE,
-};
+use super::common::{Hypervisor, MemSharingHypervisor, MmioGuardedHypervisor};
use crate::error::{Error, Result};
use crate::util::page_address;
use core::fmt::{self, Display, Formatter};
@@ -96,13 +94,15 @@
}
impl MmioGuardedHypervisor for GeniezoneHypervisor {
- fn init(&self) -> Result<()> {
- mmio_guard_enroll()?;
- let mmio_granule = mmio_guard_granule()?;
- if mmio_granule != MMIO_GUARD_GRANULE_SIZE {
- return Err(Error::UnsupportedMmioGuardGranule(mmio_granule));
+ fn enroll(&self) -> Result<()> {
+ let args = [0u64; 17];
+ match success_or_error_64(hvc64(VENDOR_HYP_GZVM_MMIO_GUARD_ENROLL_FUNC_ID, args)[0]) {
+ Ok(()) => Ok(()),
+ Err(GeniezoneError::NotSupported) | Err(GeniezoneError::NotRequired) => {
+ Err(Error::MmioGuardNotSupported)
+ }
+ Err(e) => Err(Error::GeniezoneError(e, VENDOR_HYP_GZVM_MMIO_GUARD_ENROLL_FUNC_ID)),
}
- Ok(())
}
fn map(&self, addr: usize) -> Result<()> {
@@ -118,6 +118,12 @@
checked_hvc64_expect_zero(VENDOR_HYP_GZVM_MMIO_GUARD_UNMAP_FUNC_ID, args)
}
+
+ fn granule(&self) -> Result<usize> {
+ let args = [0u64; 17];
+ let granule = checked_hvc64(VENDOR_HYP_GZVM_MMIO_GUARD_INFO_FUNC_ID, args)?;
+ Ok(granule.try_into().unwrap())
+ }
}
impl MemSharingHypervisor for GeniezoneHypervisor {
@@ -142,23 +148,6 @@
}
}
-fn mmio_guard_granule() -> Result<usize> {
- let args = [0u64; 17];
-
- let granule = checked_hvc64(VENDOR_HYP_GZVM_MMIO_GUARD_INFO_FUNC_ID, args)?;
- Ok(granule.try_into().unwrap())
-}
-
-fn mmio_guard_enroll() -> Result<()> {
- let args = [0u64; 17];
- match success_or_error_64(hvc64(VENDOR_HYP_GZVM_MMIO_GUARD_ENROLL_FUNC_ID, args)[0]) {
- Ok(_) => Ok(()),
- Err(GeniezoneError::NotSupported) => Err(Error::MmioGuardNotsupported),
- Err(GeniezoneError::NotRequired) => Err(Error::MmioGuardNotsupported),
- Err(e) => Err(Error::GeniezoneError(e, VENDOR_HYP_GZVM_MMIO_GUARD_ENROLL_FUNC_ID)),
- }
-}
-
fn checked_hvc64_expect_zero(function: u32, args: [u64; 17]) -> Result<()> {
success_or_error_64(hvc64(function, args)[0]).map_err(|e| Error::GeniezoneError(e, function))
}
diff --git a/libs/hyp/src/hypervisor/kvm.rs b/libs/hyp/src/hypervisor/kvm.rs
index a95b8de..5835346 100644
--- a/libs/hyp/src/hypervisor/kvm.rs
+++ b/libs/hyp/src/hypervisor/kvm.rs
@@ -14,9 +14,7 @@
//! Wrappers around calls to the KVM hypervisor.
-use super::common::{
- Hypervisor, MemSharingHypervisor, MmioGuardedHypervisor, MMIO_GUARD_GRANULE_SIZE,
-};
+use super::common::{Hypervisor, MemSharingHypervisor, MmioGuardedHypervisor};
use crate::error::{Error, Result};
use crate::util::page_address;
use core::fmt::{self, Display, Formatter};
@@ -95,13 +93,13 @@
}
impl MmioGuardedHypervisor for ProtectedKvmHypervisor {
- fn init(&self) -> Result<()> {
- mmio_guard_enroll()?;
- let mmio_granule = mmio_guard_granule()?;
- if mmio_granule != MMIO_GUARD_GRANULE_SIZE {
- return Err(Error::UnsupportedMmioGuardGranule(mmio_granule));
+ fn enroll(&self) -> Result<()> {
+ let args = [0u64; 17];
+ match success_or_error_64(hvc64(VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID, args)[0]) {
+ Ok(()) => Ok(()),
+ Err(KvmError::NotSupported) => Err(Error::MmioGuardNotSupported),
+ Err(e) => Err(Error::KvmError(e, VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID)),
}
- Ok(())
}
fn map(&self, addr: usize) -> Result<()> {
@@ -125,6 +123,12 @@
Err(e) => Err(Error::KvmError(e, VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID)),
}
}
+
+ fn granule(&self) -> Result<usize> {
+ let args = [0u64; 17];
+ let granule = checked_hvc64(VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID, args)?;
+ Ok(granule.try_into().unwrap())
+ }
}
impl MemSharingHypervisor for ProtectedKvmHypervisor {
@@ -149,22 +153,6 @@
}
}
-fn mmio_guard_granule() -> Result<usize> {
- let args = [0u64; 17];
-
- let granule = checked_hvc64(VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID, args)?;
- Ok(granule.try_into().unwrap())
-}
-
-fn mmio_guard_enroll() -> Result<()> {
- let args = [0u64; 17];
- match success_or_error_64(hvc64(VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID, args)[0]) {
- Ok(_) => Ok(()),
- Err(KvmError::NotSupported) => Err(Error::MmioGuardNotsupported),
- Err(e) => Err(Error::KvmError(e, VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID)),
- }
-}
-
fn checked_hvc64_expect_zero(function: u32, args: [u64; 17]) -> Result<()> {
success_or_error_64(hvc64(function, args)[0]).map_err(|e| Error::KvmError(e, function))
}
diff --git a/libs/hyp/src/hypervisor/mod.rs b/libs/hyp/src/hypervisor/mod.rs
index bc9e406..309f967 100644
--- a/libs/hyp/src/hypervisor/mod.rs
+++ b/libs/hyp/src/hypervisor/mod.rs
@@ -60,8 +60,10 @@
GeniezoneHypervisor::UUID => Ok(HypervisorBackend::Geniezone),
GunyahHypervisor::UUID => Ok(HypervisorBackend::Gunyah),
RegularKvmHypervisor::UUID => {
- // Protected KVM has the same UUID so differentiate based on MEM_SHARE.
- match ProtectedKvmHypervisor.as_mem_sharer().unwrap().granule() {
+ // Protected KVM has the same UUID as "regular" KVM so issue an HVC that is assumed
+ // to only be supported by pKVM: if it returns SUCCESS, deduce that this is pKVM
+ // and if it returns NOT_SUPPORTED assume that it is "regular" KVM.
+ match ProtectedKvmHypervisor.as_mmio_guard().unwrap().granule() {
Ok(_) => Ok(HypervisorBackend::ProtectedKvm),
Err(Error::KvmError(KvmError::NotSupported, _)) => {
Ok(HypervisorBackend::RegularKvm)
@@ -101,7 +103,7 @@
}
fn detect_hypervisor() -> HypervisorBackend {
- query_vendor_hyp_call_uid().try_into().expect("Unknown hypervisor")
+ query_vendor_hyp_call_uid().try_into().expect("Failed to detect hypervisor")
}
/// Gets the hypervisor singleton.
diff --git a/libs/statslog_virtualization/statslog_wrapper.rs b/libs/statslog_virtualization/statslog_wrapper.rs
index 4d1a0fa..b069d7c 100644
--- a/libs/statslog_virtualization/statslog_wrapper.rs
+++ b/libs/statslog_virtualization/statslog_wrapper.rs
@@ -1,4 +1,5 @@
#![allow(clippy::too_many_arguments)]
+#![allow(clippy::undocumented_unsafe_blocks)]
#![allow(missing_docs)]
#![allow(unused)]
diff --git a/pvmfw/avb/src/descriptor/collection.rs b/pvmfw/avb/src/descriptor/collection.rs
index c6698c0..14c47b1 100644
--- a/pvmfw/avb/src/descriptor/collection.rs
+++ b/pvmfw/avb/src/descriptor/collection.rs
@@ -170,9 +170,9 @@
/// Behavior is undefined if any of the following conditions are violated:
/// * The `descriptor` pointer must be non-null and point to a valid `AvbDescriptor`.
unsafe fn from_descriptor_ptr(descriptor: *const AvbDescriptor) -> utils::Result<Self> {
+ let avb_descriptor =
// SAFETY: It is safe as the raw pointer `descriptor` is non-null and points to
// a valid `AvbDescriptor`.
- let avb_descriptor =
unsafe { get_valid_descriptor(descriptor, avb_descriptor_validate_and_byteswap)? };
let len = usize_checked_add(
size_of::<AvbDescriptor>(),
@@ -189,9 +189,9 @@
Ok(Self::Hash(descriptor))
}
Ok(AvbDescriptorTag::AVB_DESCRIPTOR_TAG_PROPERTY) => {
+ let descriptor =
// SAFETY: It is safe because the caller ensures that `descriptor` is a non-null
// pointer pointing to a valid struct.
- let descriptor =
unsafe { PropertyDescriptor::from_descriptor_ptr(descriptor, data)? };
Ok(Self::Property(descriptor))
}
diff --git a/pvmfw/avb/src/ops.rs b/pvmfw/avb/src/ops.rs
index e7f0ac7..8f7295c 100644
--- a/pvmfw/avb/src/ops.rs
+++ b/pvmfw/avb/src/ops.rs
@@ -320,8 +320,8 @@
pub(crate) fn vbmeta_images(&self) -> Result<&[AvbVBMetaData], AvbSlotVerifyError> {
let data = self.as_ref();
is_not_null(data.vbmeta_images).map_err(|_| AvbSlotVerifyError::Io)?;
- // SAFETY: It is safe as the raw pointer `data.vbmeta_images` is a nonnull pointer.
let vbmeta_images =
+ // SAFETY: It is safe as the raw pointer `data.vbmeta_images` is a nonnull pointer.
unsafe { slice::from_raw_parts(data.vbmeta_images, data.num_vbmeta_images) };
Ok(vbmeta_images)
}
@@ -329,10 +329,10 @@
pub(crate) fn loaded_partitions(&self) -> Result<&[AvbPartitionData], AvbSlotVerifyError> {
let data = self.as_ref();
is_not_null(data.loaded_partitions).map_err(|_| AvbSlotVerifyError::Io)?;
+ let loaded_partitions =
// SAFETY: It is safe as the raw pointer `data.loaded_partitions` is a nonnull pointer and
// is guaranteed by libavb to point to a valid `AvbPartitionData` array as part of the
// `AvbSlotVerifyData` struct.
- let loaded_partitions =
unsafe { slice::from_raw_parts(data.loaded_partitions, data.num_loaded_partitions) };
Ok(loaded_partitions)
}
diff --git a/pvmfw/src/crypto.rs b/pvmfw/src/crypto.rs
index 3d9c8d1..94714c0 100644
--- a/pvmfw/src/crypto.rs
+++ b/pvmfw/src/crypto.rs
@@ -46,17 +46,14 @@
impl Error {
fn get() -> Option<Self> {
- let mut file = MaybeUninit::uninit();
- let mut line = MaybeUninit::uninit();
- // SAFETY - The function writes to the provided pointers, validated below.
- let packed = unsafe { ERR_get_error_line(file.as_mut_ptr(), line.as_mut_ptr()) };
- // SAFETY - Any possible value returned could be considered a valid *const c_char.
- let file = unsafe { file.assume_init() };
- // SAFETY - Any possible value returned could be considered a valid c_int.
- let line = unsafe { line.assume_init() };
+ let mut file = ptr::null();
+ let mut line = 0;
+ // SAFETY: The function writes to the provided pointers, which are valid because they come
+ // from references. It doesn't retain them after it returns.
+ let packed = unsafe { ERR_get_error_line(&mut file, &mut line) };
let packed = packed.try_into().ok()?;
- // SAFETY - Any non-NULL result is expected to point to a global const C string.
+ // SAFETY: Any non-NULL result is expected to point to a global const C string.
let file = unsafe { as_static_cstr(file) };
Some(Self { packed, file, line })
@@ -67,16 +64,16 @@
}
fn library_name(&self) -> Option<&'static CStr> {
- // SAFETY - Call to a pure function.
+ // SAFETY: Call to a pure function.
let name = unsafe { ERR_lib_error_string(self.packed_value()) };
- // SAFETY - Any non-NULL result is expected to point to a global const C string.
+ // SAFETY: Any non-NULL result is expected to point to a global const C string.
unsafe { as_static_cstr(name) }
}
fn reason(&self) -> Option<&'static CStr> {
- // SAFETY - Call to a pure function.
+ // SAFETY: Call to a pure function.
let reason = unsafe { ERR_reason_error_string(self.packed_value()) };
- // SAFETY - Any non-NULL result is expected to point to a global const C string.
+ // SAFETY: Any non-NULL result is expected to point to a global const C string.
unsafe { as_static_cstr(reason) }
}
}
@@ -111,18 +108,18 @@
impl Aead {
pub fn aes_256_gcm_randnonce() -> Option<&'static Self> {
- // SAFETY - Returned pointer is checked below.
+ // SAFETY: Returned pointer is checked below.
let aead = unsafe { EVP_aead_aes_256_gcm_randnonce() };
if aead.is_null() {
None
} else {
- // SAFETY - We assume that the non-NULL value points to a valid and static EVP_AEAD.
+ // SAFETY: We assume that the non-NULL value points to a valid and static EVP_AEAD.
Some(unsafe { &*(aead as *const _) })
}
}
pub fn max_overhead(&self) -> usize {
- // SAFETY - Function should only read from self.
+ // SAFETY: Function should only read from self.
unsafe { EVP_AEAD_max_overhead(self.as_ref() as *const _) }
}
}
@@ -141,7 +138,7 @@
const DEFAULT_TAG_LENGTH: usize = 0;
let engine = ptr::null_mut(); // Use default implementation.
let mut ctx = MaybeUninit::zeroed();
- // SAFETY - Initialize the EVP_AEAD_CTX with const pointers to the AEAD and key.
+ // SAFETY: Initialize the EVP_AEAD_CTX with const pointers to the AEAD and key.
let result = unsafe {
EVP_AEAD_CTX_init(
ctx.as_mut_ptr(),
@@ -154,7 +151,7 @@
};
if result == 1 {
- // SAFETY - We assume that the non-NULL value points to a valid and static EVP_AEAD.
+ // SAFETY: We assume that the non-NULL value points to a valid and static EVP_AEAD.
Ok(Self(unsafe { ctx.assume_init() }))
} else {
Err(ErrorIterator {})
@@ -162,12 +159,12 @@
}
pub fn aead(&self) -> Option<&'static Aead> {
- // SAFETY - The function should only read from self.
+ // SAFETY: The function should only read from self.
let aead = unsafe { EVP_AEAD_CTX_aead(self.as_ref() as *const _) };
if aead.is_null() {
None
} else {
- // SAFETY - We assume that the non-NULL value points to a valid and static EVP_AEAD.
+ // SAFETY: We assume that the non-NULL value points to a valid and static EVP_AEAD.
Some(unsafe { &*(aead as *const _) })
}
}
@@ -178,7 +175,7 @@
let ad = ptr::null_mut();
let ad_len = 0;
let mut out_len = MaybeUninit::uninit();
- // SAFETY - The function should only read from self and write to out (at most the provided
+ // SAFETY: The function should only read from self and write to out (at most the provided
// number of bytes) and out_len while reading from data (at most the provided number of
// bytes), ignoring any NULL input.
let result = unsafe {
@@ -197,7 +194,7 @@
};
if result == 1 {
- // SAFETY - Any value written to out_len could be a valid usize. The value itself is
+ // SAFETY: Any value written to out_len could be a valid usize. The value itself is
// validated as being a proper slice length by panicking in the following indexing
// otherwise.
let out_len = unsafe { out_len.assume_init() };
@@ -213,7 +210,7 @@
let ad = ptr::null_mut();
let ad_len = 0;
let mut out_len = MaybeUninit::uninit();
- // SAFETY - The function should only read from self and write to out (at most the provided
+ // SAFETY: The function should only read from self and write to out (at most the provided
// number of bytes) while reading from data (at most the provided number of bytes),
// ignoring any NULL input.
let result = unsafe {
@@ -232,7 +229,7 @@
};
if result == 1 {
- // SAFETY - Any value written to out_len could be a valid usize. The value itself is
+ // SAFETY: Any value written to out_len could be a valid usize. The value itself is
// validated as being a proper slice length by panicking in the following indexing
// otherwise.
let out_len = unsafe { out_len.assume_init() };
@@ -272,12 +269,12 @@
pub fn hkdf_sh512<const N: usize>(secret: &[u8], salt: &[u8], info: &[u8]) -> Result<[u8; N]> {
let mut key = [0; N];
- // SAFETY - The function shouldn't access any Rust variable and the returned value is accepted
+ // SAFETY: The function shouldn't access any Rust variable and the returned value is accepted
// as a potentially NULL pointer.
let digest = unsafe { EVP_sha512() };
assert!(!digest.is_null());
- // SAFETY - Only reads from/writes to the provided slices and supports digest was checked not
+ // SAFETY: Only reads from/writes to the provided slices and supports digest was checked not
// be NULL.
let result = unsafe {
HKDF(
@@ -301,6 +298,6 @@
}
pub fn init() {
- // SAFETY - Configures the internal state of the library - may be called multiple times.
+ // SAFETY: Configures the internal state of the library - may be called multiple times.
unsafe { CRYPTO_library_init() }
}
diff --git a/pvmfw/src/dice.rs b/pvmfw/src/dice.rs
index fbab013..28271d3 100644
--- a/pvmfw/src/dice.rs
+++ b/pvmfw/src/dice.rs
@@ -91,7 +91,7 @@
/// .data, or provided BCC).
#[no_mangle]
unsafe extern "C" fn DiceClearMemory(_ctx: *mut c_void, size: usize, addr: *mut c_void) {
- // SAFETY - We must trust that the slice will be valid arrays/variables on the C code stack.
+ // SAFETY: We must trust that the slice will be valid arrays/variables on the C code stack.
let region = unsafe { slice::from_raw_parts_mut(addr as *mut u8, size) };
flushed_zeroize(region)
}
diff --git a/pvmfw/src/entry.rs b/pvmfw/src/entry.rs
index f3bd637..9c929a9 100644
--- a/pvmfw/src/entry.rs
+++ b/pvmfw/src/entry.rs
@@ -93,7 +93,7 @@
RebootReason::InternalError
})?;
- // SAFETY - The tracker validated the range to be in main memory, mapped, and not overlap.
+ // 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 fdt = libfdt::Fdt::from_mut_slice(fdt).map_err(|e| {
error!("Failed to spawn the FDT wrapper: {e}");
@@ -153,9 +153,9 @@
return Err(RebootReason::InvalidPayload);
};
- // SAFETY - The tracker validated the range to be in main memory, mapped, and not overlap.
- let kernel =
- unsafe { slice::from_raw_parts(kernel_range.start as *const u8, kernel_range.len()) };
+ let kernel = kernel_range.start as *const u8;
+ // SAFETY: The tracker validated the range to be in main memory, mapped, and not overlap.
+ let kernel = unsafe { slice::from_raw_parts(kernel, kernel_range.len()) };
let ramdisk = if let Some(r) = info.initrd_range {
debug!("Located ramdisk at {r:?}");
@@ -164,7 +164,7 @@
RebootReason::InvalidRamdisk
})?;
- // SAFETY - The region was validated by memory to be in main memory, mapped, and
+ // SAFETY: The region was validated by memory to be in main memory, mapped, and
// not overlap.
Some(unsafe { slice::from_raw_parts(r.start as *const u8, r.len()) })
} else {
@@ -198,7 +198,7 @@
RebootReason::InternalError
})?;
- // SAFETY - We only get the appended payload from here, once. The region was statically mapped,
+ // SAFETY: We only get the appended payload from here, once. The region was statically mapped,
// then remapped by `init_page_table()`.
let appended_data = unsafe { get_appended_data_slice() };
@@ -277,7 +277,7 @@
// Disable the exception vector, caches and page table and then jump to the payload at the
// given address, passing it the given FDT pointer.
//
- // SAFETY - We're exiting pvmfw by passing the register values we need to a noreturn asm!().
+ // SAFETY: We're exiting pvmfw by passing the register values we need to a noreturn asm!().
unsafe {
asm!(
"cmp {scratch}, {bcc}",
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index 38db7aa..b3d0402 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -567,7 +567,7 @@
*v = v.to_be();
}
- // SAFETY - array size is the same
+ // SAFETY: array size is the same
let value = unsafe {
core::mem::transmute::<
[u32; NUM_INTERRUPTS * CELLS_PER_INTERRUPT],
@@ -809,7 +809,7 @@
}
};
- // SAFETY - on failure, the corrupted DT is restored using the backup.
+ // SAFETY: on failure, the corrupted DT is restored using the backup.
if let Err(e) = unsafe { fdt.apply_overlay(overlay) } {
warn!("Failed to apply debug policy: {e}. Recovering...");
fdt.copy_from_slice(backup_fdt.as_slice())?;
diff --git a/pvmfw/src/gpt.rs b/pvmfw/src/gpt.rs
index 892850c..1060460 100644
--- a/pvmfw/src/gpt.rs
+++ b/pvmfw/src/gpt.rs
@@ -130,7 +130,7 @@
for i in Header::ENTRIES_LBA..Header::ENTRIES_LBA.checked_add(num_blocks).unwrap() {
self.read_block(i, &mut blk)?;
let entries = blk.as_ptr().cast::<Entry>();
- // SAFETY - blk is assumed to be properly aligned for Entry and its size is assert-ed
+ // SAFETY: blk is assumed to be properly aligned for Entry and its size is assert-ed
// above. All potential values of the slice will produce valid Entry values.
let entries = unsafe { slice::from_raw_parts(entries, min(rem, entries_per_blk)) };
for entry in entries {
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index d0a8e85..b2497b1 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -1100,8 +1100,9 @@
Status::new_service_specific_error_str(-1, Some(format!("Failed to create pipe: {:?}", e)))
})?;
- // SAFETY: We are the sole owners of these fds as they were just created.
+ // SAFETY: We are the sole owner of this FD as we just created it, and it is valid and open.
let mut reader = BufReader::new(unsafe { File::from_raw_fd(raw_read_fd) });
+ // SAFETY: We are the sole owner of this FD as we just created it, and it is valid and open.
let write_fd = unsafe { File::from_raw_fd(raw_write_fd) };
std::thread::spawn(move || loop {
diff --git a/virtualizationmanager/src/atom.rs b/virtualizationmanager/src/atom.rs
index d6eb141..1d2d191 100644
--- a/virtualizationmanager/src/atom.rs
+++ b/virtualizationmanager/src/atom.rs
@@ -83,7 +83,7 @@
// This matches how crosvm determines the number of logical cores.
// For telemetry purposes only.
pub(crate) fn get_num_cpus() -> Option<usize> {
- // SAFETY - Only integer constants passed back and forth.
+ // SAFETY: Only integer constants passed back and forth.
let ret = unsafe { libc::sysconf(libc::_SC_NPROCESSORS_CONF) };
if ret > 0 {
ret.try_into().ok()
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 8c412f6..31db3f6 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -592,7 +592,7 @@
}
let guest_time_ticks = data_list[42].parse::<i64>()?;
- // SAFETY : It just returns an integer about CPU tick information.
+ // SAFETY: It just returns an integer about CPU tick information.
let ticks_per_sec = unsafe { sysconf(_SC_CLK_TCK) };
Ok(guest_time_ticks * MILLIS_PER_SEC / ticks_per_sec)
}
@@ -910,8 +910,9 @@
/// Creates a new pipe with the `O_CLOEXEC` flag set, and returns the read side and write side.
fn create_pipe() -> Result<(File, File), Error> {
let (raw_read, raw_write) = pipe2(OFlag::O_CLOEXEC)?;
- // SAFETY: We are the sole owners of these fds as they were just created.
+ // SAFETY: We are the sole owner of this FD as we just created it, and it is valid and open.
let read_fd = unsafe { File::from_raw_fd(raw_read) };
+ // SAFETY: We are the sole owner of this FD as we just created it, and it is valid and open.
let write_fd = unsafe { File::from_raw_fd(raw_write) };
Ok((read_fd, write_fd))
}
diff --git a/virtualizationmanager/src/debug_config.rs b/virtualizationmanager/src/debug_config.rs
index 7172e7d..9b13475 100644
--- a/virtualizationmanager/src/debug_config.rs
+++ b/virtualizationmanager/src/debug_config.rs
@@ -42,7 +42,7 @@
}
fn to_path(&self) -> PathBuf {
- // SAFETY -- unwrap() is safe for to_str() because node_path and prop_name were &str.
+ // unwrap() is safe for to_str() because node_path and prop_name were &str.
PathBuf::from(
[
"/sys/firmware/devicetree/base",
@@ -129,7 +129,7 @@
.map_err(Error::msg)
.with_context(|| "Malformed {overlay_file_path:?}")?;
- // SAFETY - Return immediately if error happens. Damaged fdt_buf and fdt are discarded.
+ // SAFETY: Return immediately if error happens. Damaged fdt_buf and fdt are discarded.
unsafe {
fdt.apply_overlay(overlay_fdt).map_err(Error::msg).with_context(|| {
"Failed to overlay {overlay_file_path:?} onto empty device tree"
@@ -141,7 +141,7 @@
}
fn as_fdt(&self) -> &Fdt {
- // SAFETY - Checked validity of buffer when instantiate.
+ // SAFETY: Checked validity of buffer when instantiate.
unsafe { Fdt::unchecked_from_slice(&self.buffer) }
}
}
diff --git a/virtualizationmanager/src/main.rs b/virtualizationmanager/src/main.rs
index bd7f8af..f058547 100644
--- a/virtualizationmanager/src/main.rs
+++ b/virtualizationmanager/src/main.rs
@@ -86,7 +86,7 @@
}
owned_fds.push(raw_fd);
- // SAFETY - Initializing OwnedFd for a RawFd provided in cmdline arguments.
+ // SAFETY: Initializing OwnedFd for a RawFd provided in cmdline arguments.
// We checked that the integer value corresponds to a valid FD and that this
// is the first argument to claim its ownership.
Ok(unsafe { OwnedFd::from_raw_fd(raw_fd) })
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 5c5a7e4..7dfabb0 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -95,7 +95,7 @@
let pid = get_calling_pid();
let lim = libc::rlimit { rlim_cur: libc::RLIM_INFINITY, rlim_max: libc::RLIM_INFINITY };
- // SAFETY - borrowing the new limit struct only
+ // SAFETY: borrowing the new limit struct only
let ret = unsafe { libc::prlimit(pid, libc::RLIMIT_MEMLOCK, &lim, std::ptr::null_mut()) };
match ret {
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 64da2d9..f50bd50 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -382,14 +382,14 @@
/// Safely duplicate the file descriptor.
fn duplicate_fd<T: AsRawFd>(file: T) -> io::Result<File> {
let fd = file.as_raw_fd();
- // Safe because this just duplicates a file descriptor which we know to be valid, and we check
- // for an error.
+ // SAFETY: This just duplicates a file descriptor which we know to be valid, and we check for an
+ // an error.
let dup_fd = unsafe { libc::dup(fd) };
if dup_fd < 0 {
Err(io::Error::last_os_error())
} else {
- // Safe because we have just duplicated the file descriptor so we own it, and `from_raw_fd`
- // takes ownership of it.
+ // SAFETY: We have just duplicated the file descriptor so we own it, and `from_raw_fd` takes
+ // ownership of it.
Ok(unsafe { File::from_raw_fd(dup_fd) })
}
}
diff --git a/vmbase/Android.bp b/vmbase/Android.bp
index 46f4937..71b9e76 100644
--- a/vmbase/Android.bp
+++ b/vmbase/Android.bp
@@ -84,6 +84,7 @@
"libspin_nostd",
"libtinyvec_nostd",
"libvirtio_drivers",
+ "libzerocopy_nostd",
"libzeroize_nostd",
],
whole_static_libs: [
diff --git a/vmbase/src/entry.rs b/vmbase/src/entry.rs
index 24b5035..2ff66cc 100644
--- a/vmbase/src/entry.rs
+++ b/vmbase/src/entry.rs
@@ -26,7 +26,8 @@
console::init();
if let Some(mmio_guard) = get_mmio_guard() {
- mmio_guard.init()?;
+ mmio_guard.enroll()?;
+ mmio_guard.validate_granule()?;
mmio_guard.map(console::BASE_ADDRESS)?;
}
diff --git a/vmbase/src/hvc.rs b/vmbase/src/hvc.rs
index ebd1625..1197143 100644
--- a/vmbase/src/hvc.rs
+++ b/vmbase/src/hvc.rs
@@ -37,7 +37,7 @@
(version as u32 as i32).try_into()
}
-pub type TrngRng64Entropy = (u64, u64, u64);
+pub type TrngRng64Entropy = [u64; 3];
pub fn trng_rnd64(nbits: u64) -> trng::Result<TrngRng64Entropy> {
let mut args = [0u64; 17];
@@ -46,7 +46,7 @@
let regs = hvc64(ARM_SMCCC_TRNG_RND64, args);
success_or_error_64::<Error>(regs[0])?;
- Ok((regs[1], regs[2], regs[3]))
+ Ok([regs[1], regs[2], regs[3]])
}
pub fn trng_features(fid: u32) -> trng::Result<u64> {
diff --git a/vmbase/src/rand.rs b/vmbase/src/rand.rs
index 6b8d7e0..2acc390 100644
--- a/vmbase/src/rand.rs
+++ b/vmbase/src/rand.rs
@@ -14,10 +14,13 @@
//! Functions and drivers for obtaining true entropy.
-use crate::hvc::{self, TrngRng64Entropy};
+use crate::hvc;
use core::fmt;
use core::mem::size_of;
use smccc::{self, Hvc};
+use zerocopy::AsBytes as _;
+
+type Entropy = [u8; size_of::<u64>() * 3];
/// Error type for rand operations.
pub enum Error {
@@ -95,46 +98,55 @@
/// Fills a slice of bytes with true entropy.
pub fn fill_with_entropy(s: &mut [u8]) -> Result<()> {
- const MAX_BYTES_PER_CALL: usize = size_of::<TrngRng64Entropy>();
+ const MAX_BYTES_PER_CALL: usize = size_of::<Entropy>();
- let (aligned, remainder) = s.split_at_mut(s.len() - s.len() % MAX_BYTES_PER_CALL);
-
- for chunk in aligned.chunks_exact_mut(MAX_BYTES_PER_CALL) {
- let (r, s, t) = repeat_trng_rnd(chunk.len())?;
-
- let mut words = chunk.chunks_exact_mut(size_of::<u64>());
- words.next().unwrap().clone_from_slice(&t.to_ne_bytes());
- words.next().unwrap().clone_from_slice(&s.to_ne_bytes());
- words.next().unwrap().clone_from_slice(&r.to_ne_bytes());
- }
-
- if !remainder.is_empty() {
- let mut entropy = [0; MAX_BYTES_PER_CALL];
- let (r, s, t) = repeat_trng_rnd(remainder.len())?;
-
- let mut words = entropy.chunks_exact_mut(size_of::<u64>());
- words.next().unwrap().clone_from_slice(&t.to_ne_bytes());
- words.next().unwrap().clone_from_slice(&s.to_ne_bytes());
- words.next().unwrap().clone_from_slice(&r.to_ne_bytes());
-
- remainder.clone_from_slice(&entropy[..remainder.len()]);
+ for chunk in s.chunks_mut(MAX_BYTES_PER_CALL) {
+ let entropy = repeat_trng_rnd(chunk.len())?;
+ chunk.clone_from_slice(&entropy[..chunk.len()]);
}
Ok(())
}
-fn repeat_trng_rnd(n_bytes: usize) -> Result<TrngRng64Entropy> {
- let bits = usize::try_from(u8::BITS).unwrap();
- let n_bits = (n_bytes * bits).try_into().unwrap();
+/// Returns an array where the first `n_bytes` bytes hold entropy.
+///
+/// The rest of the array should be ignored.
+fn repeat_trng_rnd(n_bytes: usize) -> Result<Entropy> {
loop {
- match hvc::trng_rnd64(n_bits) {
- Ok(entropy) => return Ok(entropy),
- Err(hvc::trng::Error::NoEntropy) => (),
- Err(e) => return Err(e.into()),
+ if let Some(entropy) = rnd64(n_bytes)? {
+ return Ok(entropy);
}
}
}
+/// Returns an array where the first `n_bytes` bytes hold entropy, if available.
+///
+/// The rest of the array should be ignored.
+fn rnd64(n_bytes: usize) -> Result<Option<Entropy>> {
+ let bits = usize::try_from(u8::BITS).unwrap();
+ let result = hvc::trng_rnd64((n_bytes * bits).try_into().unwrap());
+ let entropy = if matches!(result, Err(hvc::trng::Error::NoEntropy)) {
+ None
+ } else {
+ let r = result?;
+ // From the SMCCC TRNG:
+ //
+ // A MAX_BITS-bits wide value (Entropy) is returned across X1 to X3.
+ // The requested conditioned entropy is returned in Entropy[N-1:0].
+ //
+ // X1 Entropy[191:128]
+ // X2 Entropy[127:64]
+ // X3 Entropy[63:0]
+ //
+ // The bits in Entropy[MAX_BITS-1:N] are 0.
+ let reordered = [r[2].to_le(), r[1].to_le(), r[0].to_le()];
+
+ Some(reordered.as_bytes().try_into().unwrap())
+ };
+
+ Ok(entropy)
+}
+
/// Generate an array of fixed-size initialized with true-random bytes.
pub fn random_array<const N: usize>() -> Result<[u8; N]> {
let mut arr = [0; N];
diff --git a/vmclient/src/lib.rs b/vmclient/src/lib.rs
index cfd015a..7c0383b 100644
--- a/vmclient/src/lib.rs
+++ b/vmclient/src/lib.rs
@@ -67,7 +67,7 @@
// file descriptors (expected by SharedChild).
let (raw1, raw2) = pipe2(OFlag::O_CLOEXEC)?;
- // SAFETY - Taking ownership of brand new FDs.
+ // SAFETY: Taking ownership of brand new FDs.
unsafe { Ok((OwnedFd::from_raw_fd(raw1), OwnedFd::from_raw_fd(raw2))) }
}
@@ -80,7 +80,7 @@
let (raw1, raw2) =
socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::SOCK_CLOEXEC)?;
- // SAFETY - Taking ownership of brand new FDs.
+ // SAFETY: Taking ownership of brand new FDs.
unsafe { Ok((OwnedFd::from_raw_fd(raw1), OwnedFd::from_raw_fd(raw2))) }
}