Merge "Add test for enabled adb and logcat"
diff --git a/authfs/fd_server/Android.bp b/authfs/fd_server/Android.bp
index 5097408..db1fd44 100644
--- a/authfs/fd_server/Android.bp
+++ b/authfs/fd_server/Android.bp
@@ -12,6 +12,7 @@
"libauthfs_fsverity_metadata",
"libbinder_rs",
"libclap",
+ "libfsverity_rs",
"liblibc",
"liblog_rust",
"libnix",
@@ -31,6 +32,7 @@
"libauthfs_fsverity_metadata",
"libbinder_rs",
"libclap",
+ "libfsverity_rs",
"liblibc",
"liblog_rust",
"libnix",
diff --git a/authfs/fd_server/src/aidl.rs b/authfs/fd_server/src/aidl.rs
index 01b8209..ada3ffb 100644
--- a/authfs/fd_server/src/aidl.rs
+++ b/authfs/fd_server/src/aidl.rs
@@ -31,7 +31,6 @@
use std::path::{Component, Path, PathBuf, MAIN_SEPARATOR};
use std::sync::{Arc, RwLock};
-use crate::fsverity;
use authfs_aidl_interface::aidl::com::android::virt::fs::IVirtFdService::{
BnVirtFdService, FsStat::FsStat, IVirtFdService, MAX_REQUESTING_DATA,
};
diff --git a/authfs/fd_server/src/fsverity.rs b/authfs/fd_server/src/fsverity.rs
deleted file mode 100644
index 576f9dd..0000000
--- a/authfs/fd_server/src/fsverity.rs
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-use nix::ioctl_readwrite;
-use std::io;
-
-// Constants/values from uapi/linux/fsverity.h
-const FS_VERITY_METADATA_TYPE_MERKLE_TREE: u64 = 1;
-const FS_VERITY_METADATA_TYPE_SIGNATURE: u64 = 3;
-const FS_IOCTL_MAGIC: u8 = b'f';
-const FS_IOCTL_READ_VERITY_METADATA: u8 = 135;
-
-#[repr(C)]
-pub struct fsverity_read_metadata_arg {
- metadata_type: u64,
- offset: u64,
- length: u64,
- buf_ptr: u64,
- __reserved: u64,
-}
-
-ioctl_readwrite!(
- read_verity_metadata,
- FS_IOCTL_MAGIC,
- FS_IOCTL_READ_VERITY_METADATA,
- fsverity_read_metadata_arg
-);
-
-fn read_metadata(fd: i32, metadata_type: u64, offset: u64, buf: &mut [u8]) -> io::Result<usize> {
- let mut arg = fsverity_read_metadata_arg {
- metadata_type,
- offset,
- length: buf.len() as u64,
- buf_ptr: buf.as_mut_ptr() as u64,
- __reserved: 0,
- };
- Ok(unsafe { read_verity_metadata(fd, &mut arg) }? as usize)
-}
-
-/// Read the raw Merkle tree from the fd, if it exists. The API semantics is similar to a regular
-/// pread(2), and may not return full requested buffer.
-pub fn read_merkle_tree(fd: i32, offset: u64, buf: &mut [u8]) -> io::Result<usize> {
- read_metadata(fd, FS_VERITY_METADATA_TYPE_MERKLE_TREE, offset, buf)
-}
-
-/// Read the fs-verity signature from the fd (if exists). The returned signature should be complete.
-pub fn read_signature(fd: i32, buf: &mut [u8]) -> io::Result<usize> {
- read_metadata(fd, FS_VERITY_METADATA_TYPE_SIGNATURE, 0 /* offset */, buf)
-}
diff --git a/authfs/fd_server/src/main.rs b/authfs/fd_server/src/main.rs
index f91ebec..47983cb 100644
--- a/authfs/fd_server/src/main.rs
+++ b/authfs/fd_server/src/main.rs
@@ -23,7 +23,6 @@
//! client can then request the content of file 9 by offset and size.
mod aidl;
-mod fsverity;
use anyhow::{bail, Result};
use clap::Parser;
diff --git a/authfs/tests/common/src/java/com/android/fs/common/AuthFsTestRule.java b/authfs/tests/common/src/java/com/android/fs/common/AuthFsTestRule.java
index 357edea..7c85797 100644
--- a/authfs/tests/common/src/java/com/android/fs/common/AuthFsTestRule.java
+++ b/authfs/tests/common/src/java/com/android/fs/common/AuthFsTestRule.java
@@ -88,7 +88,7 @@
private static CommandRunner sAndroid;
private static CommandRunner sMicrodroid;
- private final ExecutorService mThreadPool = Executors.newCachedThreadPool();
+ private ExecutorService mThreadPool;
public static void setUpAndroid(TestInformation testInfo) throws Exception {
assertNotNull(testInfo.getDevice());
@@ -242,6 +242,7 @@
}
public void setUpTest() throws Exception {
+ mThreadPool = Executors.newCachedThreadPool();
if (sAndroid != null) {
sAndroid.run("mkdir -p " + TEST_OUTPUT_DIR);
}
@@ -264,5 +265,10 @@
archiveLogThenDelete(this, getDevice(), vmRecentLog, "vm_recent.log-" + testName);
sAndroid.run("rm -rf " + TEST_OUTPUT_DIR);
+
+ if (mThreadPool != null) {
+ mThreadPool.shutdownNow();
+ mThreadPool = null;
+ }
}
}
diff --git a/compos/composd/Android.bp b/compos/composd/Android.bp
index cee4b01..b0294dd 100644
--- a/compos/composd/Android.bp
+++ b/compos/composd/Android.bp
@@ -16,10 +16,13 @@
"libbinder_rs",
"libcompos_common",
"libcomposd_native_rust",
+ "libfsverity_rs",
"libminijail_rust",
"libnix",
"liblibc",
"liblog_rust",
+ "libodsign_proto_rust",
+ "libprotobuf",
"librustutils",
"libshared_child",
"libvmclient",
diff --git a/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl b/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl
index 569bba5..a3ce553 100644
--- a/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl
+++ b/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl
@@ -25,6 +25,8 @@
CompilationFailed,
/** We ran compilation in the VM, but it reported a problem. */
UnexpectedCompilationResult,
+ /** We failed to enable fs-verity completely to the output artifacts. */
+ FailedToEnableFsverity,
}
/**
diff --git a/compos/composd/src/instance_manager.rs b/compos/composd/src/instance_manager.rs
index 2db13c7..98d4a1b 100644
--- a/compos/composd/src/instance_manager.rs
+++ b/compos/composd/src/instance_manager.rs
@@ -27,7 +27,7 @@
use virtualizationservice::IVirtualizationService::IVirtualizationService;
// Enough memory to complete odrefresh in the VM.
-const VM_MEMORY_MIB: i32 = 1280;
+const VM_MEMORY_MIB: i32 = 1024;
pub struct InstanceManager {
service: Strong<dyn IVirtualizationService>,
diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs
index 3a699ab..a98f50d 100644
--- a/compos/composd/src/odrefresh_task.rs
+++ b/compos/composd/src/odrefresh_task.rs
@@ -28,11 +28,16 @@
CompilationMode::CompilationMode, ICompOsService, OdrefreshArgs::OdrefreshArgs,
};
use compos_common::odrefresh::{
- is_system_property_interesting, ExitCode, ODREFRESH_OUTPUT_ROOT_DIR,
+ is_system_property_interesting, ExitCode, CURRENT_ARTIFACTS_SUBDIR, ODREFRESH_OUTPUT_ROOT_DIR,
+ PENDING_ARTIFACTS_SUBDIR,
};
+use compos_common::BUILD_MANIFEST_SYSTEM_EXT_APK_PATH;
use log::{error, info, warn};
+use odsign_proto::odsign_info::OdsignInfo;
+use protobuf::Message;
use rustutils::system_properties;
-use std::fs::{remove_dir_all, OpenOptions};
+use std::fs::{remove_dir_all, File, OpenOptions};
+use std::os::fd::AsFd;
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::{AsRawFd, OwnedFd};
use std::path::Path;
@@ -103,8 +108,21 @@
let result = match exit_code {
Ok(ExitCode::CompilationSuccess) => {
- info!("CompilationSuccess");
- callback.onSuccess()
+ if compilation_mode == CompilationMode::TEST_COMPILE {
+ info!("Compilation success");
+ callback.onSuccess()
+ } else {
+ // compos.info is generated only during NORMAL_COMPILE
+ if let Err(e) = enable_fsverity_to_all() {
+ let message =
+ format!("Unexpected failure when enabling fs-verity: {:?}", e);
+ error!("{}", message);
+ callback.onFailure(FailureReason::FailedToEnableFsverity, &message)
+ } else {
+ info!("Compilation success, fs-verity enabled");
+ callback.onSuccess()
+ }
+ }
}
Ok(exit_code) => {
let message = format!("Unexpected odrefresh result: {:?}", exit_code);
@@ -161,13 +179,20 @@
let output_dir_raw_fd = output_dir_fd.as_raw_fd();
let staging_dir_raw_fd = staging_dir_fd.as_raw_fd();
- // Get the /system_ext FD differently because it may not exist.
- let (system_ext_dir_raw_fd, ro_dir_fds) =
- if let Ok(system_ext_dir_fd) = open_dir(Path::new("/system_ext")) {
- (system_ext_dir_fd.as_raw_fd(), vec![system_dir_fd, system_ext_dir_fd])
- } else {
- (-1, vec![system_dir_fd])
- };
+ // When the VM starts, it starts with or without mouting the extra build manifest APK from
+ // /system_ext. Later on request (here), we need to pass the directory FD of /system_ext, but
+ // only if the VM is configured to need it.
+ //
+ // It is possible to plumb the information from ComposClient to here, but it's extra complexity
+ // and feel slightly weird to encode the VM's state to the task itself, as it is a request to
+ // the VM.
+ let need_system_ext = Path::new(BUILD_MANIFEST_SYSTEM_EXT_APK_PATH).exists();
+ let (system_ext_dir_raw_fd, ro_dir_fds) = if need_system_ext {
+ let system_ext_dir_fd = open_dir(Path::new("/system_ext"))?;
+ (system_ext_dir_fd.as_raw_fd(), vec![system_dir_fd, system_ext_dir_fd])
+ } else {
+ (-1, vec![system_dir_fd])
+ };
// Spawn a fd_server to serve the FDs.
let fd_server_config = FdServerConfig {
@@ -197,6 +222,31 @@
ExitCode::from_i32(exit_code.into())
}
+/// Enable fs-verity to output artifacts according to compos.info in the pending directory. Any
+/// error before the completion will just abort, leaving the previous files enabled.
+fn enable_fsverity_to_all() -> Result<()> {
+ let odrefresh_current_dir = Path::new(ODREFRESH_OUTPUT_ROOT_DIR).join(CURRENT_ARTIFACTS_SUBDIR);
+ let pending_dir = Path::new(ODREFRESH_OUTPUT_ROOT_DIR).join(PENDING_ARTIFACTS_SUBDIR);
+ let mut reader =
+ File::open(&pending_dir.join("compos.info")).context("Failed to open compos.info")?;
+ let compos_info = OdsignInfo::parse_from_reader(&mut reader).context("Failed to parse")?;
+
+ for path_str in compos_info.file_hashes.keys() {
+ // Need to rebase the directory on to compos-pending first
+ if let Ok(relpath) = Path::new(path_str).strip_prefix(&odrefresh_current_dir) {
+ let path = pending_dir.join(relpath);
+ let file = File::open(&path).with_context(|| format!("Failed to open {:?}", path))?;
+ // We don't expect error. But when it happens, don't bother handle it here. For
+ // simplicity, just let odsign do the regular check.
+ fsverity::enable(file.as_fd())
+ .with_context(|| format!("Failed to enable fs-verity to {:?}", path))?;
+ } else {
+ warn!("Skip due to unexpected path: {}", path_str);
+ }
+ }
+ Ok(())
+}
+
/// Returns an `OwnedFD` of the directory.
fn open_dir(path: &Path) -> Result<OwnedFd> {
Ok(OwnedFd::from(
diff --git a/compos/service/java/com/android/server/compos/IsolatedCompilationJobService.java b/compos/service/java/com/android/server/compos/IsolatedCompilationJobService.java
index 479ae7f..933ac7a 100644
--- a/compos/service/java/com/android/server/compos/IsolatedCompilationJobService.java
+++ b/compos/service/java/com/android/server/compos/IsolatedCompilationJobService.java
@@ -234,6 +234,10 @@
result = IsolatedCompilationMetrics.RESULT_UNEXPECTED_COMPILATION_RESULT;
break;
+ case ICompilationTaskCallback.FailureReason.FailedToEnableFsverity:
+ result = IsolatedCompilationMetrics.RESULT_FAILED_TO_ENABLE_FSVERITY;
+ break;
+
default:
result = IsolatedCompilationMetrics.RESULT_UNKNOWN_FAILURE;
break;
diff --git a/compos/service/java/com/android/server/compos/IsolatedCompilationMetrics.java b/compos/service/java/com/android/server/compos/IsolatedCompilationMetrics.java
index e333198..f7799a4 100644
--- a/compos/service/java/com/android/server/compos/IsolatedCompilationMetrics.java
+++ b/compos/service/java/com/android/server/compos/IsolatedCompilationMetrics.java
@@ -36,9 +36,17 @@
// TODO(b/218525257): Move the definition of these enums to atoms.proto
@Retention(RetentionPolicy.SOURCE)
- @IntDef({RESULT_UNKNOWN, RESULT_SUCCESS, RESULT_UNKNOWN_FAILURE, RESULT_FAILED_TO_START,
- RESULT_JOB_CANCELED, RESULT_COMPILATION_FAILED, RESULT_UNEXPECTED_COMPILATION_RESULT,
- RESULT_COMPOSD_DIED})
+ @IntDef({
+ RESULT_UNKNOWN,
+ RESULT_SUCCESS,
+ RESULT_UNKNOWN_FAILURE,
+ RESULT_FAILED_TO_START,
+ RESULT_JOB_CANCELED,
+ RESULT_COMPILATION_FAILED,
+ RESULT_UNEXPECTED_COMPILATION_RESULT,
+ RESULT_COMPOSD_DIED,
+ RESULT_FAILED_TO_ENABLE_FSVERITY
+ })
public @interface CompilationResult {}
// Keep this in sync with Result enum in IsolatedCompilationEnded in
@@ -59,6 +67,9 @@
.ISOLATED_COMPILATION_ENDED__COMPILATION_RESULT__RESULT_UNEXPECTED_COMPILATION_RESULT;
public static final int RESULT_COMPOSD_DIED =
ArtStatsLog.ISOLATED_COMPILATION_ENDED__COMPILATION_RESULT__RESULT_COMPOSD_DIED;
+ public static final int RESULT_FAILED_TO_ENABLE_FSVERITY =
+ ArtStatsLog
+ .ISOLATED_COMPILATION_ENDED__COMPILATION_RESULT__RESULT_FAILED_TO_ENABLE_FSVERITY;
@Retention(RetentionPolicy.SOURCE)
@IntDef({SCHEDULING_RESULT_UNKNOWN, SCHEDULING_SUCCESS, SCHEDULING_FAILURE})
diff --git a/docs/debug/tracing.md b/docs/debug/tracing.md
index 7d7ea0c..facd9d0 100644
--- a/docs/debug/tracing.md
+++ b/docs/debug/tracing.md
@@ -16,11 +16,13 @@
* Only boot clock is supported, and there is no way for user space to change the tracing_clock.
* Hypervisor tracing periodically polls the data from the hypervisor, this is different from the
regular ftrace instance which pushes the events into the ring buffer.
+* Resetting ring buffers (by clearing the trace file) is only supported when there are no active
+ readers. If the trace file is cleared while there are active readers, then the ring buffers will
+ be cleared after the last reader disconnects.
+* Changing the size of the ring buffer while the tracing session is active is also not supported.
Note: the list above is not exhaustive.
-TODO(b/271412868): add more documentation on the user space interface.
-
### Perfetto integration
[Perfetto](https://perfetto.dev/docs/) is an open-source stack for performance instrumentation and
@@ -32,8 +34,15 @@
Consider first familiarizing yourself with Perfetto documentation for recording traces on Android:
https://perfetto.dev/docs/quickstart/android-tracing.
-So far it is only possible to capture hypervisor trace events by providing the full trace config
-file to Perfetto. Here is the minimal
+The [record_android_trace](
+https://cs.android.com/android/platform/superproject/+/master:external/perfetto/tools/record_android_trace)
+script supports a shortcut to capture all hypervisor events that are known to Perfetto:
+
+```shell
+external/perfetto/tools/record_android_trace hyp -t 15s -b 32mb -o /tmp/hyp.pftrace
+```
+
+Alternatively you can use full trace config to capture hypervisor. Example usage:
```shell
cat<<EOF>config.pbtx
@@ -66,10 +75,120 @@
#### Capturing hypervisor traces on QEMU
-TODO(b/271412868): fill in this section
+Perfetto supports capturing traces on Linux: https://perfetto.dev/docs/quickstart/linux-tracing.
+However, since pKVM hypervisor is only supported on arm64, you will need to cross-compile Perfetto
+binaries for linux-arm64 (unless you have an arm64 workstation).
-TODO(b/271412868): Stay tuned, more docs coming soon!
+1. Checkout Perfetto repository: https://perfetto.dev/docs/contributing/getting-started
+2. Follow https://perfetto.dev/docs/contributing/build-instructions#cross-compiling-for-linux-arm-64
+ to compile Perfetto binaries for arm64 architecture.
+3. Copy the tracebox binary to QEMU
+4. Run `tracebox` binary on QEMU to capture traces, it's interface is very similar to the
+`record_android_trace` binary. E.g. to capture all hypervisor events run:
+```shell
+tracebox -t 15s -b 32mb hyp
+```
+
+### Analysing traces using SQL
+
+On top of visualisation, Perfetto also provides a SQL interface to analyse traces. More
+documentation is available at https://perfetto.dev/docs/quickstart/trace-analysis and
+https://perfetto.dev/docs/analysis/trace-processor.
+
+Hypervisor events can be queried via `pkvm_hypervisor_events` SQL view. You can load that view by
+calling `SELECT IMPORT("pkvm.hypervisor");`, e.g.:
+
+```sql
+SELECT IMPORT("pkvm.hypervisor");
+SELECT * FROM pkvm_hypervisor_events limit 5;
+```
+
+Below are some SQL queries that might be useful when analysing hypervisor traces.
+
+**What is the longest time CPU spent in hypervisor, grouped by the reason to enter hypervisor**
+```sql
+SELECT IMPORT("pkvm.hypervisor");
+
+SELECT
+ cpu,
+ reason,
+ ts,
+ dur
+FROM pkvm_hypervisor_events
+JOIN (
+ SELECT
+ MAX(dur) as dur2,
+ cpu as cpu2,
+ reason as reason2
+ FROM pkvm_hypervisor_events
+ GROUP BY 2, 3) AS sc
+ON
+ cpu = sc.cpu2
+ AND dur = sc.dur2
+ AND (reason = sc.reason2 OR (reason IS NULL AND sc.reason2 IS NULL))
+ORDER BY dur desc;
+```
+
+**What are the 10 longest times CPU spent in hypervisor because of host_mem_abort**
+```sql
+SELECT
+ hyp.dur as dur,
+ hyp.ts as ts,
+ EXTRACT_ARG(slices.arg_set_id, 'esr') as esr,
+ EXTRACT_ARG(slices.arg_set_id, 'addr') as addr
+FROM pkvm_hypervisor_events as hyp
+JOIN slices
+ON hyp.slice_id = slices.id
+WHERE hyp.reason = 'host_mem_abort'
+ORDER BY dur desc
+LIMIT 10;
+```
## Microdroid VM tracing
+IMPORTANT: Tracing is only supported for debuggable Microdroid VMs.
+
+### Capturing trace in Microdroid
+
+Starting with Android U, Microdroid contains Perfetto tracing binaries, which makes it possible to
+capture traces inside Microdroid VM using Perfetto stack. The commands used to capture traces on
+Android should work for Microdroid VM as well, with a difference that Perfetto's tracing binaries
+are not enabled in Microdroid by default, so you need to manually start them by setting
+`persist.traced.enable` system property to `1`.
+
+Here is a quick example on how trace Microdroid VM:
+
+1. First start your VM. For this example we are going to use
+`adb shell /apex/com.android.virt/bin/vm run-microdroid`.
+
+2. Set up an adb connection with the running VM:
+```shell
+adb shell forward tcp:9876 vsock:${CID}:5555
+adb connect localhost:9876
+adb -s localhost:9876 root
+```
+Where `${CID}` corresponds to the running Microdroid VM that you want to establish adb connection
+with. List of running VMs can be obtained by running `adb shell /apex/com.android.virt/bin/vm list`.
+Alternatively you can use `vm_shell` utility to connect to a running VM, i.e.: `vm_shell connect`.
+
+3. Start Perfetto daemons and capture trace
+```shell
+adb -s localhost:9876 shell setprop persist.traced.enable 1
+${ANDROID_BULD_TOP}/external/perfetto/tools/record_android_trace \
+ -s localhost:9876 \
+ -o /tmp/microdroid-trace-file.pftrace \
+ -t 10s \
+ -b 32mb \
+ sched/sched_switch task/task_newtask sched/sched_process_exit
+```
+
+If you don't have Android repo checked out, then you can download the record_android_trace script by
+following the following [instructions](
+https://perfetto.dev/docs/quickstart/android-tracing#recording-a-trace-through-the-cmdline)
+
+More documentation on Perfetto's tracing on Android is available here:
+https://perfetto.dev/docs/quickstart/android-tracing
+
+### Capturing Microdroid boot trace
+
TODO(b/271412868): Stay tuned, more docs are coming soon!
diff --git a/libs/fdtpci/src/lib.rs b/libs/fdtpci/src/lib.rs
index e32e16d..96d98d6 100644
--- a/libs/fdtpci/src/lib.rs
+++ b/libs/fdtpci/src/lib.rs
@@ -197,24 +197,32 @@
Ok(memory_address..memory_address + memory_size)
}
+/// Encodes memory flags of a PCI range
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-struct PciMemoryFlags(u32);
+pub struct PciMemoryFlags(pub u32);
impl PciMemoryFlags {
+ /// Returns whether this PCI range is prefetchable
pub fn prefetchable(self) -> bool {
self.0 & 0x80000000 != 0
}
+ /// Returns the type of this PCI range
pub fn range_type(self) -> PciRangeType {
PciRangeType::from((self.0 & 0x3000000) >> 24)
}
}
+/// Type of a PCI range
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-enum PciRangeType {
+pub enum PciRangeType {
+ /// Range represents the PCI configuration space
ConfigurationSpace,
+ /// Range is on IO space
IoSpace,
+ /// Range is on 32-bit MMIO space
Memory32,
+ /// Range is on 64-bit MMIO space
Memory64,
}
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index a2a4138..de06d01 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -70,11 +70,8 @@
"libartpalette-system",
"apexd.microdroid",
- "atrace",
"debuggerd",
"linker",
- "tombstoned.microdroid",
- "tombstone_transmit.microdroid",
"cgroups.json",
"task_profiles.json",
"public.libraries.android.txt",
@@ -89,6 +86,12 @@
"libvm_payload", // used by payload to interact with microdroid manager
"prng_seeder_microdroid",
+
+ // Binaries required to capture traces in Microdroid.
+ "atrace",
+ "traced",
+ "traced_probes",
+ "perfetto",
] + microdroid_shell_and_utilities,
multilib: {
common: {
@@ -107,13 +110,26 @@
"authfs",
"authfs_service",
"encryptedstore",
- "microdroid_crashdump_kernel",
"microdroid_kexec",
"microdroid_manager",
"zipfuse",
],
},
},
+ arch: {
+ // b/273792258: These could be in multilib.lib64 except that
+ // microdroid_crashdump_kernel doesn't exist for riscv64 yet
+ arm64: {
+ deps: [
+ "microdroid_crashdump_kernel",
+ ],
+ },
+ x86_64: {
+ deps: [
+ "microdroid_crashdump_kernel",
+ ],
+ },
+ },
linker_config_src: "linker.config.json",
base_dir: "system",
dirs: microdroid_rootdirs,
diff --git a/microdroid/README.md b/microdroid/README.md
index 41278a5..28785fd 100644
--- a/microdroid/README.md
+++ b/microdroid/README.md
@@ -138,6 +138,7 @@
TEST_ROOT=/data/local/tmp/virt
adb shell /apex/com.android.virt/bin/vm run-app \
--log $TEST_ROOT/log.txt \
+--console $TEST_ROOT/console.txt \
PATH_TO_YOUR_APP \
$TEST_ROOT/MyApp.apk.idsig \
$TEST_ROOT/instance.img \
@@ -145,9 +146,9 @@
```
The last command lets you know the CID assigned to the VM. The console output
-from the VM is stored to `$TEST_ROOT/log.txt` file for debugging purpose. If you
-omit the `--log $TEST_ROOT/log.txt` option, it will be emitted to the current
-console.
+from the VM is stored to `$TEST_ROOT/console.txt` and logcat is stored to
+`$TEST_ROOT/log.txt` file for debugging purpose. If you omit `--log` or
+`--console` option, they will be emitted to the current console.
Stopping the VM can be done as follows:
@@ -159,12 +160,50 @@
invoked with the `--daemonize` flag. If the flag was not used, press Ctrl+C on
the console where the `vm run-app` command was invoked.
-## ADB
+## Debuggable microdroid
-On userdebug builds, you can have an adb connection to microdroid. To do so,
-first, delete `$TEST_ROOT/instance.img`; this is because changing debug settings
-requires a new instance. Then add the `--debug=full` flag to the
-`/apex/com.android.virt/bin/vm run-app` command, and then
+### Debugging features
+Microdroid supports following debugging features:
+
+- VM log
+- console output
+- kernel output
+- logcat output
+- [ramdump](../docs/debug/ramdump.md)
+- crashdump
+- [adb](#adb)
+- [gdb](#debugging-the-payload-on-microdroid)
+
+### Enabling debugging features
+There's two ways to enable the debugging features:
+
+#### Option 1) Running microdroid on AVF debug policy configured device
+
+microdroid can be started with debugging features by debug policies from the
+host. Host bootloader may provide debug policies to host OS's device tree for
+VMs.
+
+For protected VM, such device tree will be available in microdroid. microdroid
+can check which debuging features is enabled.
+
+Here are list of device tree properties for debugging features.
+
+- `/avf/guest/common/log`: `<1>` to enable kernel log and logcat. Ignored
+ otherwise.
+- `/avf/guest/common/ramdump`: `<1>` to enable ramdump. Ignored otherwise.
+- `/avf/guest/microdroid/adb`: `<1>` to enable `adb`. Ignored otherwise.
+
+#### Option 2) Lauching microdroid with debug level.
+
+microdroid can be started with debugging features. To do so, first, delete
+`$TEST_ROOT/instance.img`; this is because changing debug settings requires a
+new instance. Then add the `--debug=full` flag to the
+`/apex/com.android.virt/bin/vm run-app` command. This will enable all debugging
+features.
+
+### ADB
+
+If `adb` connection is enabled, launch following command.
```sh
vm_shell
@@ -175,7 +214,7 @@
Once you have an adb connection with `vm_shell`, `localhost:8000` will be the
serial of microdroid.
-## Debugging the payload on microdroid
+### Debugging the payload on microdroid
Like a normal adb device, you can debug native processes using `lldbclient.py`
script, either by running a new process, or attaching to an existing process.
diff --git a/microdroid/init.rc b/microdroid/init.rc
index 5187a12..29f8970 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -35,7 +35,10 @@
restorecon /mnt/extra-apk
# Wait for apexd to finish activating APEXes before starting more processes.
- wait_for_prop apexd.status activated
+ # Microdroid starts apexd in VM mode in which apexd doesn't wait for init after setting
+ # apexd.status to activated, but immediately transitions to ready. Therefore, it's not safe to
+ # wait for the activated status, by the time this line is reached it may be already be ready.
+ wait_for_prop apexd.status ready
perform_apex_config
# Notify to microdroid_manager that perform_apex_config is done.
@@ -123,14 +126,6 @@
mkdir /data/vendor_de 0771 root root
mkdir /data/vendor/hardware 0771 root root
- # Start tombstoned early to be able to store tombstones.
- # microdroid doesn't have anr, but tombstoned requires it
- mkdir /data/anr 0775 system system
- mkdir /data/tombstones 0771 system system
- mkdir /data/vendor/tombstones 0771 root root
-
- start tombstoned
-
# For security reasons, /data/local/tmp should always be empty.
# Do not place files or directories in /data/local/tmp
mkdir /data/local 0751 root root
@@ -143,15 +138,6 @@
# Mark boot completed. This will notify microdroid_manager to run payload.
setprop dev.bootcomplete 1
-on property:tombstone_transmit.start=1
- mkdir /data/tombstones 0771 system system
- start tombstone_transmit
-
-service tombstone_transmit /system/bin/tombstone_transmit.microdroid -cid 2 -port 2000 -remove_tombstones_after_transmitting
- user system
- group system
- shutdown critical
-
service apexd-vm /system/bin/apexd --vm
user root
group system
diff --git a/microdroid/init_debug_policy/Android.bp b/microdroid/init_debug_policy/Android.bp
index b56ef79..afc2e73 100644
--- a/microdroid/init_debug_policy/Android.bp
+++ b/microdroid/init_debug_policy/Android.bp
@@ -1,3 +1,7 @@
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
rust_binary {
name: "microdroid_init_debug_policy",
srcs: ["src/init_debug_policy.rs"],
diff --git a/microdroid/kdump/kexec.c b/microdroid/kdump/kexec.c
index 8d88951..d3e8e02 100644
--- a/microdroid/kdump/kexec.c
+++ b/microdroid/kdump/kexec.c
@@ -23,6 +23,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
@@ -53,6 +54,20 @@
if (syscall(SYS_kexec_file_load, open_checked(KERNEL), open_checked(INITRD), cmdline_len,
CMDLINE, KEXEC_FILE_ON_CRASH) == -1) {
fprintf(stderr, "Failed to load panic kernel: %s\n", strerror(errno));
+ if (errno == EADDRNOTAVAIL) {
+ struct stat st;
+ off_t kernel_size = 0;
+ off_t initrd_size = 0;
+
+ if (stat(KERNEL, &st) == 0) {
+ kernel_size = st.st_size;
+ }
+ if (stat(INITRD, &st) == 0) {
+ initrd_size = st.st_size;
+ }
+ fprintf(stderr, "Image size too big? %s:%ld bytes, %s:%ld bytes", KERNEL, kernel_size,
+ INITRD, initrd_size);
+ }
return 1;
}
return 0;
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index fa96bf4..ffa2e45 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -81,7 +81,6 @@
const ZIPFUSE_BIN: &str = "/system/bin/zipfuse";
const APEX_CONFIG_DONE_PROP: &str = "apex_config.done";
-const TOMBSTONE_TRANSMIT_DONE_PROP: &str = "tombstone_transmit.init_done";
const DEBUGGABLE_PROP: &str = "ro.boot.microdroid.debuggable";
// SYNC WITH virtualizationservice/src/crosvm.rs
@@ -423,12 +422,11 @@
setup_config_sysprops(&config)?;
- // Start tombstone_transmit if enabled
+ // Set export_tombstones if enabled
if should_export_tombstones(&config) {
- system_properties::write("tombstone_transmit.start", "1")
- .context("set tombstone_transmit.start")?;
- } else {
- control_service("stop", "tombstoned")?;
+ // This property is read by tombstone_handler.
+ system_properties::write("microdroid_manager.export_tombstones.enabled", "1")
+ .context("set microdroid_manager.export_tombstones.enabled")?;
}
// Wait until zipfuse has mounted the APKs so we can access the payload
@@ -448,20 +446,10 @@
system_properties::write("microdroid_manager.init_done", "1")
.context("set microdroid_manager.init_done")?;
- // Wait for tombstone_transmit to init
- if should_export_tombstones(&config) {
- wait_for_tombstone_transmit_done()?;
- }
-
info!("boot completed, time to run payload");
exec_task(task, service).context("Failed to run payload")
}
-fn control_service(action: &str, service: &str) -> Result<()> {
- system_properties::write(&format!("ctl.{}", action), service)
- .with_context(|| format!("Failed to {} {}", action, service))
-}
-
struct ApkDmverityArgument<'a> {
apk: &'a str,
idsig: &'a str,
@@ -733,11 +721,6 @@
wait_for_property_true(APEX_CONFIG_DONE_PROP).context("Failed waiting for apex config done")
}
-fn wait_for_tombstone_transmit_done() -> Result<()> {
- wait_for_property_true(TOMBSTONE_TRANSMIT_DONE_PROP)
- .context("Failed waiting for tombstone transmit done")
-}
-
fn wait_for_property_true(property_name: &str) -> Result<()> {
let mut prop = PropertyWatcher::new(property_name)?;
loop {
diff --git a/pvmfw/README.md b/pvmfw/README.md
index c652f01..04ad8c4 100644
--- a/pvmfw/README.md
+++ b/pvmfw/README.md
@@ -234,3 +234,31 @@
[dice-dt]: https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/google%2Copen-dice.yaml
[Layering]: https://pigweed.googlesource.com/open-dice/+/refs/heads/main/docs/specification.md#layering-details
[Trusty-BCC]: https://android.googlesource.com/trusty/lib/+/1696be0a8f3a7103/lib/hwbcc/common/swbcc.c#554
+
+#### pVM Device Tree Overlay
+
+Config header can provide a DTBO to be overlaid on top of the baseline device
+tree from crosvm.
+
+The DTBO may contain debug policies as follows.
+
+```
+/ {
+ fragment@avf {
+ target-path = "/";
+
+ __overlay__ {
+ avf {
+ /* your debug policy here */
+ };
+ };
+ };
+}; /* end of avf */
+```
+
+For specifying DTBO, host bootloader should apply the DTBO to both host
+OS's device tree and config header of `pvmfw`. Both `virtualizationmanager` and
+`pvmfw` will prepare for debugging features.
+
+For details about device tree properties for debug policies, see
+[microdroid's debugging policy guide](../microdroid/README.md#option-1-running-microdroid-on-avf-debug-policy-configured-device).
diff --git a/pvmfw/platform.dts b/pvmfw/platform.dts
index 056fa2f..127f69a 100644
--- a/pvmfw/platform.dts
+++ b/pvmfw/platform.dts
@@ -245,4 +245,11 @@
clock-names = "apb_pclk";
clocks = <&clk>;
};
+
+ vmwdt@3000 {
+ compatible = "qemu,vcpu-stall-detector";
+ reg = <0x00 0x3000 0x00 0x1000>;
+ clock-frequency = <10>;
+ timeout-sec = <8>;
+ };
};
diff --git a/pvmfw/src/crypto.rs b/pvmfw/src/crypto.rs
index 85dc6c9..275de7a 100644
--- a/pvmfw/src/crypto.rs
+++ b/pvmfw/src/crypto.rs
@@ -14,6 +14,8 @@
//! Wrapper around BoringSSL/OpenSSL symbols.
+use crate::cstr;
+
use core::convert::AsRef;
use core::ffi::{c_char, c_int, CStr};
use core::fmt;
@@ -81,14 +83,10 @@
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- let unknown_library = CStr::from_bytes_with_nul(b"{unknown library}\0").unwrap();
- let unknown_reason = CStr::from_bytes_with_nul(b"{unknown reason}\0").unwrap();
- let unknown_file = CStr::from_bytes_with_nul(b"??\0").unwrap();
-
let packed = self.packed_value();
- let library = self.library_name().unwrap_or(unknown_library).to_str().unwrap();
- let reason = self.reason().unwrap_or(unknown_reason).to_str().unwrap();
- let file = self.file.unwrap_or(unknown_file).to_str().unwrap();
+ let library = self.library_name().unwrap_or(cstr!("{unknown library}")).to_str().unwrap();
+ let reason = self.reason().unwrap_or(cstr!("{unknown reason}")).to_str().unwrap();
+ let file = self.file.unwrap_or(cstr!("??")).to_str().unwrap();
let line = self.line;
write!(f, "{file}:{line}: {library}: {reason} ({packed:#x})")
diff --git a/pvmfw/src/debug_policy.rs b/pvmfw/src/debug_policy.rs
index 23d3e1d..15efa1c 100644
--- a/pvmfw/src/debug_policy.rs
+++ b/pvmfw/src/debug_policy.rs
@@ -14,6 +14,7 @@
//! Support for the debug policy overlay in pvmfw
+use crate::cstr;
use alloc::{vec, vec::Vec};
use core::ffi::CStr;
use core::fmt;
@@ -65,11 +66,8 @@
/// Disables ramdump by removing crashkernel from bootargs in /chosen.
fn disable_ramdump(fdt: &mut libfdt::Fdt) -> Result<(), DebugPolicyError> {
- let chosen_path = CStr::from_bytes_with_nul(b"/chosen\0").unwrap();
- let bootargs_name = CStr::from_bytes_with_nul(b"bootargs\0").unwrap();
-
let chosen = match fdt
- .node(chosen_path)
+ .node(cstr!("/chosen"))
.map_err(|e| DebugPolicyError::Fdt("Failed to find /chosen", e))?
{
Some(node) => node,
@@ -77,7 +75,7 @@
};
let bootargs = match chosen
- .getprop_str(bootargs_name)
+ .getprop_str(cstr!("bootargs"))
.map_err(|e| DebugPolicyError::Fdt("Failed to find bootargs prop", e))?
{
Some(value) if !value.to_bytes().is_empty() => value,
@@ -100,8 +98,8 @@
new_bootargs.push(b'\0');
// We've checked existence of /chosen node at the beginning.
- let mut chosen_mut = fdt.node_mut(chosen_path).unwrap().unwrap();
- chosen_mut.setprop(bootargs_name, new_bootargs.as_slice()).map_err(|e| {
+ let mut chosen_mut = fdt.node_mut(cstr!("/chosen")).unwrap().unwrap();
+ chosen_mut.setprop(cstr!("bootargs"), new_bootargs.as_slice()).map_err(|e| {
DebugPolicyError::OverlaidFdt("Failed to remove crashkernel. FDT might be corrupted", e)
})
}
@@ -109,7 +107,7 @@
/// Returns true only if fdt has ramdump prop in the /avf/guest/common node with value <1>
fn is_ramdump_enabled(fdt: &libfdt::Fdt) -> Result<bool, DebugPolicyError> {
let common = match fdt
- .node(CStr::from_bytes_with_nul(b"/avf/guest/common\0").unwrap())
+ .node(cstr!("/avf/guest/common"))
.map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find /avf/guest/common node", e))?
{
Some(node) => node,
@@ -117,7 +115,7 @@
};
match common
- .getprop_u32(CStr::from_bytes_with_nul(b"ramdump\0").unwrap())
+ .getprop_u32(cstr!("ramdump"))
.map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find ramdump prop", e))?
{
Some(1) => Ok(true),
@@ -128,11 +126,8 @@
/// Enables console output by adding kernel.printk.devkmsg and kernel.console to bootargs.
/// This uses hardcoded console name 'hvc0' and it should be match with microdroid's bootconfig.debuggable.
fn enable_console_output(fdt: &mut libfdt::Fdt) -> Result<(), DebugPolicyError> {
- let chosen_path = CStr::from_bytes_with_nul(b"/chosen\0").unwrap();
- let bootargs_name = CStr::from_bytes_with_nul(b"bootargs\0").unwrap();
-
let chosen = match fdt
- .node(chosen_path)
+ .node(cstr!("/chosen"))
.map_err(|e| DebugPolicyError::Fdt("Failed to find /chosen", e))?
{
Some(node) => node,
@@ -140,7 +135,7 @@
};
let bootargs = match chosen
- .getprop_str(bootargs_name)
+ .getprop_str(cstr!("bootargs"))
.map_err(|e| DebugPolicyError::Fdt("Failed to find bootargs prop", e))?
{
Some(value) if !value.to_bytes().is_empty() => value,
@@ -154,8 +149,8 @@
fdt.unpack().map_err(|e| DebugPolicyError::OverlaidFdt("Failed to unpack", e))?;
// We've checked existence of /chosen node at the beginning.
- let mut chosen_mut = fdt.node_mut(chosen_path).unwrap().unwrap();
- chosen_mut.setprop(bootargs_name, new_bootargs.as_slice()).map_err(|e| {
+ let mut chosen_mut = fdt.node_mut(cstr!("/chosen")).unwrap().unwrap();
+ chosen_mut.setprop(cstr!("bootargs"), new_bootargs.as_slice()).map_err(|e| {
DebugPolicyError::OverlaidFdt("Failed to enabled console output. FDT might be corrupted", e)
})?;
@@ -166,7 +161,7 @@
/// Returns true only if fdt has log prop in the /avf/guest/common node with value <1>
fn is_console_output_enabled(fdt: &libfdt::Fdt) -> Result<bool, DebugPolicyError> {
let common = match fdt
- .node(CStr::from_bytes_with_nul(b"/avf/guest/common\0").unwrap())
+ .node(cstr!("/avf/guest/common"))
.map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find /avf/guest/common node", e))?
{
Some(node) => node,
@@ -174,7 +169,7 @@
};
match common
- .getprop_u32(CStr::from_bytes_with_nul(b"log\0").unwrap())
+ .getprop_u32(cstr!("log"))
.map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find log prop", e))?
{
Some(1) => Ok(true),
diff --git a/pvmfw/src/dice.rs b/pvmfw/src/dice.rs
index 3ceb8ef..bad3453 100644
--- a/pvmfw/src/dice.rs
+++ b/pvmfw/src/dice.rs
@@ -14,6 +14,7 @@
//! Support for DICE derivation and BCC generation.
+use crate::cstr;
use crate::helpers::flushed_zeroize;
use core::ffi::c_void;
use core::ffi::CStr;
@@ -60,10 +61,9 @@
self,
salt: &[u8; HIDDEN_SIZE],
) -> diced_open_dice::Result<InputValues> {
- let component_name = CStr::from_bytes_with_nul(b"vm_entry\0").unwrap();
let mut config_descriptor_buffer = [0; 128];
let config_descriptor_size = bcc_format_config_descriptor(
- Some(component_name),
+ Some(cstr!("vm_entry")),
None, // component_version
false, // resettable
&mut config_descriptor_buffer,
diff --git a/pvmfw/src/fdt.rs b/pvmfw/src/fdt.rs
index 793eaac..f56d6e0 100644
--- a/pvmfw/src/fdt.rs
+++ b/pvmfw/src/fdt.rs
@@ -14,18 +14,27 @@
//! High-level FDT functions.
+use crate::cstr;
+use crate::helpers::GUEST_PAGE_SIZE;
+use crate::RebootReason;
use core::ffi::CStr;
+use core::num::NonZeroUsize;
use core::ops::Range;
+use fdtpci::PciMemoryFlags;
+use fdtpci::PciRangeType;
+use libfdt::AddressRange;
+use libfdt::CellIterator;
use libfdt::Fdt;
use libfdt::FdtError;
+use log::error;
+use tinyvec::ArrayVec;
/// Extract from /config the address range containing the pre-loaded kernel.
pub fn kernel_range(fdt: &libfdt::Fdt) -> libfdt::Result<Option<Range<usize>>> {
- let config = CStr::from_bytes_with_nul(b"/config\0").unwrap();
- let addr = CStr::from_bytes_with_nul(b"kernel-address\0").unwrap();
- let size = CStr::from_bytes_with_nul(b"kernel-size\0").unwrap();
+ let addr = cstr!("kernel-address");
+ let size = cstr!("kernel-size");
- if let Some(config) = fdt.node(config)? {
+ if let Some(config) = fdt.node(cstr!("/config"))? {
if let (Some(addr), Some(size)) = (config.getprop_u32(addr)?, config.getprop_u32(size)?) {
let addr = addr as usize;
let size = size as usize;
@@ -39,8 +48,8 @@
/// Extract from /chosen the address range containing the pre-loaded ramdisk.
pub fn initrd_range(fdt: &libfdt::Fdt) -> libfdt::Result<Option<Range<usize>>> {
- let start = CStr::from_bytes_with_nul(b"linux,initrd-start\0").unwrap();
- let end = CStr::from_bytes_with_nul(b"linux,initrd-end\0").unwrap();
+ let start = cstr!("linux,initrd-start");
+ let end = cstr!("linux,initrd-end");
if let Some(chosen) = fdt.chosen()? {
if let (Some(start), Some(end)) = (chosen.getprop_u32(start)?, chosen.getprop_u32(end)?) {
@@ -51,6 +60,404 @@
Ok(None)
}
+/// Read and validate the size and base address of memory, and returns the size
+fn parse_memory_node(fdt: &libfdt::Fdt) -> Result<NonZeroUsize, RebootReason> {
+ let memory_range = fdt
+ .memory()
+ // Actually, these checks are unnecessary because we read /memory node in entry.rs
+ // where the exactly same checks are done. We are repeating the same check just for
+ // extra safety (in case when the code structure changes in the future).
+ .map_err(|e| {
+ error!("Failed to get /memory from the DT: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("Node /memory was found empty");
+ RebootReason::InvalidFdt
+ })?
+ .next()
+ .ok_or_else(|| {
+ error!("Failed to read memory range from the DT");
+ RebootReason::InvalidFdt
+ })?;
+
+ let base = memory_range.start;
+ if base as u64 != DeviceTreeInfo::RAM_BASE_ADDR {
+ error!("Memory base address {:#x} is not {:#x}", base, DeviceTreeInfo::RAM_BASE_ADDR);
+ return Err(RebootReason::InvalidFdt);
+ }
+
+ let size = memory_range.len(); // end is exclusive
+ if size % GUEST_PAGE_SIZE != 0 {
+ error!("Memory size {:#x} is not a multiple of page size {:#x}", size, GUEST_PAGE_SIZE);
+ return Err(RebootReason::InvalidFdt);
+ }
+ // In the u-boot implementation, we checked if base + size > u64::MAX, but we don't need that
+ // because memory() function uses checked_add when constructing the Range object. If an
+ // overflow happened, we should have gotten None from the next() call above and would have
+ // bailed already.
+
+ NonZeroUsize::new(size).ok_or_else(|| {
+ error!("Memory size can't be 0");
+ RebootReason::InvalidFdt
+ })
+}
+
+/// Read the number of CPUs
+fn parse_cpu_nodes(fdt: &libfdt::Fdt) -> Result<NonZeroUsize, RebootReason> {
+ let num = fdt
+ .compatible_nodes(cstr!("arm,arm-v8"))
+ .map_err(|e| {
+ error!("Failed to read compatible nodes \"arm,arm-v8\" from DT: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .count();
+ NonZeroUsize::new(num).ok_or_else(|| {
+ error!("Number of CPU can't be 0");
+ RebootReason::InvalidFdt
+ })
+}
+
+#[derive(Debug)]
+#[allow(dead_code)] // TODO: remove this
+struct PciInfo {
+ ranges: [Range<u64>; 2],
+ num_irq: usize,
+}
+
+/// Read and validate PCI node
+fn parse_pci_nodes(fdt: &libfdt::Fdt) -> Result<PciInfo, RebootReason> {
+ let node = fdt
+ .compatible_nodes(cstr!("pci-host-cam-generic"))
+ .map_err(|e| {
+ error!("Failed to read compatible node \"pci-host-cam-generic\" from DT: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .next()
+ .ok_or_else(|| {
+ // pvmfw requires at least one pci device (virtio-blk) for the instance disk. So,
+ // let's fail early.
+ error!("Compatible node \"pci-host-cam-generic\" doesn't exist");
+ RebootReason::InvalidFdt
+ })?;
+
+ let mut iter = node
+ .ranges::<(u32, u64), u64, u64>()
+ .map_err(|e| {
+ error!("Failed to read ranges from PCI node: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("PCI node missing ranges property");
+ RebootReason::InvalidFdt
+ })?;
+
+ let range0 = iter.next().ok_or_else(|| {
+ error!("First range missing in PCI node");
+ RebootReason::InvalidFdt
+ })?;
+ let range0 = get_and_validate_pci_range(&range0)?;
+
+ let range1 = iter.next().ok_or_else(|| {
+ error!("Second range missing in PCI node");
+ RebootReason::InvalidFdt
+ })?;
+ let range1 = get_and_validate_pci_range(&range1)?;
+
+ let num_irq = count_and_validate_pci_irq_masks(&node)?;
+
+ validate_pci_irq_maps(&node)?;
+
+ Ok(PciInfo { ranges: [range0, range1], num_irq })
+}
+
+fn get_and_validate_pci_range(
+ range: &AddressRange<(u32, u64), u64, u64>,
+) -> Result<Range<u64>, RebootReason> {
+ let mem_flags = PciMemoryFlags(range.addr.0);
+ let range_type = mem_flags.range_type();
+ let prefetchable = mem_flags.prefetchable();
+ let bus_addr = range.addr.1;
+ let cpu_addr = range.parent_addr;
+ let size = range.size;
+ if range_type != PciRangeType::Memory64 {
+ error!("Invalid range type {:?} for bus address {:#x} in PCI node", range_type, bus_addr);
+ return Err(RebootReason::InvalidFdt);
+ }
+ if prefetchable {
+ error!("PCI bus address {:#x} in PCI node is prefetchable", bus_addr);
+ return Err(RebootReason::InvalidFdt);
+ }
+ // Enforce ID bus-to-cpu mappings, as used by crosvm.
+ if bus_addr != cpu_addr {
+ error!("PCI bus address: {:#x} is different from CPU address: {:#x}", bus_addr, cpu_addr);
+ return Err(RebootReason::InvalidFdt);
+ }
+ let bus_end = bus_addr.checked_add(size).ok_or_else(|| {
+ error!("PCI address range size {:#x} too big", size);
+ RebootReason::InvalidFdt
+ })?;
+ Ok(bus_addr..bus_end)
+}
+
+/// Iterator that takes N cells as a chunk
+struct CellChunkIterator<'a, const N: usize> {
+ cells: CellIterator<'a>,
+}
+
+impl<'a, const N: usize> CellChunkIterator<'a, N> {
+ fn new(cells: CellIterator<'a>) -> Self {
+ Self { cells }
+ }
+}
+
+impl<'a, const N: usize> Iterator for CellChunkIterator<'a, N> {
+ type Item = [u32; N];
+ fn next(&mut self) -> Option<Self::Item> {
+ let mut ret: Self::Item = [0; N];
+ for i in ret.iter_mut() {
+ *i = self.cells.next()?;
+ }
+ Some(ret)
+ }
+}
+
+fn count_and_validate_pci_irq_masks(pci_node: &libfdt::FdtNode) -> Result<usize, RebootReason> {
+ const IRQ_MASK_CELLS: usize = 4;
+ const IRQ_MASK_ADDR_HI: u32 = 0xf800;
+ const IRQ_MASK_ADDR_ME: u32 = 0x0;
+ const IRQ_MASK_ADDR_LO: u32 = 0x0;
+ const IRQ_MASK_ANY_IRQ: u32 = 0x7;
+ const EXPECTED: [u32; IRQ_MASK_CELLS] =
+ [IRQ_MASK_ADDR_HI, IRQ_MASK_ADDR_ME, IRQ_MASK_ADDR_LO, IRQ_MASK_ANY_IRQ];
+
+ let mut irq_count: usize = 0;
+ for irq_mask in CellChunkIterator::<IRQ_MASK_CELLS>::new(
+ pci_node
+ .getprop_cells(cstr!("interrupt-map-mask"))
+ .map_err(|e| {
+ error!("Failed to read interrupt-map-mask property: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("PCI node missing interrupt-map-mask property");
+ RebootReason::InvalidFdt
+ })?,
+ ) {
+ if irq_mask != EXPECTED {
+ error!("invalid irq mask {:?}", irq_mask);
+ return Err(RebootReason::InvalidFdt);
+ }
+ irq_count += 1;
+ }
+ Ok(irq_count)
+}
+
+fn validate_pci_irq_maps(pci_node: &libfdt::FdtNode) -> Result<(), RebootReason> {
+ const IRQ_MAP_CELLS: usize = 10;
+ const PCI_DEVICE_IDX: usize = 11;
+ const PCI_IRQ_ADDR_ME: u32 = 0;
+ const PCI_IRQ_ADDR_LO: u32 = 0;
+ const PCI_IRQ_INTC: u32 = 1;
+ const AARCH64_IRQ_BASE: u32 = 4; // from external/crosvm/aarch64/src/lib.rs
+ const GIC_SPI: u32 = 0;
+ const IRQ_TYPE_LEVEL_HIGH: u32 = 4;
+
+ let mut phys_hi: u32 = 0;
+ let mut irq_nr = AARCH64_IRQ_BASE;
+
+ for irq_map in CellChunkIterator::<IRQ_MAP_CELLS>::new(
+ pci_node
+ .getprop_cells(cstr!("interrupt-map"))
+ .map_err(|e| {
+ error!("Failed to read interrupt-map property: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("PCI node missing interrupt-map property");
+ RebootReason::InvalidFdt
+ })?,
+ ) {
+ phys_hi += 0x1 << PCI_DEVICE_IDX;
+
+ let pci_addr = (irq_map[0], irq_map[1], irq_map[2]);
+ let pci_irq_number = irq_map[3];
+ let _controller_phandle = irq_map[4]; // skipped.
+ let gic_addr = (irq_map[5], irq_map[6]); // address-cells is <2> for GIC
+ // interrupt-cells is <3> for GIC
+ let gic_peripheral_interrupt_type = irq_map[7];
+ let gic_irq_number = irq_map[8];
+ let gic_irq_type = irq_map[9];
+
+ let expected_pci_addr = (phys_hi, PCI_IRQ_ADDR_ME, PCI_IRQ_ADDR_LO);
+
+ if pci_addr != expected_pci_addr {
+ error!("PCI device address {:#x} {:#x} {:#x} in interrupt-map is different from expected address \
+ {:#x} {:#x} {:#x}",
+ pci_addr.0, pci_addr.1, pci_addr.2, expected_pci_addr.0, expected_pci_addr.1, expected_pci_addr.2);
+ return Err(RebootReason::InvalidFdt);
+ }
+ if pci_irq_number != PCI_IRQ_INTC {
+ error!(
+ "PCI INT# {:#x} in interrupt-map is different from expected value {:#x}",
+ pci_irq_number, PCI_IRQ_INTC
+ );
+ return Err(RebootReason::InvalidFdt);
+ }
+ if gic_addr != (0, 0) {
+ error!(
+ "GIC address {:#x} {:#x} in interrupt-map is different from expected address \
+ {:#x} {:#x}",
+ gic_addr.0, gic_addr.1, 0, 0
+ );
+ return Err(RebootReason::InvalidFdt);
+ }
+ if gic_peripheral_interrupt_type != GIC_SPI {
+ error!("GIC peripheral interrupt type {:#x} in interrupt-map is different from expected value \
+ {:#x}", gic_peripheral_interrupt_type, GIC_SPI);
+ return Err(RebootReason::InvalidFdt);
+ }
+ if gic_irq_number != irq_nr {
+ error!(
+ "GIC irq number {:#x} in interrupt-map is unexpected. Expected {:#x}",
+ gic_irq_number, irq_nr
+ );
+ return Err(RebootReason::InvalidFdt);
+ }
+ irq_nr += 1; // move to next irq
+ if gic_irq_type != IRQ_TYPE_LEVEL_HIGH {
+ error!(
+ "IRQ type in {:#x} is invalid. Must be LEVEL_HIGH {:#x}",
+ gic_irq_type, IRQ_TYPE_LEVEL_HIGH
+ );
+ return Err(RebootReason::InvalidFdt);
+ }
+ }
+ Ok(())
+}
+
+#[derive(Default, Debug)]
+#[allow(dead_code)] // TODO: remove this
+pub struct SerialInfo {
+ addrs: ArrayVec<[u64; Self::SERIAL_MAX_COUNT]>,
+}
+
+impl SerialInfo {
+ const SERIAL_MAX_COUNT: usize = 4;
+}
+
+fn parse_serial_nodes(fdt: &libfdt::Fdt) -> Result<SerialInfo, RebootReason> {
+ let mut ret: SerialInfo = Default::default();
+ for (i, node) in fdt
+ .compatible_nodes(cstr!("ns16550a"))
+ .map_err(|e| {
+ error!("Failed to read compatible nodes \"ns16550a\" from DT: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .enumerate()
+ {
+ if i >= ret.addrs.capacity() {
+ error!("Too many serials: {i}");
+ return Err(RebootReason::InvalidFdt);
+ }
+ let reg = node
+ .reg()
+ .map_err(|e| {
+ error!("Failed to read reg property from \"ns16550a\" node: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("No reg property in \"ns16550a\" node");
+ RebootReason::InvalidFdt
+ })?
+ .next()
+ .ok_or_else(|| {
+ error!("No value in reg property of \"ns16550a\" node");
+ RebootReason::InvalidFdt
+ })?;
+ ret.addrs.push(reg.addr);
+ }
+ Ok(ret)
+}
+
+#[derive(Debug)]
+#[allow(dead_code)] // TODO: remove this
+pub struct SwiotlbInfo {
+ size: u64,
+ align: u64,
+}
+
+fn parse_swiotlb_nodes(fdt: &libfdt::Fdt) -> Result<SwiotlbInfo, RebootReason> {
+ let node = fdt
+ .compatible_nodes(cstr!("restricted-dma-pool"))
+ .map_err(|e| {
+ error!("Failed to read compatible nodes \"restricted-dma-pool\" from DT: {e}");
+ RebootReason::InvalidFdt
+ })?
+ .next()
+ .ok_or_else(|| {
+ error!("No compatible node \"restricted-dma-pool\" in DT");
+ RebootReason::InvalidFdt
+ })?;
+ let size = node
+ .getprop_u64(cstr!("size"))
+ .map_err(|e| {
+ error!("Failed to read \"size\" property of \"restricted-dma-pool\": {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("No \"size\" property in \"restricted-dma-pool\"");
+ RebootReason::InvalidFdt
+ })?;
+
+ let align = node
+ .getprop_u64(cstr!("alignment"))
+ .map_err(|e| {
+ error!("Failed to read \"alignment\" property of \"restricted-dma-pool\": {e}");
+ RebootReason::InvalidFdt
+ })?
+ .ok_or_else(|| {
+ error!("No \"alignment\" property in \"restricted-dma-pool\"");
+ RebootReason::InvalidFdt
+ })?;
+
+ if size == 0 || (size % GUEST_PAGE_SIZE as u64) != 0 {
+ error!("Invalid swiotlb size {:#x}", size);
+ return Err(RebootReason::InvalidFdt);
+ }
+
+ if (align % GUEST_PAGE_SIZE as u64) != 0 {
+ error!("Invalid swiotlb alignment {:#x}", align);
+ return Err(RebootReason::InvalidFdt);
+ }
+
+ Ok(SwiotlbInfo { size, align })
+}
+
+#[derive(Debug)]
+#[allow(dead_code)] // TODO: remove this
+pub struct DeviceTreeInfo {
+ memory_size: NonZeroUsize,
+ num_cpu: NonZeroUsize,
+ pci_info: PciInfo,
+ serial_info: SerialInfo,
+ swiotlb_info: SwiotlbInfo,
+}
+
+impl DeviceTreeInfo {
+ const RAM_BASE_ADDR: u64 = 0x8000_0000;
+}
+
+pub fn parse_device_tree(fdt: &libfdt::Fdt) -> Result<DeviceTreeInfo, RebootReason> {
+ Ok(DeviceTreeInfo {
+ memory_size: parse_memory_node(fdt)?,
+ num_cpu: parse_cpu_nodes(fdt)?,
+ pci_info: parse_pci_nodes(fdt)?,
+ serial_info: parse_serial_nodes(fdt)?,
+ swiotlb_info: parse_swiotlb_nodes(fdt)?,
+ })
+}
+
/// Modifies the input DT according to the fields of the configuration.
pub fn modify_for_next_stage(
fdt: &mut Fdt,
@@ -62,16 +469,8 @@
add_dice_node(fdt, bcc.as_ptr() as usize, bcc.len())?;
- set_or_clear_chosen_flag(
- fdt,
- CStr::from_bytes_with_nul(b"avf,strict-boot\0").unwrap(),
- strict_boot,
- )?;
- set_or_clear_chosen_flag(
- fdt,
- CStr::from_bytes_with_nul(b"avf,new-instance\0").unwrap(),
- new_instance,
- )?;
+ set_or_clear_chosen_flag(fdt, cstr!("avf,strict-boot"), strict_boot)?;
+ set_or_clear_chosen_flag(fdt, cstr!("avf,new-instance"), new_instance)?;
fdt.pack()?;
@@ -80,24 +479,20 @@
/// Add a "google,open-dice"-compatible reserved-memory node to the tree.
fn add_dice_node(fdt: &mut Fdt, addr: usize, size: usize) -> libfdt::Result<()> {
- let reserved_memory = CStr::from_bytes_with_nul(b"/reserved-memory\0").unwrap();
// We reject DTs with missing reserved-memory node as validation should have checked that the
// "swiotlb" subnode (compatible = "restricted-dma-pool") was present.
- let mut reserved_memory = fdt.node_mut(reserved_memory)?.ok_or(libfdt::FdtError::NotFound)?;
+ let mut reserved_memory =
+ fdt.node_mut(cstr!("/reserved-memory"))?.ok_or(libfdt::FdtError::NotFound)?;
- let dice = CStr::from_bytes_with_nul(b"dice\0").unwrap();
- let mut dice = reserved_memory.add_subnode(dice)?;
+ let mut dice = reserved_memory.add_subnode(cstr!("dice"))?;
- let compatible = CStr::from_bytes_with_nul(b"compatible\0").unwrap();
- dice.appendprop(compatible, b"google,open-dice\0")?;
+ dice.appendprop(cstr!("compatible"), b"google,open-dice\0")?;
- let no_map = CStr::from_bytes_with_nul(b"no-map\0").unwrap();
- dice.appendprop(no_map, &[])?;
+ dice.appendprop(cstr!("no-map"), &[])?;
let addr = addr.try_into().unwrap();
let size = size.try_into().unwrap();
- let reg = CStr::from_bytes_with_nul(b"reg\0").unwrap();
- dice.appendprop_addrrange(reg, addr, size)?;
+ dice.appendprop_addrrange(cstr!("reg"), addr, size)?;
Ok(())
}
diff --git a/pvmfw/src/heap.rs b/pvmfw/src/heap.rs
index 435a6ff..eea2e98 100644
--- a/pvmfw/src/heap.rs
+++ b/pvmfw/src/heap.rs
@@ -53,7 +53,15 @@
#[no_mangle]
unsafe extern "C" fn malloc(size: usize) -> *mut c_void {
- malloc_(size).map_or(ptr::null_mut(), |p| p.cast::<c_void>().as_ptr())
+ malloc_(size, false).map_or(ptr::null_mut(), |p| p.cast::<c_void>().as_ptr())
+}
+
+#[no_mangle]
+unsafe extern "C" fn calloc(nmemb: usize, size: usize) -> *mut c_void {
+ let Some(size) = nmemb.checked_mul(size) else {
+ return ptr::null_mut()
+ };
+ malloc_(size, true).map_or(ptr::null_mut(), |p| p.cast::<c_void>().as_ptr())
}
#[no_mangle]
@@ -67,9 +75,11 @@
}
}
-unsafe fn malloc_(size: usize) -> Option<NonNull<usize>> {
+unsafe fn malloc_(size: usize, zeroed: bool) -> Option<NonNull<usize>> {
let size = NonZeroUsize::new(size)?.checked_add(mem::size_of::<usize>())?;
- let ptr = HEAP_ALLOCATOR.alloc(malloc_layout(size)?);
+ let layout = malloc_layout(size)?;
+ let ptr =
+ if zeroed { HEAP_ALLOCATOR.alloc_zeroed(layout) } else { HEAP_ALLOCATOR.alloc(layout) };
let ptr = NonNull::new(ptr)?.cast::<usize>().as_ptr();
*ptr = size.get();
NonNull::new(ptr.offset(1))
diff --git a/pvmfw/src/helpers.rs b/pvmfw/src/helpers.rs
index e6e3406..fddd8c3 100644
--- a/pvmfw/src/helpers.rs
+++ b/pvmfw/src/helpers.rs
@@ -113,3 +113,11 @@
reg.zeroize();
flush(reg)
}
+
+/// Create &CStr out of &str literal
+#[macro_export]
+macro_rules! cstr {
+ ($str:literal) => {{
+ CStr::from_bytes_with_nul(concat!($str, "\0").as_bytes()).unwrap()
+ }};
+}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 577ad6e..e1ecac4 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -45,6 +45,7 @@
use crate::dice::PartialInputs;
use crate::entry::RebootReason;
use crate::fdt::modify_for_next_stage;
+use crate::fdt::parse_device_tree;
use crate::helpers::flush;
use crate::helpers::GUEST_PAGE_SIZE;
use crate::instance::get_or_generate_instance_salt;
@@ -83,6 +84,11 @@
})?;
trace!("BCC: {bcc_handover:x?}");
+ // This parsing step includes validation. So this effectively ensures that the DT can't be
+ // abused by the host to attack pvmfw in pci::initialize below.
+ let device_tree_info = parse_device_tree(fdt)?;
+ debug!("Device tree info: {:?}", device_tree_info);
+
// Set up PCI bus for VirtIO devices.
let pci_info = PciInfo::from_fdt(fdt).map_err(handle_pci_error)?;
debug!("PCI: {:#x?}", pci_info);
diff --git a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
index 20a6045..a7f7906 100644
--- a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
+++ b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
@@ -67,6 +67,8 @@
// remove any leftover files under test root
android.tryRun("rm", "-rf", TEST_ROOT + "*");
+
+ android.tryRun("mkdir " + TEST_ROOT);
}
public static void cleanUpVirtualizationTestSetup(ITestDevice androidDevice)
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index e015d9d..749d75f 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -725,10 +725,11 @@
/// user devices (W^X).
fn check_label_is_allowed(context: &SeContext) -> Result<()> {
match context.selinux_type()? {
- | "system_file" // immutable dm-verity protected partition
| "apk_data_file" // APKs of an installed app
- | "staging_data_file" // updated/staged APEX images
| "shell_data_file" // test files created via adb shell
+ | "staging_data_file" // updated/staged APEX images
+ | "system_file" // immutable dm-verity protected partition
+ | "virtualizationservice_data_file" // files created by VS / VirtMgr
=> Ok(()),
_ => bail!("Label {} is not allowed", context),
}
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 9db0971..19f5f01 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -53,6 +53,7 @@
use rpcbinder::RpcServer;
/// external/crosvm
+use base::AsRawDescriptor;
use base::UnixSeqpacketListener;
use vm_control::{BalloonControlCommand, VmRequest, VmResponse};
@@ -823,7 +824,9 @@
let control_server_socket = UnixSeqpacketListener::bind(crosvm_control_socket_path)
.context("failed to create control server")?;
- command.arg("--socket").arg(add_preserved_fd(&mut preserved_fds, &control_server_socket));
+ command
+ .arg("--socket")
+ .arg(add_preserved_fd(&mut preserved_fds, &control_server_socket.as_raw_descriptor()));
debug!("Preserving FDs {:?}", preserved_fds);
command.preserved_fds(preserved_fds);
diff --git a/vmbase/README.md b/vmbase/README.md
index 3554ae6..552ac31 100644
--- a/vmbase/README.md
+++ b/vmbase/README.md
@@ -25,28 +25,18 @@
```soong
rust_ffi_static {
name: "libvmbase_example",
+ defaults: ["vmbase_ffi_defaults"],
crate_name: "vmbase_example",
srcs: ["src/main.rs"],
- edition: "2021",
- no_stdlibs: true,
- stdlibs: [
- "libcompiler_builtins.rust_sysroot",
- "libcore.rust_sysroot",
- ],
rustlibs: [
"libvmbase",
],
- enabled: false,
- target: {
- android_arm64: {
- enabled: true,
- },
- },
}
```
-Note that stdlibs must be explicitly specified, as we don't want the normal set of libraries used
-for a C++ binary intended to run in Android userspace.
+`vmbase_ffi_defaults`, among other things, specifies the stdlibs including the `compiler_builtins`
+and `core` crate. These must be explicitly specified as we don't want the normal set of libraries
+used for a C++ binary intended to run in Android userspace.
### Entry point
@@ -139,30 +129,18 @@
```soong
cc_binary {
- name: "vmbase_example_elf",
- stem: "vmbase_example",
+ name: "vmbase_example",
+ defaults: ["vmbase_elf_defaults"],
srcs: [
"idmap.S",
],
static_libs: [
- "libvmbase_entry",
"libvmbase_example",
],
- static_executable: true,
- nocrt: true,
- system_shared_libs: ["libc"],
- stl: "none",
linker_scripts: [
"image.ld",
":vmbase_sections",
],
- installable: false,
- enabled: false,
- target: {
- android_arm64: {
- enabled: true,
- },
- },
}
```
@@ -174,9 +152,9 @@
```soong
raw_binary {
- name: "vmbase_example",
- src: ":vmbase_example_elf",
+ name: "vmbase_example_bin",
stem: "vmbase_example.bin",
+ src: ":vmbase_example",
enabled: false,
target: {
android_arm64: {