Merge "[test][vmbase] Check VirtIO socket device in integration test"
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/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index ffb2c11..5ec4ca8 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -128,6 +128,13 @@
public void setup() {
grantPermission(VirtualMachine.MANAGE_VIRTUAL_MACHINE_PERMISSION);
prepareTestSetup(mProtectedVm);
+ // USE_CUSTOM_VIRTUAL_MACHINE permission has protection level signature|development, meaning
+ // that it will be automatically granted when test apk is installed. We have some tests
+ // checking the behavior when caller doesn't have this permission (e.g.
+ // createVmWithConfigRequiresPermission). Proactively revoke the permission so that such
+ // tests can pass when ran by itself, e.g.:
+ // atest com.android.microdroid.test.MicrodroidTests#createVmWithConfigRequiresPermission
+ revokePermission(VirtualMachine.USE_CUSTOM_VIRTUAL_MACHINE_PERMISSION);
}
@After
@@ -548,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
@@ -586,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())
@@ -1575,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();
@@ -1589,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/vmbase/example/src/layout.rs b/vmbase/example/src/layout.rs
index f95958f..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);
@@ -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/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 e9298cc..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) }
}
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 f67e518..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 }
}};
}
@@ -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 c84ca5e..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
@@ -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");
}