Merge "Revoke USE_CUSTOM_VIRTUAL_MACHINE permission in @Before setup()"
diff --git a/authfs/Android.bp b/authfs/Android.bp
index 2532026..154a1d6 100644
--- a/authfs/Android.bp
+++ b/authfs/Android.bp
@@ -23,7 +23,7 @@
"liblog_rust",
"libnix",
"libopenssl",
- "libprotobuf_deprecated",
+ "libprotobuf",
"librpcbinder_rs",
"libthiserror",
],
diff --git a/compos/Android.bp b/compos/Android.bp
index c120b0f..2f6be98 100644
--- a/compos/Android.bp
+++ b/compos/Android.bp
@@ -18,7 +18,7 @@
"libminijail_rust",
"libnix",
"libodsign_proto_rust",
- "libprotobuf_deprecated",
+ "libprotobuf",
"libregex",
"librpcbinder_rs",
"librustutils",
diff --git a/compos/composd/Android.bp b/compos/composd/Android.bp
index f66de32..b0294dd 100644
--- a/compos/composd/Android.bp
+++ b/compos/composd/Android.bp
@@ -22,7 +22,7 @@
"liblibc",
"liblog_rust",
"libodsign_proto_rust",
- "libprotobuf_deprecated",
+ "libprotobuf",
"librustutils",
"libshared_child",
"libvmclient",
diff --git a/compos/src/artifact_signer.rs b/compos/src/artifact_signer.rs
index d3843fc..908e438 100644
--- a/compos/src/artifact_signer.rs
+++ b/compos/src/artifact_signer.rs
@@ -63,7 +63,7 @@
/// with accompanying sigature file.
pub fn write_info_and_signature(self, info_path: &Path) -> Result<()> {
let mut info = OdsignInfo::new();
- info.mut_file_hashes().extend(self.file_digests.into_iter());
+ info.file_hashes.extend(self.file_digests.into_iter());
let bytes = info.write_to_bytes()?;
let signature = compos_key::sign(&bytes)?;
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 8e0bb65..afc36d0 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -16,6 +16,8 @@
//! to a bare-metal environment.
#![no_std]
+#![deny(unsafe_op_in_unsafe_fn)]
+#![deny(clippy::undocumented_unsafe_blocks)]
mod iterators;
@@ -205,7 +207,7 @@
}
/// Find parent node.
pub fn parent(&self) -> Result<Self> {
- // SAFETY - Accesses (read-only) are constrained to the DT totalsize.
+ // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_parent_offset(self.fdt.as_ptr(), self.offset) };
Ok(Self { fdt: self.fdt, offset: fdt_err(ret)? })
@@ -311,7 +313,7 @@
name: &CStr,
) -> Result<Option<(*const c_void, usize)>> {
let mut len: i32 = 0;
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor) and the
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) and the
// function respects the passed number of characters.
let prop = unsafe {
libfdt_bindgen::fdt_getprop_namelen(
@@ -342,7 +344,7 @@
}
fn next_compatible(self, compatible: &CStr) -> Result<Option<Self>> {
- // SAFETY - Accesses (read-only) are constrained to the DT totalsize.
+ // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe {
libfdt_bindgen::fdt_node_offset_by_compatible(
self.fdt.as_ptr(),
@@ -355,14 +357,14 @@
}
fn address_cells(&self) -> Result<AddrCells> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor).
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
unsafe { libfdt_bindgen::fdt_address_cells(self.fdt.as_ptr(), self.offset) }
.try_into()
.map_err(|_| FdtError::Internal)
}
fn size_cells(&self) -> Result<SizeCells> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor).
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
unsafe { libfdt_bindgen::fdt_size_cells(self.fdt.as_ptr(), self.offset) }
.try_into()
.map_err(|_| FdtError::Internal)
@@ -378,7 +380,7 @@
impl<'a> FdtNodeMut<'a> {
/// Append a property name-value (possibly empty) pair to the given node.
pub fn appendprop<T: AsRef<[u8]>>(&mut self, name: &CStr, value: &T) -> Result<()> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor).
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe {
libfdt_bindgen::fdt_appendprop(
self.fdt.as_mut_ptr(),
@@ -394,7 +396,7 @@
/// Append a (address, size) pair property to the given node.
pub fn appendprop_addrrange(&mut self, name: &CStr, addr: u64, size: u64) -> Result<()> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor).
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe {
libfdt_bindgen::fdt_appendprop_addrrange(
self.fdt.as_mut_ptr(),
@@ -411,7 +413,7 @@
/// Create or change a property name-value pair to the given node.
pub fn setprop(&mut self, name: &CStr, value: &[u8]) -> Result<()> {
- // SAFETY - New value size is constrained to the DT totalsize
+ // SAFETY: New value size is constrained to the DT totalsize
// (validated by underlying libfdt).
let ret = unsafe {
libfdt_bindgen::fdt_setprop(
@@ -429,7 +431,7 @@
/// Replace the value of the given property with the given value, and ensure that the given
/// value has the same length as the current value length
pub fn setprop_inplace(&mut self, name: &CStr, value: &[u8]) -> Result<()> {
- // SAFETY - fdt size is not altered
+ // SAFETY: fdt size is not altered
let ret = unsafe {
libfdt_bindgen::fdt_setprop_inplace(
self.fdt.as_mut_ptr(),
@@ -457,7 +459,7 @@
/// Delete the given property.
pub fn delprop(&mut self, name: &CStr) -> Result<()> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor) when the
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) when the
// library locates the node's property. Removing the property may shift the offsets of
// other nodes and properties but the borrow checker should prevent this function from
// being called when FdtNode instances are in use.
@@ -470,7 +472,7 @@
/// Overwrite the given property with FDT_NOP, effectively removing it from the DT.
pub fn nop_property(&mut self, name: &CStr) -> Result<()> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor) when the
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) when the
// library locates the node's property.
let ret = unsafe {
libfdt_bindgen::fdt_nop_property(self.fdt.as_mut_ptr(), self.offset, name.as_ptr())
@@ -490,7 +492,7 @@
return Err(FdtError::NoSpace);
}
- // SAFETY - new_size is smaller than the old size
+ // SAFETY: new_size is smaller than the old size
let ret = unsafe {
libfdt_bindgen::fdt_setprop(
self.fdt.as_mut_ptr(),
@@ -511,7 +513,7 @@
/// Add a new subnode to the given node and return it as a FdtNodeMut on success.
pub fn add_subnode(&'a mut self, name: &CStr) -> Result<Self> {
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor).
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor).
let ret = unsafe {
libfdt_bindgen::fdt_add_subnode(self.fdt.as_mut_ptr(), self.offset, name.as_ptr())
};
@@ -520,7 +522,7 @@
}
fn parent(&'a self) -> Result<FdtNode<'a>> {
- // SAFETY - Accesses (read-only) are constrained to the DT totalsize.
+ // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe { libfdt_bindgen::fdt_parent_offset(self.fdt.as_ptr(), self.offset) };
Ok(FdtNode { fdt: &*self.fdt, offset: fdt_err(ret)? })
@@ -528,7 +530,7 @@
/// Return the compatible node of the given name that is next to this node
pub fn next_compatible(self, compatible: &CStr) -> Result<Option<Self>> {
- // SAFETY - Accesses (read-only) are constrained to the DT totalsize.
+ // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe {
libfdt_bindgen::fdt_node_offset_by_compatible(
self.fdt.as_ptr(),
@@ -553,7 +555,7 @@
// mutable reference to DT, so we can't use current node (which also has a mutable reference to
// DT).
pub fn delete_and_next_compatible(self, compatible: &CStr) -> Result<Option<Self>> {
- // SAFETY - Accesses (read-only) are constrained to the DT totalsize.
+ // SAFETY: Accesses (read-only) are constrained to the DT totalsize.
let ret = unsafe {
libfdt_bindgen::fdt_node_offset_by_compatible(
self.fdt.as_ptr(),
@@ -563,7 +565,7 @@
};
let next_offset = fdt_err_or_option(ret)?;
- // SAFETY - fdt_nop_node alter only the bytes in the blob which contain the node and its
+ // SAFETY: fdt_nop_node alter only the bytes in the blob which contain the node and its
// properties and subnodes, and will not alter or move any other part of the tree.
let ret = unsafe { libfdt_bindgen::fdt_nop_node(self.fdt.as_mut_ptr(), self.offset) };
fdt_err_expect_zero(ret)?;
@@ -611,7 +613,7 @@
///
/// Fails if the FDT does not pass validation.
pub fn from_slice(fdt: &[u8]) -> Result<&Self> {
- // SAFETY - The FDT will be validated before it is returned.
+ // SAFETY: The FDT will be validated before it is returned.
let fdt = unsafe { Self::unchecked_from_slice(fdt) };
fdt.check_full()?;
Ok(fdt)
@@ -621,7 +623,7 @@
///
/// Fails if the FDT does not pass validation.
pub fn from_mut_slice(fdt: &mut [u8]) -> Result<&mut Self> {
- // SAFETY - The FDT will be validated before it is returned.
+ // SAFETY: The FDT will be validated before it is returned.
let fdt = unsafe { Self::unchecked_from_mut_slice(fdt) };
fdt.check_full()?;
Ok(fdt)
@@ -629,7 +631,7 @@
/// Creates an empty Flattened Device Tree with a mutable slice.
pub fn create_empty_tree(fdt: &mut [u8]) -> Result<&mut Self> {
- // SAFETY - fdt_create_empty_tree() only write within the specified length,
+ // SAFETY: fdt_create_empty_tree() only write within the specified length,
// and returns error if buffer was insufficient.
// There will be no memory write outside of the given fdt.
let ret = unsafe {
@@ -640,7 +642,7 @@
};
fdt_err_expect_zero(ret)?;
- // SAFETY - The FDT will be validated before it is returned.
+ // SAFETY: The FDT will be validated before it is returned.
let fdt = unsafe { Self::unchecked_from_mut_slice(fdt) };
fdt.check_full()?;
@@ -653,7 +655,9 @@
///
/// The returned FDT might be invalid, only use on slices containing a valid DT.
pub unsafe fn unchecked_from_slice(fdt: &[u8]) -> &Self {
- mem::transmute::<&[u8], &Self>(fdt)
+ // SAFETY: Fdt is a wrapper around a [u8], so the transmute is valid. The caller is
+ // responsible for ensuring that it is actually a valid FDT.
+ unsafe { mem::transmute::<&[u8], &Self>(fdt) }
}
/// Wraps a mutable slice containing a Flattened Device Tree.
@@ -662,7 +666,9 @@
///
/// The returned FDT might be invalid, only use on slices containing a valid DT.
pub unsafe fn unchecked_from_mut_slice(fdt: &mut [u8]) -> &mut Self {
- mem::transmute::<&mut [u8], &mut Self>(fdt)
+ // SAFETY: Fdt is a wrapper around a [u8], so the transmute is valid. The caller is
+ // responsible for ensuring that it is actually a valid FDT.
+ unsafe { mem::transmute::<&mut [u8], &mut Self>(fdt) }
}
/// Update this FDT from a slice containing another FDT
@@ -682,7 +688,7 @@
/// Make the whole slice containing the DT available to libfdt.
pub fn unpack(&mut self) -> Result<()> {
- // SAFETY - "Opens" the DT in-place (supported use-case) by updating its header and
+ // SAFETY: "Opens" the DT in-place (supported use-case) by updating its header and
// internal structures to make use of the whole self.fdt slice but performs no accesses
// outside of it and leaves the DT in a state that will be detected by other functions.
let ret = unsafe {
@@ -699,7 +705,7 @@
///
/// Doesn't shrink the underlying memory slice.
pub fn pack(&mut self) -> Result<()> {
- // SAFETY - "Closes" the DT in-place by updating its header and relocating its structs.
+ // SAFETY: "Closes" the DT in-place by updating its header and relocating its structs.
let ret = unsafe { libfdt_bindgen::fdt_pack(self.as_mut_ptr()) };
fdt_err_expect_zero(ret)
}
@@ -710,10 +716,12 @@
///
/// On failure, the library corrupts the DT and overlay so both must be discarded.
pub unsafe fn apply_overlay<'a>(&'a mut self, overlay: &'a mut Fdt) -> Result<&'a mut Self> {
- fdt_err_expect_zero(libfdt_bindgen::fdt_overlay_apply(
- self.as_mut_ptr(),
- overlay.as_mut_ptr(),
- ))?;
+ let ret =
+ // SAFETY: Both pointers are valid because they come from references, and fdt_overlay_apply
+ // doesn't keep them after it returns. It may corrupt their contents if there is an error,
+ // but that's our caller's responsibility.
+ unsafe { libfdt_bindgen::fdt_overlay_apply(self.as_mut_ptr(), overlay.as_mut_ptr()) };
+ fdt_err_expect_zero(ret)?;
Ok(self)
}
@@ -779,7 +787,7 @@
fn path_offset(&self, path: &CStr) -> Result<Option<c_int>> {
let len = path.to_bytes().len().try_into().map_err(|_| FdtError::BadPath)?;
- // SAFETY - Accesses are constrained to the DT totalsize (validated by ctor) and the
+ // SAFETY: Accesses are constrained to the DT totalsize (validated by ctor) and the
// function respects the passed number of characters.
let ret = unsafe {
// *_namelen functions don't include the trailing nul terminator in 'len'.
@@ -791,7 +799,7 @@
fn check_full(&self) -> Result<()> {
let len = self.buffer.len();
- // SAFETY - Only performs read accesses within the limits of the slice. If successful, this
+ // SAFETY: Only performs read accesses within the limits of the slice. If successful, this
// call guarantees to other unsafe calls that the header contains a valid totalsize (w.r.t.
// 'len' i.e. the self.fdt slice) that those C functions can use to perform bounds
// checking. The library doesn't maintain an internal state (such as pointers) between
@@ -815,7 +823,7 @@
fn header(&self) -> &libfdt_bindgen::fdt_header {
let p = self.as_ptr().cast::<_>();
- // SAFETY - A valid FDT (verified by constructor) must contain a valid fdt_header.
+ // SAFETY: A valid FDT (verified by constructor) must contain a valid fdt_header.
unsafe { &*p }
}
diff --git a/microdroid/payload/Android.bp b/microdroid/payload/Android.bp
index 4814a64..8225875 100644
--- a/microdroid/payload/Android.bp
+++ b/microdroid/payload/Android.bp
@@ -31,6 +31,7 @@
protos: ["metadata.proto"],
source_stem: "microdroid_metadata",
host_supported: true,
+ use_protobuf3: true,
apex_available: [
"com.android.virt",
],
diff --git a/microdroid/payload/metadata/Android.bp b/microdroid/payload/metadata/Android.bp
index e3138e8..cd182fc 100644
--- a/microdroid/payload/metadata/Android.bp
+++ b/microdroid/payload/metadata/Android.bp
@@ -12,7 +12,7 @@
rustlibs: [
"libanyhow",
"libmicrodroid_metadata_proto_rust",
- "libprotobuf_deprecated",
+ "libprotobuf",
],
apex_available: [
"com.android.virt",
diff --git a/microdroid/payload/metadata/src/lib.rs b/microdroid/payload/metadata/src/lib.rs
index bfbec60..f00391a 100644
--- a/microdroid/payload/metadata/src/lib.rs
+++ b/microdroid/payload/metadata/src/lib.rs
@@ -24,7 +24,7 @@
use std::io::Write;
pub use microdroid_metadata::metadata::{
- ApexPayload, ApkPayload, Metadata, Metadata_oneof_payload as PayloadMetadata, PayloadConfig,
+ metadata::Payload as PayloadMetadata, ApexPayload, ApkPayload, Metadata, PayloadConfig,
};
/// Reads a metadata from a reader
diff --git a/microdroid_manager/src/dice.rs b/microdroid_manager/src/dice.rs
index 3a2a1e6..bacefcd 100644
--- a/microdroid_manager/src/dice.rs
+++ b/microdroid_manager/src/dice.rs
@@ -170,21 +170,23 @@
/// PayloadConfig = {
/// 1: tstr // payload_binary_name
/// }
-pub fn format_payload_config_descriptor(payload_metadata: &PayloadMetadata) -> Result<Vec<u8>> {
+pub fn format_payload_config_descriptor(payload: &PayloadMetadata) -> Result<Vec<u8>> {
const MICRODROID_PAYLOAD_COMPONENT_NAME: &str = "Microdroid payload";
- let config_descriptor_cbor_value = match payload_metadata {
- PayloadMetadata::config_path(payload_config_path) => cbor!({
+ let config_descriptor_cbor_value = match payload {
+ PayloadMetadata::ConfigPath(payload_config_path) => cbor!({
-70002 => MICRODROID_PAYLOAD_COMPONENT_NAME,
-71000 => payload_config_path
}),
- PayloadMetadata::config(payload_config) => cbor!({
+ PayloadMetadata::Config(payload_config) => cbor!({
-70002 => MICRODROID_PAYLOAD_COMPONENT_NAME,
-71001 => {1 => payload_config.payload_binary_name}
}),
+ _ => bail!("Failed to match the payload against a config type: {:?}", payload),
}
.context("Failed to build a CBOR Value from payload metadata")?;
let mut config_descriptor = Vec::new();
+
ser::into_writer(&config_descriptor_cbor_value, &mut config_descriptor)?;
Ok(config_descriptor)
}
@@ -196,7 +198,7 @@
#[test]
fn payload_metadata_with_path_formats_correctly() -> Result<()> {
- let payload_metadata = PayloadMetadata::config_path("/config_path".to_string());
+ let payload_metadata = PayloadMetadata::ConfigPath("/config_path".to_string());
let config_descriptor = format_payload_config_descriptor(&payload_metadata)?;
static EXPECTED_CONFIG_DESCRIPTOR: &[u8] = &[
0xa2, 0x3a, 0x00, 0x01, 0x11, 0x71, 0x72, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x64, 0x72,
@@ -214,7 +216,7 @@
payload_binary_name: "payload_binary".to_string(),
..Default::default()
};
- let payload_metadata = PayloadMetadata::config(payload_config);
+ let payload_metadata = PayloadMetadata::Config(payload_config);
let config_descriptor = format_payload_config_descriptor(&payload_metadata)?;
static EXPECTED_CONFIG_DESCRIPTOR: &[u8] = &[
0xa2, 0x3a, 0x00, 0x01, 0x11, 0x71, 0x72, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x64, 0x72,
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 9c19feb..1cdcde1 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -228,7 +228,7 @@
load_crashkernel_if_supported().context("Failed to load crashkernel")?;
- swap::init_swap().context("Failed to initialise swap")?;
+ swap::init_swap().context("Failed to initialize swap")?;
info!("swap enabled.");
let service = get_vms_rpc_binder()
@@ -435,8 +435,9 @@
// Restricted APIs are only allowed to be used by platform or test components. Infer this from
// the use of a VM config file since those can only be used by platform and test components.
let allow_restricted_apis = match payload_metadata {
- PayloadMetadata::config_path(_) => true,
- PayloadMetadata::config(_) => false,
+ PayloadMetadata::ConfigPath(_) => true,
+ PayloadMetadata::Config(_) => false,
+ _ => false, // default is false for safety
};
let config = load_config(payload_metadata).context("Failed to load payload metadata")?;
@@ -792,14 +793,14 @@
fn load_config(payload_metadata: PayloadMetadata) -> Result<VmPayloadConfig> {
match payload_metadata {
- PayloadMetadata::config_path(path) => {
+ PayloadMetadata::ConfigPath(path) => {
let path = Path::new(&path);
info!("loading config from {:?}...", path);
let file = ioutil::wait_for_file(path, WAIT_TIMEOUT)
.with_context(|| format!("Failed to read {:?}", path))?;
Ok(serde_json::from_reader(file)?)
}
- PayloadMetadata::config(payload_config) => {
+ PayloadMetadata::Config(payload_config) => {
let task = Task {
type_: TaskType::MicrodroidLauncher,
command: payload_config.payload_binary_name,
@@ -814,6 +815,7 @@
enable_authfs: false,
})
}
+ _ => bail!("Failed to match config against a config type."),
}
}
diff --git a/pvmfw/src/exceptions.rs b/pvmfw/src/exceptions.rs
index c3f8a29..797138c 100644
--- a/pvmfw/src/exceptions.rs
+++ b/pvmfw/src/exceptions.rs
@@ -114,9 +114,22 @@
}
}
+/// Prints the details of an exception failure, excluding UART exceptions.
#[inline]
-fn handling_uart_exception(esr: Esr, far: usize) -> bool {
- esr == Esr::DataAbortSyncExternalAbort && page_4kb_of(far) == UART_PAGE
+fn print_exception_failure(
+ esr: Esr,
+ far: usize,
+ elr: u64,
+ e: HandleExceptionError,
+ exception_name: &str,
+) {
+ let is_uart_exception = esr == Esr::DataAbortSyncExternalAbort && page_4kb_of(far) == UART_PAGE;
+ // Don't print to the UART if we are handling an exception it could raise.
+ if !is_uart_exception {
+ eprintln!("{exception_name}");
+ eprintln!("{e}");
+ eprintln!("{esr}, far={far:#08x}, elr={elr:#08x}");
+ }
}
#[no_mangle]
@@ -127,12 +140,7 @@
let far = read_sysreg!("far_el1");
if let Err(e) = handle_exception(esr, far) {
- // Don't print to the UART if we are handling an exception it could raise.
- if !handling_uart_exception(esr, far) {
- eprintln!("sync_exception_current");
- eprintln!("{e}");
- eprintln!("{esr}, far={far:#08x}, elr={elr:#08x}");
- }
+ print_exception_failure(esr, far, elr, e, "sync_exception_current");
reboot()
}
}
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index efb354c..319100f 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -124,8 +124,24 @@
node.setprop(cstr!("bootargs"), bootargs.to_bytes_with_nul())
}
-/// Check if memory range is ok
-fn validate_memory_range(range: &Range<usize>) -> Result<(), RebootReason> {
+/// Reads and validates the memory range in the DT.
+///
+/// Only one memory range is expected with the crosvm setup for now.
+fn read_and_validate_memory_range(fdt: &Fdt) -> Result<Range<usize>, RebootReason> {
+ let mut memory = fdt.memory().map_err(|e| {
+ error!("Failed to read memory range from DT: {e}");
+ RebootReason::InvalidFdt
+ })?;
+ let range = memory.next().ok_or_else(|| {
+ error!("The /memory node in the DT contains no range.");
+ RebootReason::InvalidFdt
+ })?;
+ if memory.next().is_some() {
+ warn!(
+ "The /memory node in the DT contains more than one memory range, \
+ while only one is expected."
+ );
+ }
let base = range.start;
if base != MEM_START {
error!("Memory base address {:#x} is not {:#x}", base, MEM_START);
@@ -142,7 +158,7 @@
error!("Memory size is 0");
return Err(RebootReason::InvalidFdt);
}
- Ok(())
+ Ok(range)
}
fn patch_memory_range(fdt: &mut Fdt, memory_range: &Range<usize>) -> libfdt::Result<()> {
@@ -600,11 +616,7 @@
RebootReason::InvalidFdt
})?;
- let memory_range = fdt.first_memory_range().map_err(|e| {
- error!("Failed to read memory range from DT: {e}");
- RebootReason::InvalidFdt
- })?;
- validate_memory_range(&memory_range)?;
+ let memory_range = read_and_validate_memory_range(fdt)?;
let bootargs = read_bootargs_from(fdt).map_err(|e| {
error!("Failed to read bootargs from DT: {e}");
diff --git a/pvmfw/src/gpt.rs b/pvmfw/src/gpt.rs
index b553705..892850c 100644
--- a/pvmfw/src/gpt.rs
+++ b/pvmfw/src/gpt.rs
@@ -24,9 +24,11 @@
use uuid::Uuid;
use virtio_drivers::device::blk::SECTOR_SIZE;
use vmbase::util::ceiling_div;
-use vmbase::virtio::pci::VirtIOBlk;
+use vmbase::virtio::{pci, HalImpl};
use zerocopy::FromBytes;
+type VirtIOBlk = pci::VirtIOBlk<HalImpl>;
+
pub enum Error {
/// VirtIO error during read operation.
FailedRead(virtio_drivers::Error),
diff --git a/pvmfw/src/instance.rs b/pvmfw/src/instance.rs
index 1035559..f2b34da 100644
--- a/pvmfw/src/instance.rs
+++ b/pvmfw/src/instance.rs
@@ -32,6 +32,7 @@
use vmbase::rand;
use vmbase::util::ceiling_div;
use vmbase::virtio::pci::{PciTransportIterator, VirtIOBlk};
+use vmbase::virtio::HalImpl;
use zerocopy::AsBytes;
use zerocopy::FromBytes;
@@ -183,10 +184,11 @@
}
fn find_instance_img(pci_root: &mut PciRoot) -> Result<Partition> {
- for transport in
- PciTransportIterator::new(pci_root).filter(|t| DeviceType::Block == t.device_type())
+ for transport in PciTransportIterator::<HalImpl>::new(pci_root)
+ .filter(|t| DeviceType::Block == t.device_type())
{
- let device = VirtIOBlk::new(transport).map_err(Error::VirtIOBlkCreationFailed)?;
+ let device =
+ VirtIOBlk::<HalImpl>::new(transport).map_err(Error::VirtIOBlkCreationFailed)?;
match Partition::get_by_name(device, "vm-instance") {
Ok(Some(p)) => return Ok(p),
Ok(None) => {}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 61e2312..ba453e7 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -95,8 +95,8 @@
// Set up PCI bus for VirtIO devices.
let pci_info = PciInfo::from_fdt(fdt).map_err(handle_pci_error)?;
debug!("PCI: {:#x?}", pci_info);
- let mut pci_root = pci::initialise(pci_info, MEMORY.lock().as_mut().unwrap()).map_err(|e| {
- error!("Failed to initialise PCI: {e}");
+ let mut pci_root = pci::initialize(pci_info, MEMORY.lock().as_mut().unwrap()).map_err(|e| {
+ error!("Failed to initialize PCI: {e}");
RebootReason::InternalError
})?;
diff --git a/rialto/Android.bp b/rialto/Android.bp
index 9aa4667..1840278 100644
--- a/rialto/Android.bp
+++ b/rialto/Android.bp
@@ -13,6 +13,7 @@
"libfdtpci",
"liblibfdt",
"liblog_rust_nostd",
+ "libvirtio_drivers",
"libvmbase",
],
}
diff --git a/rialto/src/error.rs b/rialto/src/error.rs
index 8e2991c..84228c4 100644
--- a/rialto/src/error.rs
+++ b/rialto/src/error.rs
@@ -19,7 +19,7 @@
use fdtpci::PciError;
use hyp::Error as HypervisorError;
use libfdt::FdtError;
-use vmbase::memory::MemoryTrackerError;
+use vmbase::{memory::MemoryTrackerError, virtio::pci};
pub type Result<T> = result::Result<T, Error>;
@@ -37,6 +37,8 @@
InvalidPci(PciError),
/// Failed memory operation.
MemoryOperationFailed(MemoryTrackerError),
+ /// Failed to initialize PCI.
+ PciInitializationFailed(pci::PciError),
}
impl fmt::Display for Error {
@@ -50,6 +52,7 @@
Self::InvalidFdt(e) => write!(f, "Invalid FDT: {e}"),
Self::InvalidPci(e) => write!(f, "Invalid PCI: {e}"),
Self::MemoryOperationFailed(e) => write!(f, "Failed memory operation: {e}"),
+ Self::PciInitializationFailed(e) => write!(f, "Failed to initialize PCI: {e}"),
}
}
}
diff --git a/rialto/src/main.rs b/rialto/src/main.rs
index ce83624..61c985e 100644
--- a/rialto/src/main.rs
+++ b/rialto/src/main.rs
@@ -37,12 +37,12 @@
main,
memory::{MemoryTracker, PageTable, MEMORY, PAGE_SIZE, SIZE_64KB},
power::reboot,
+ virtio::pci,
};
fn new_page_table() -> Result<PageTable> {
let mut page_table = PageTable::default();
- page_table.map_device(&crosvm::MMIO_RANGE)?;
page_table.map_data(&layout::scratch_range())?;
page_table.map_data(&layout::stack_range(40 * PAGE_SIZE))?;
page_table.map_code(&layout::text_range())?;
@@ -91,8 +91,6 @@
let fdt = unsafe { slice::from_raw_parts(fdt_range.start as *mut u8, fdt_range.len()) };
// We do not need to validate the DT since it is already validated in pvmfw.
let fdt = libfdt::Fdt::from_slice(fdt)?;
- let pci_info = PciInfo::from_fdt(fdt)?;
- debug!("PCI: {pci_info:#x?}");
let memory_range = fdt.first_memory_range()?;
MEMORY.lock().as_mut().unwrap().shrink(&memory_range).map_err(|e| {
@@ -116,6 +114,12 @@
e
})?;
}
+
+ let pci_info = PciInfo::from_fdt(fdt)?;
+ debug!("PCI: {pci_info:#x?}");
+ let pci_root = pci::initialize(pci_info, MEMORY.lock().as_mut().unwrap())
+ .map_err(Error::PciInitializationFailed)?;
+ debug!("PCI root: {pci_root:#x?}");
Ok(())
}
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index dc045a1..5ec4ca8 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -555,6 +555,14 @@
.setVmOutputCaptured(true);
e = assertThrows(IllegalStateException.class, () -> captureOutputOnNonDebuggable.build());
assertThat(e).hasMessageThat().contains("debug level must be FULL to capture output");
+
+ VirtualMachineConfig.Builder captureInputOnNonDebuggable =
+ newVmConfigBuilder()
+ .setPayloadBinaryName("binary.so")
+ .setDebugLevel(VirtualMachineConfig.DEBUG_LEVEL_NONE)
+ .setVmConsoleInputSupported(true);
+ e = assertThrows(IllegalStateException.class, () -> captureInputOnNonDebuggable.build());
+ assertThat(e).hasMessageThat().contains("debug level must be FULL to use console input");
}
@Test
@@ -593,6 +601,9 @@
newBaselineBuilder().setDebugLevel(DEBUG_LEVEL_FULL);
VirtualMachineConfig debuggable = debuggableBuilder.build();
assertConfigCompatible(debuggable, debuggableBuilder.setVmOutputCaptured(true)).isFalse();
+ assertConfigCompatible(debuggable, debuggableBuilder.setVmOutputCaptured(false)).isTrue();
+ assertConfigCompatible(debuggable, debuggableBuilder.setVmConsoleInputSupported(true))
+ .isFalse();
VirtualMachineConfig currentContextConfig =
new VirtualMachineConfig.Builder(getContext())
@@ -1582,6 +1593,7 @@
.setProtectedVm(mProtectedVm)
.setPayloadBinaryName("MicrodroidTestNativeLib.so")
.setDebugLevel(DEBUG_LEVEL_FULL)
+ .setVmConsoleInputSupported(true) // even if console input is supported
.build();
final VirtualMachine vm = forceCreateNewVirtualMachine("test_vm_forward_log", vmConfig);
vm.run();
@@ -1596,6 +1608,28 @@
}
}
+ @Test
+ public void inputShouldBeExplicitlyAllowed() throws Exception {
+ assumeSupportedDevice();
+
+ final VirtualMachineConfig vmConfig =
+ new VirtualMachineConfig.Builder(getContext())
+ .setProtectedVm(mProtectedVm)
+ .setPayloadBinaryName("MicrodroidTestNativeLib.so")
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .setVmOutputCaptured(true) // even if output is captured
+ .build();
+ final VirtualMachine vm = forceCreateNewVirtualMachine("test_vm_forward_log", vmConfig);
+ vm.run();
+
+ try {
+ assertThrowsVmExceptionContaining(
+ () -> vm.getConsoleInput(), "VM console input is not supported");
+ } finally {
+ vm.stop();
+ }
+ }
+
private boolean checkVmOutputIsRedirectedToLogcat(boolean debuggable) throws Exception {
String time =
LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS"));
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 13367c3..8c412f6 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -527,7 +527,7 @@
}
}
Ok(VmResponse::Err(e)) => {
- // ENOTSUP is returned when the balloon protocol is not initialised. This
+ // ENOTSUP is returned when the balloon protocol is not initialized. This
// can occur for numerous reasons: Guest is still booting, guest doesn't
// support ballooning, host doesn't support ballooning. We don't log or
// raise an error in this case: trim is just a hint and we can ignore it.
@@ -792,7 +792,7 @@
// devices in the same PCI bus and serial devices comes before the block devices. Arm crosvm
// doesn't have the issue.
// /dev/ttyS0
- command.arg(format!("--serial={}{},hardware=serial,num=1", &console_out_arg, &console_in_arg));
+ command.arg(format!("--serial={},hardware=serial,num=1", &console_out_arg));
// /dev/ttyS1
command.arg(format!("--serial=type=file,path={},hardware=serial,num=2", &failure_serial_path));
// /dev/hvc0
diff --git a/virtualizationmanager/src/payload.rs b/virtualizationmanager/src/payload.rs
index 33659d4..733add6 100644
--- a/virtualizationmanager/src/payload.rs
+++ b/virtualizationmanager/src/payload.rs
@@ -194,12 +194,12 @@
temporary_directory: &Path,
) -> Result<ParcelFileDescriptor> {
let payload_metadata = match &app_config.payload {
- Payload::PayloadConfig(payload_config) => PayloadMetadata::config(PayloadConfig {
+ Payload::PayloadConfig(payload_config) => PayloadMetadata::Config(PayloadConfig {
payload_binary_name: payload_config.payloadBinaryName.clone(),
..Default::default()
}),
Payload::ConfigPath(config_path) => {
- PayloadMetadata::config_path(format!("/mnt/apk/{}", config_path))
+ PayloadMetadata::ConfigPath(format!("/mnt/apk/{}", config_path))
}
};
diff --git a/vmbase/README.md b/vmbase/README.md
index 7f621fb..280d7e1 100644
--- a/vmbase/README.md
+++ b/vmbase/README.md
@@ -6,7 +6,7 @@
In particular it provides:
-- An [entry point](entry.S) that initialises the MMU with a hard-coded identity mapping, enables the
+- An [entry point](entry.S) that initializes the MMU with a hard-coded identity mapping, enables the
cache, prepares the image and allocates a stack.
- An [exception vector](exceptions.S) to call your exception handlers.
- A UART driver and `println!` macro for early console logging.
@@ -62,7 +62,7 @@
}
```
-vmbase adds a wrapper around your main function to initialise the console driver first (with the
+vmbase adds a wrapper around your main function to initialize the console driver first (with the
UART at base address `0x3f8`, the first UART allocated by crosvm), and make a PSCI `SYSTEM_OFF` call
to shutdown the VM if your main function ever returns.
@@ -93,7 +93,7 @@
The `println!` macro shouldn't be used in exception handlers, because it relies on a global instance
of the UART driver which might be locked when the exception happens, which would result in deadlock.
-Instead you can use `emergency_write_str` and `eprintln!`, which will re-initialise the UART every
+Instead you can use `emergency_write_str` and `eprintln!`, which will re-initialize the UART every
time to ensure that it can be used. This should still be used with care, as it may interfere with
whatever the rest of the program is doing with the UART.
diff --git a/vmbase/example/src/layout.rs b/vmbase/example/src/layout.rs
index 2e9d27a..1954a90 100644
--- a/vmbase/example/src/layout.rs
+++ b/vmbase/example/src/layout.rs
@@ -19,7 +19,6 @@
use core::ops::Range;
use log::info;
use vmbase::layout;
-use vmbase::STACK_CHK_GUARD;
/// The first 1 GiB of memory are used for MMIO.
pub const DEVICE_REGION: MemoryRegion = MemoryRegion::new(0, 0x40000000);
@@ -48,7 +47,7 @@
into_va_range(layout::data_range())
}
-/// Zero-initialised writable data.
+/// Zero-initialized writable data.
pub fn bss_range() -> Range<VirtualAddress> {
into_va_range(layout::bss_range())
}
@@ -108,8 +107,3 @@
*ptr
}
}
-
-/// Value of __stack_chk_guard.
-pub fn stack_chk_guard() -> u64 {
- *STACK_CHK_GUARD
-}
diff --git a/vmbase/example/src/main.rs b/vmbase/example/src/main.rs
index b3b5732..021daa4 100644
--- a/vmbase/example/src/main.rs
+++ b/vmbase/example/src/main.rs
@@ -25,7 +25,7 @@
use crate::layout::{
bionic_tls, boot_stack_range, dtb_range, print_addresses, rodata_range, scratch_range,
- stack_chk_guard, text_range, DEVICE_REGION,
+ text_range, DEVICE_REGION,
};
use crate::pci::{check_pci, get_bar_region};
use aarch64_paging::{idmap::IdMap, paging::Attributes};
@@ -33,7 +33,7 @@
use fdtpci::PciInfo;
use libfdt::Fdt;
use log::{debug, error, info, trace, warn, LevelFilter};
-use vmbase::{configure_heap, cstr, logger, main, memory::SIZE_64KB};
+use vmbase::{configure_heap, cstr, layout::stack_chk_guard, logger, main, memory::SIZE_64KB};
static INITIALISED_DATA: [u32; 4] = [1, 2, 3, 4];
static mut ZEROED_DATA: [u32; 10] = [0; 10];
diff --git a/vmbase/example/src/pci.rs b/vmbase/example/src/pci.rs
index 6abe66e..7188cde 100644
--- a/vmbase/example/src/pci.rs
+++ b/vmbase/example/src/pci.rs
@@ -20,13 +20,14 @@
use fdtpci::PciInfo;
use log::{debug, info};
use virtio_drivers::{
- device::{blk::VirtIOBlk, console::VirtIOConsole},
+ device::console::VirtIOConsole,
transport::{
- pci::{bus::PciRoot, virtio_device_type, PciTransport},
+ pci::{bus::PciRoot, PciTransport},
DeviceType, Transport,
},
BufferDirection, Error, Hal, PhysAddr, PAGE_SIZE,
};
+use vmbase::virtio::pci::{self, PciTransportIterator};
/// The standard sector size of a VirtIO block device, in bytes.
const SECTOR_SIZE_BYTES: usize = 512;
@@ -37,29 +38,23 @@
pub fn check_pci(pci_root: &mut PciRoot) {
let mut checked_virtio_device_count = 0;
let mut block_device_count = 0;
- for (device_function, info) in pci_root.enumerate_bus(0) {
- let (status, command) = pci_root.get_status_command(device_function);
- info!("Found {} at {}, status {:?} command {:?}", info, device_function, status, command);
- if let Some(virtio_type) = virtio_device_type(&info) {
- info!(" VirtIO {:?}", virtio_type);
- let mut transport = PciTransport::new::<HalImpl>(pci_root, device_function).unwrap();
- info!(
- "Detected virtio PCI device with device type {:?}, features {:#018x}",
- transport.device_type(),
- transport.read_device_features(),
- );
- match virtio_type {
- DeviceType::Block => {
- check_virtio_block_device(transport, block_device_count);
- block_device_count += 1;
- checked_virtio_device_count += 1;
- }
- DeviceType::Console => {
- check_virtio_console_device(transport);
- checked_virtio_device_count += 1;
- }
- _ => {}
+ for mut transport in PciTransportIterator::<HalImpl>::new(pci_root) {
+ info!(
+ "Detected virtio PCI device with device type {:?}, features {:#018x}",
+ transport.device_type(),
+ transport.read_device_features(),
+ );
+ match transport.device_type() {
+ DeviceType::Block => {
+ check_virtio_block_device(transport, block_device_count);
+ block_device_count += 1;
+ checked_virtio_device_count += 1;
}
+ DeviceType::Console => {
+ check_virtio_console_device(transport);
+ checked_virtio_device_count += 1;
+ }
+ _ => {}
}
}
@@ -68,8 +63,8 @@
}
/// Checks the given VirtIO block device.
-fn check_virtio_block_device(transport: impl Transport, index: usize) {
- let mut blk = VirtIOBlk::<HalImpl, _>::new(transport).expect("failed to create blk driver");
+fn check_virtio_block_device(transport: PciTransport, index: usize) {
+ let mut blk = pci::VirtIOBlk::<HalImpl>::new(transport).expect("failed to create blk driver");
info!("Found {} KiB block device.", blk.capacity() * SECTOR_SIZE_BYTES as u64 / 1024);
match index {
0 => {
@@ -94,8 +89,8 @@
}
/// Checks the given VirtIO console device.
-fn check_virtio_console_device(transport: impl Transport) {
- let mut console = VirtIOConsole::<HalImpl, _>::new(transport)
+fn check_virtio_console_device(transport: PciTransport) {
+ let mut console = VirtIOConsole::<HalImpl, PciTransport>::new(transport)
.expect("Failed to create VirtIO console driver");
info!("Found console device: {:?}", console.info());
for &c in b"Hello VirtIO console\n" {
diff --git a/vmbase/src/arch.rs b/vmbase/src/arch.rs
index d7b63b3..d8bb8b2 100644
--- a/vmbase/src/arch.rs
+++ b/vmbase/src/arch.rs
@@ -19,8 +19,8 @@
macro_rules! read_sysreg {
($sysreg:literal) => {{
let mut r: usize;
- // Safe because it reads a system register and does not affect Rust.
#[allow(unused_unsafe)] // In case the macro is used within an unsafe block.
+ // SAFETY: Reading a system register does not affect memory.
unsafe {
core::arch::asm!(
concat!("mrs {}, ", $sysreg),
@@ -53,8 +53,8 @@
#[macro_export]
macro_rules! isb {
() => {{
- // Safe because this is just a memory barrier and does not affect Rust.
#[allow(unused_unsafe)] // In case the macro is used within an unsafe block.
+ // SAFETY: memory barriers do not affect Rust's memory model.
unsafe {
core::arch::asm!("isb", options(nomem, nostack, preserves_flags));
}
@@ -65,8 +65,8 @@
#[macro_export]
macro_rules! dsb {
($option:literal) => {{
- // Safe because this is just a memory barrier and does not affect Rust.
#[allow(unused_unsafe)] // In case the macro is used within an unsafe block.
+ // SAFETY: memory barriers do not affect Rust's memory model.
unsafe {
core::arch::asm!(concat!("dsb ", $option), options(nomem, nostack, preserves_flags));
}
@@ -79,9 +79,9 @@
($option:literal, $asid:expr, $addr:expr) => {{
let asid: usize = $asid;
let addr: usize = $addr;
- // Safe because it invalidates TLB and doesn't affect Rust. When the address matches a
- // block entry larger than the page size, all translations for the block are invalidated.
#[allow(unused_unsafe)] // In case the macro is used within an unsafe block.
+ // SAFETY: Invalidating the TLB doesn't affect Rust. When the address matches a
+ // block entry larger than the page size, all translations for the block are invalidated.
unsafe {
core::arch::asm!(
concat!("tlbi ", $option, ", {x}"),
diff --git a/vmbase/src/bionic.rs b/vmbase/src/bionic.rs
index 69da521..5af9ebc 100644
--- a/vmbase/src/bionic.rs
+++ b/vmbase/src/bionic.rs
@@ -23,13 +23,9 @@
use crate::console;
use crate::eprintln;
-use crate::linker;
const EOF: c_int = -1;
-/// Reference to __stack_chk_guard.
-pub static STACK_CHK_GUARD: &u64 = unsafe { &linker::__stack_chk_guard };
-
#[no_mangle]
extern "C" fn __stack_chk_fail() -> ! {
panic!("stack guard check failed");
@@ -46,11 +42,13 @@
#[no_mangle]
unsafe extern "C" fn __errno() -> *mut c_int {
- &mut ERRNO as *mut _
+ // SAFETY: C functions which call this are only called from the main thread, not from exception
+ // handlers.
+ unsafe { &mut ERRNO as *mut _ }
}
fn set_errno(value: c_int) {
- // SAFETY - vmbase is currently single-threaded.
+ // SAFETY: vmbase is currently single-threaded.
unsafe { ERRNO = value };
}
@@ -58,15 +56,15 @@
///
/// # Safety
///
-/// Input strings `prefix` and `format` must be properly NULL-terminated.
+/// Input strings `prefix` and `format` must be valid and properly NUL-terminated.
///
/// # Note
///
/// This Rust functions is missing the last argument of its C/C++ counterpart, a va_list.
#[no_mangle]
unsafe extern "C" fn async_safe_fatal_va_list(prefix: *const c_char, format: *const c_char) {
- let prefix = CStr::from_ptr(prefix);
- let format = CStr::from_ptr(format);
+ // SAFETY: The caller guaranteed that both strings were valid and NUL-terminated.
+ let (prefix, format) = unsafe { (CStr::from_ptr(prefix), CStr::from_ptr(format)) };
if let (Ok(prefix), Ok(format)) = (prefix.to_str(), format.to_str()) {
// We don't bother with printf formatting.
@@ -100,7 +98,7 @@
#[no_mangle]
extern "C" fn fputs(c_str: *const c_char, stream: usize) -> c_int {
- // SAFETY - Just like libc, we need to assume that `s` is a valid NULL-terminated string.
+ // SAFETY: Just like libc, we need to assume that `s` is a valid NULL-terminated string.
let c_str = unsafe { CStr::from_ptr(c_str) };
if let (Ok(s), Ok(_)) = (c_str.to_str(), File::try_from(stream)) {
@@ -116,7 +114,7 @@
extern "C" fn fwrite(ptr: *const c_void, size: usize, nmemb: usize, stream: usize) -> usize {
let length = size.saturating_mul(nmemb);
- // SAFETY - Just like libc, we need to assume that `ptr` is valid.
+ // SAFETY: Just like libc, we need to assume that `ptr` is valid.
let bytes = unsafe { slice::from_raw_parts(ptr as *const u8, length) };
if let (Ok(s), Ok(_)) = (str::from_utf8(bytes), File::try_from(stream)) {
diff --git a/vmbase/src/console.rs b/vmbase/src/console.rs
index 7c8ddf6..a7d37b4 100644
--- a/vmbase/src/console.rs
+++ b/vmbase/src/console.rs
@@ -25,7 +25,7 @@
/// Initialises a new instance of the UART driver and returns it.
fn create() -> Uart {
- // Safe because BASE_ADDRESS is the base of the MMIO region for a UART and is mapped as device
+ // SAFETY: BASE_ADDRESS is the base of the MMIO region for a UART and is mapped as device
// memory.
unsafe { Uart::new(BASE_ADDRESS) }
}
@@ -51,7 +51,7 @@
write(CONSOLE.lock().as_mut().unwrap(), format_args).unwrap();
}
-/// Reinitialises the UART driver and writes a string to it.
+/// Reinitializes the UART driver and writes a string to it.
///
/// This is intended for use in situations where the UART may be in an unknown state or the global
/// instance may be locked, such as in an exception handler or panic handler.
@@ -60,7 +60,7 @@
let _ = uart.write_str(s);
}
-/// Reinitialises the UART driver and writes a formatted string to it.
+/// Reinitializes the UART driver and writes a formatted string to it.
///
/// This is intended for use in situations where the UART may be in an unknown state or the global
/// instance may be locked, such as in an exception handler or panic handler.
@@ -71,7 +71,7 @@
/// Prints the given formatted string to the console, followed by a newline.
///
-/// Panics if the console has not yet been initialised. May hang if used in an exception context;
+/// Panics if the console has not yet been initialized. May hang if used in an exception context;
/// use `eprintln!` instead.
macro_rules! println {
() => ($crate::console::write_str("\n"));
diff --git a/vmbase/src/entry.rs b/vmbase/src/entry.rs
index df0bb7c..0a96d86 100644
--- a/vmbase/src/entry.rs
+++ b/vmbase/src/entry.rs
@@ -19,9 +19,11 @@
/// This is the entry point to the Rust code, called from the binary entry point in `entry.S`.
#[no_mangle]
extern "C" fn rust_entry(x0: u64, x1: u64, x2: u64, x3: u64) -> ! {
- // SAFETY - Only called once, from here, and inaccessible to client code.
+ // SAFETY: Only called once, from here, and inaccessible to client code.
unsafe { heap::init() };
console::init();
+ // SAFETY: `main` is provided by the application using the `main!` macro, and we make sure it
+ // has the right type.
unsafe {
main(x0, x1, x2, x3);
}
diff --git a/vmbase/src/heap.rs b/vmbase/src/heap.rs
index b00ca6f..c8b76ac 100644
--- a/vmbase/src/heap.rs
+++ b/vmbase/src/heap.rs
@@ -33,7 +33,7 @@
($len:expr) => {
static mut __HEAP_ARRAY: [u8; $len] = [0; $len];
#[export_name = "HEAP"]
- // SAFETY - HEAP will only be accessed once as mut, from init().
+ // SAFETY: HEAP will only be accessed once as mut, from init().
static mut __HEAP: &'static mut [u8] = unsafe { &mut __HEAP_ARRAY };
};
}
@@ -65,12 +65,12 @@
pub fn aligned_boxed_slice(size: usize, align: usize) -> Option<Box<[u8]>> {
let size = NonZeroUsize::new(size)?.get();
let layout = Layout::from_size_align(size, align).ok()?;
- // SAFETY - We verify that `size` and the returned `ptr` are non-null.
+ // SAFETY: We verify that `size` and the returned `ptr` are non-null.
let ptr = unsafe { alloc(layout) };
let ptr = NonNull::new(ptr)?.as_ptr();
let slice_ptr = ptr::slice_from_raw_parts_mut(ptr, size);
- // SAFETY - The memory was allocated using the proper layout by our global_allocator.
+ // SAFETY: The memory was allocated using the proper layout by our global_allocator.
Some(unsafe { Box::from_raw(slice_ptr) })
}
@@ -100,9 +100,9 @@
heap_range.contains(&(ptr.as_ptr() as *const u8)),
"free() called on a pointer that is not part of the HEAP: {ptr:?}"
);
+ // SAFETY: ptr is non-null and was allocated by allocate, which prepends a correctly aligned
+ // usize.
let (ptr, size) = unsafe {
- // SAFETY: ptr is non-null and was allocated by allocate, which prepends a correctly aligned
- // usize.
let ptr = ptr.cast::<usize>().as_ptr().offset(-1);
(ptr, *ptr)
};
diff --git a/vmbase/src/layout/mod.rs b/vmbase/src/layout/mod.rs
index 21c113a..bca5115 100644
--- a/vmbase/src/layout/mod.rs
+++ b/vmbase/src/layout/mod.rs
@@ -17,6 +17,7 @@
pub mod crosvm;
use crate::console::BASE_ADDRESS;
+use crate::linker::__stack_chk_guard;
use core::ops::Range;
use core::ptr::addr_of;
@@ -27,6 +28,8 @@
#[macro_export]
macro_rules! linker_addr {
($symbol:ident) => {{
+ // SAFETY: We're just getting the address of an extern static symbol provided by the linker,
+ // not dereferencing it.
unsafe { addr_of!($crate::linker::$symbol) as usize }
}};
}
@@ -62,7 +65,7 @@
linker_region!(data_begin, data_end)
}
-/// Zero-initialised writable data.
+/// Zero-initialized writable data.
pub fn bss_range() -> Range<usize> {
linker_region!(bss_begin, bss_end)
}
@@ -97,3 +100,11 @@
pub fn binary_end() -> usize {
linker_addr!(bin_end)
}
+
+/// Value of __stack_chk_guard.
+pub fn stack_chk_guard() -> u64 {
+ // SAFETY: __stack_chk_guard shouldn't have any mutable aliases unless the stack overflows. If
+ // it does, then there could be undefined behaviour all over the program, but we want to at
+ // least have a chance at catching it.
+ unsafe { addr_of!(__stack_chk_guard).read_volatile() }
+}
diff --git a/vmbase/src/lib.rs b/vmbase/src/lib.rs
index 88bad8b..e490faa 100644
--- a/vmbase/src/lib.rs
+++ b/vmbase/src/lib.rs
@@ -15,6 +15,8 @@
//! Basic functionality for bare-metal binaries to run in a VM under crosvm.
#![no_std]
+#![deny(unsafe_op_in_unsafe_fn)]
+#![deny(clippy::undocumented_unsafe_blocks)]
extern crate alloc;
@@ -35,8 +37,6 @@
pub mod util;
pub mod virtio;
-pub use bionic::STACK_CHK_GUARD;
-
use core::panic::PanicInfo;
use power::reboot;
diff --git a/vmbase/src/logger.rs b/vmbase/src/logger.rs
index c30adad..226d905 100644
--- a/vmbase/src/logger.rs
+++ b/vmbase/src/logger.rs
@@ -25,14 +25,15 @@
struct Logger {
is_enabled: AtomicBool,
}
-static mut LOGGER: Logger = Logger::new();
+
+static LOGGER: Logger = Logger::new();
impl Logger {
const fn new() -> Self {
Self { is_enabled: AtomicBool::new(true) }
}
- fn swap_enabled(&mut self, enabled: bool) -> bool {
+ fn swap_enabled(&self, enabled: bool) -> bool {
self.is_enabled.swap(enabled, Ordering::Relaxed)
}
}
@@ -58,26 +59,19 @@
impl SuppressGuard {
fn new() -> Self {
- // Safe because it modifies an atomic.
- unsafe { Self { old_enabled: LOGGER.swap_enabled(false) } }
+ Self { old_enabled: LOGGER.swap_enabled(false) }
}
}
impl Drop for SuppressGuard {
fn drop(&mut self) {
- // Safe because it modifies an atomic.
- unsafe {
- LOGGER.swap_enabled(self.old_enabled);
- }
+ LOGGER.swap_enabled(self.old_enabled);
}
}
/// Initialize vmbase logger with a given max logging level.
pub fn init(max_level: LevelFilter) -> Result<(), SetLoggerError> {
- // Safe because it only sets the global logger.
- unsafe {
- log::set_logger(&LOGGER)?;
- }
+ log::set_logger(&LOGGER)?;
log::set_max_level(max_level);
Ok(())
}
diff --git a/vmbase/src/memory/dbm.rs b/vmbase/src/memory/dbm.rs
index d429b30..401022e 100644
--- a/vmbase/src/memory/dbm.rs
+++ b/vmbase/src/memory/dbm.rs
@@ -34,7 +34,7 @@
} else {
tcr &= !TCR_EL1_HA_HD_BITS
};
- // Safe because it writes to a system register and does not affect Rust.
+ // SAFETY: Changing this bit in TCR doesn't affect Rust's view of memory.
unsafe { write_sysreg!("tcr_el1", tcr) }
isb!();
}
diff --git a/vmbase/src/memory/shared.rs b/vmbase/src/memory/shared.rs
index 61cbeb0..4a75b97 100644
--- a/vmbase/src/memory/shared.rs
+++ b/vmbase/src/memory/shared.rs
@@ -69,6 +69,8 @@
payload_range: Option<MemoryRange>,
}
+// TODO: Remove this once aarch64-paging crate is updated.
+// SAFETY: Only `PageTable` doesn't implement Send, but it should.
unsafe impl Send for MemoryTracker {}
impl MemoryTracker {
@@ -93,7 +95,7 @@
set_dbm_enabled(true);
debug!("Activating dynamic page table...");
- // SAFETY - page_table duplicates the static mappings for everything that the Rust code is
+ // SAFETY: page_table duplicates the static mappings for everything that the Rust code is
// aware of so activating it shouldn't have any visible effect.
unsafe { page_table.activate() }
debug!("... Success!");
@@ -368,7 +370,7 @@
fn refill(&mut self, pool: &mut FrameAllocator<32>, hint: Layout) {
let layout = hint.align_to(self.granule).unwrap().pad_to_align();
assert_ne!(layout.size(), 0);
- // SAFETY - layout has non-zero size.
+ // SAFETY: layout has non-zero size.
let Some(shared) = NonNull::new(unsafe { alloc_zeroed(layout) }) else {
handle_alloc_error(layout);
};
@@ -396,7 +398,7 @@
get_hypervisor().mem_unshare(virt_to_phys(vaddr).try_into().unwrap()).unwrap();
}
- // SAFETY - The region was obtained from alloc_zeroed() with the recorded layout.
+ // SAFETY: The region was obtained from alloc_zeroed() with the recorded layout.
unsafe { dealloc(base as *mut _, layout) };
}
}
diff --git a/vmbase/src/memory/util.rs b/vmbase/src/memory/util.rs
index b9ef5c9..48d4c55 100644
--- a/vmbase/src/memory/util.rs
+++ b/vmbase/src/memory/util.rs
@@ -55,7 +55,7 @@
let start = unchecked_align_down(start, line_size);
for line in (start..end).step_by(line_size) {
- // SAFETY - Clearing cache lines shouldn't have Rust-visible side effects.
+ // SAFETY: Clearing cache lines shouldn't have Rust-visible side effects.
unsafe {
asm!(
"dc cvau, {x}",
diff --git a/vmbase/src/rand.rs b/vmbase/src/rand.rs
index 00567b8..26fb51a 100644
--- a/vmbase/src/rand.rs
+++ b/vmbase/src/rand.rs
@@ -114,7 +114,7 @@
#[no_mangle]
extern "C" fn CRYPTO_sysrand(out: *mut u8, req: usize) {
- // SAFETY - We need to assume that out points to valid memory of size req.
+ // SAFETY: We need to assume that out points to valid memory of size req.
let s = unsafe { core::slice::from_raw_parts_mut(out, req) };
fill_with_entropy(s).unwrap()
}
diff --git a/vmbase/src/uart.rs b/vmbase/src/uart.rs
index 0fc2494..09d747f 100644
--- a/vmbase/src/uart.rs
+++ b/vmbase/src/uart.rs
@@ -38,8 +38,8 @@
/// Writes a single byte to the UART.
pub fn write_byte(&self, byte: u8) {
- // Safe because we know that the base address points to the control registers of an UART
- // device which is appropriately mapped.
+ // SAFETY: We know that the base address points to the control registers of a UART device
+ // which is appropriately mapped.
unsafe {
write_volatile(self.base_address, byte);
}
@@ -55,5 +55,5 @@
}
}
-// Safe because it just contains a pointer to device memory, which can be accessed from any context.
+// SAFETY: `Uart` just contains a pointer to device memory, which can be accessed from any context.
unsafe impl Send for Uart {}
diff --git a/vmbase/src/virtio/hal.rs b/vmbase/src/virtio/hal.rs
index 36f9e56..0d3f445 100644
--- a/vmbase/src/virtio/hal.rs
+++ b/vmbase/src/virtio/hal.rs
@@ -32,10 +32,8 @@
/// HAL implementation for the virtio_drivers crate.
pub struct HalImpl;
-/// # Safety
-///
-/// See the 'Implementation Safety' comments on methods below for how they fulfill the safety
-/// requirements of the unsafe `Hal` trait.
+/// SAFETY: See the 'Implementation Safety' comments on methods below for how they fulfill the
+/// safety requirements of the unsafe `Hal` trait.
unsafe impl Hal for HalImpl {
/// # Implementation Safety
///
@@ -48,14 +46,14 @@
let layout = dma_layout(pages);
let vaddr =
alloc_shared(layout).expect("Failed to allocate and share VirtIO DMA range with host");
- // SAFETY - vaddr points to a region allocated for the caller so is safe to access.
+ // SAFETY: vaddr points to a region allocated for the caller so is safe to access.
unsafe { core::ptr::write_bytes(vaddr.as_ptr(), 0, layout.size()) };
let paddr = virt_to_phys(vaddr);
(paddr, vaddr)
}
unsafe fn dma_dealloc(_paddr: PhysAddr, vaddr: NonNull<u8>, pages: usize) -> i32 {
- // SAFETY - Memory was allocated by `dma_alloc` using `alloc_shared` with the same layout.
+ // SAFETY: Memory was allocated by `dma_alloc` using `alloc_shared` with the same layout.
unsafe { dealloc_shared(vaddr, dma_layout(pages)) }
.expect("Failed to unshare VirtIO DMA range with host");
0
@@ -68,7 +66,7 @@
/// range. It can't alias any other allocations because we previously validated in
/// `map_mmio_range` that the PCI MMIO range didn't overlap with any other memory ranges.
unsafe 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");
+ let pci_info = PCI_INFO.get().expect("VirtIO HAL used before PCI_INFO was initialized");
let bar_range = {
let start = pci_info.bar_range.start.try_into().unwrap();
let end = pci_info.bar_range.end.try_into().unwrap();
@@ -96,7 +94,7 @@
if direction == BufferDirection::DriverToDevice {
let src = buffer.cast::<u8>().as_ptr().cast_const();
trace!("VirtIO bounce buffer at {bounce:?} (PA:{paddr:#x}) initialized from {src:?}");
- // SAFETY - Both regions are valid, properly aligned, and don't overlap.
+ // SAFETY: Both regions are valid, properly aligned, and don't overlap.
unsafe { copy_nonoverlapping(src, bounce.as_ptr(), size) };
}
@@ -109,11 +107,11 @@
if direction == BufferDirection::DeviceToDriver {
let dest = buffer.cast::<u8>().as_ptr();
trace!("VirtIO bounce buffer at {bounce:?} (PA:{paddr:#x}) copied back to {dest:?}");
- // SAFETY - Both regions are valid, properly aligned, and don't overlap.
+ // SAFETY: Both regions are valid, properly aligned, and don't overlap.
unsafe { copy_nonoverlapping(bounce.as_ptr(), dest, size) };
}
- // SAFETY - Memory was allocated by `share` using `alloc_shared` with the same layout.
+ // SAFETY: Memory was allocated by `share` using `alloc_shared` with the same layout.
unsafe { dealloc_shared(bounce, bb_layout(size)) }
.expect("Failed to unshare and deallocate VirtIO bounce buffer");
}
diff --git a/vmbase/src/virtio/mod.rs b/vmbase/src/virtio/mod.rs
index df916bc..fbe41e3 100644
--- a/vmbase/src/virtio/mod.rs
+++ b/vmbase/src/virtio/mod.rs
@@ -16,3 +16,5 @@
mod hal;
pub mod pci;
+
+pub use hal::HalImpl;
diff --git a/vmbase/src/virtio/pci.rs b/vmbase/src/virtio/pci.rs
index cbb4d26..a75f0e2 100644
--- a/vmbase/src/virtio/pci.rs
+++ b/vmbase/src/virtio/pci.rs
@@ -14,10 +14,10 @@
//! Functions to scan the PCI bus for VirtIO devices.
-use super::hal::HalImpl;
use crate::memory::{MemoryTracker, MemoryTrackerError};
use alloc::boxed::Box;
use core::fmt;
+use core::marker::PhantomData;
use fdtpci::PciInfo;
use log::debug;
use once_cell::race::OnceBox;
@@ -27,6 +27,7 @@
bus::{BusDeviceIterator, PciRoot},
virtio_device_type, PciTransport,
},
+ Hal,
};
pub(super) static PCI_INFO: OnceBox<PciInfo> = OnceBox::new();
@@ -63,7 +64,7 @@
/// 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, PciError> {
+pub fn initialize(pci_info: PciInfo, memory: &mut MemoryTracker) -> Result<PciRoot, PciError> {
PCI_INFO.set(Box::new(pci_info.clone())).map_err(|_| PciError::DuplicateInitialization)?;
memory.map_mmio_range(pci_info.cam_range.clone()).map_err(PciError::CamMapFailed)?;
@@ -76,23 +77,24 @@
}
/// Virtio Block device.
-pub type VirtIOBlk = blk::VirtIOBlk<HalImpl, PciTransport>;
+pub type VirtIOBlk<T> = blk::VirtIOBlk<T, PciTransport>;
/// An iterator that iterates over the PCI transport for each device.
-pub struct PciTransportIterator<'a> {
+pub struct PciTransportIterator<'a, T: Hal> {
pci_root: &'a mut PciRoot,
bus: BusDeviceIterator,
+ _hal: PhantomData<T>,
}
-impl<'a> PciTransportIterator<'a> {
+impl<'a, T: Hal> PciTransportIterator<'a, T> {
/// Creates a new iterator.
pub fn new(pci_root: &'a mut PciRoot) -> Self {
let bus = pci_root.enumerate_bus(0);
- Self { pci_root, bus }
+ Self { pci_root, bus, _hal: PhantomData }
}
}
-impl<'a> Iterator for PciTransportIterator<'a> {
+impl<'a, T: Hal> Iterator for PciTransportIterator<'a, T> {
type Item = PciTransport;
fn next(&mut self) -> Option<Self::Item> {
@@ -109,7 +111,7 @@
};
debug!(" VirtIO {:?}", virtio_type);
- return PciTransport::new::<HalImpl>(self.pci_root, device_function).ok();
+ return PciTransport::new::<T>(self.pci_root, device_function).ok();
}
}
}