Merge "Update doc about rebuilding kernel for Microdroid"
diff --git a/compos/common/timeouts.rs b/compos/common/timeouts.rs
index b3ec1e5..bdabb1e 100644
--- a/compos/common/timeouts.rs
+++ b/compos/common/timeouts.rs
@@ -26,8 +26,6 @@
pub struct Timeouts {
/// Total time that odrefresh may take to perform compilation
pub odrefresh_max_execution_time: Duration,
- /// Time allowed for a single compilation step run by odrefresh
- pub odrefresh_max_child_process_time: Duration,
/// Time allowed for the CompOS VM to start up and become ready.
pub vm_max_time_to_ready: Duration,
}
@@ -55,13 +53,11 @@
pub const NORMAL_TIMEOUTS: Timeouts = Timeouts {
// Note: the source of truth for these odrefresh timeouts is art/odrefresh/odr_config.h.
odrefresh_max_execution_time: Duration::from_secs(300),
- odrefresh_max_child_process_time: Duration::from_secs(90),
- vm_max_time_to_ready: Duration::from_secs(20),
+ vm_max_time_to_ready: Duration::from_secs(15),
};
/// The timeouts that we use when need_extra_time() returns true.
pub const EXTENDED_TIMEOUTS: Timeouts = Timeouts {
odrefresh_max_execution_time: Duration::from_secs(480),
- odrefresh_max_child_process_time: Duration::from_secs(150),
vm_max_time_to_ready: Duration::from_secs(120),
};
diff --git a/compos/verify/verify.rs b/compos/verify/verify.rs
index 4dc9954..7b77c18 100644
--- a/compos/verify/verify.rs
+++ b/compos/verify/verify.rs
@@ -22,7 +22,8 @@
use compos_aidl_interface::binder::ProcessState;
use compos_common::compos_client::{VmInstance, VmParameters};
use compos_common::odrefresh::{
- CURRENT_ARTIFACTS_SUBDIR, ODREFRESH_OUTPUT_ROOT_DIR, TEST_ARTIFACTS_SUBDIR,
+ CURRENT_ARTIFACTS_SUBDIR, ODREFRESH_OUTPUT_ROOT_DIR, PENDING_ARTIFACTS_SUBDIR,
+ TEST_ARTIFACTS_SUBDIR,
};
use compos_common::{
COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR, IDSIG_FILE, IDSIG_MANIFEST_APK_FILE,
@@ -62,7 +63,7 @@
.long("instance")
.takes_value(true)
.required(true)
- .possible_values(&["current", "test"]),
+ .possible_values(&["current", "pending", "test"]),
)
.arg(clap::Arg::with_name("debug").long("debug"))
.get_matches();
@@ -70,6 +71,7 @@
let debug_mode = matches.is_present("debug");
let (instance_dir, artifacts_dir) = match matches.value_of("instance").unwrap() {
"current" => (CURRENT_INSTANCE_DIR, CURRENT_ARTIFACTS_SUBDIR),
+ "pending" => (CURRENT_INSTANCE_DIR, PENDING_ARTIFACTS_SUBDIR),
"test" => (TEST_INSTANCE_DIR, TEST_ARTIFACTS_SUBDIR),
_ => unreachable!("Unexpected instance name"),
};
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 65ce7ea..ed2c2a1 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -36,6 +36,8 @@
import android.system.virtualizationservice.VirtualMachineState;
import android.util.JsonReader;
+import com.android.internal.annotations.GuardedBy;
+
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
@@ -52,6 +54,8 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
import java.util.zip.ZipFile;
/**
@@ -92,6 +96,9 @@
DELETED,
}
+ /** Lock for internal synchronization. */
+ private final Object mLock = new Object();
+
/** The package which owns this VM. */
private final @NonNull String mPackageName;
@@ -135,9 +142,11 @@
private @Nullable IVirtualMachine mVirtualMachine;
/** The registered callback */
+ @GuardedBy("mLock")
private @Nullable VirtualMachineCallback mCallback;
/** The executor on which the callback will be executed */
+ @GuardedBy("mLock")
private @Nullable Executor mCallbackExecutor;
private @Nullable ParcelFileDescriptor mConsoleReader;
@@ -299,20 +308,37 @@
public void setCallback(
@NonNull @CallbackExecutor Executor executor,
@NonNull VirtualMachineCallback callback) {
- mCallbackExecutor = executor;
- mCallback = callback;
+ synchronized (mLock) {
+ mCallback = callback;
+ mCallbackExecutor = executor;
+ }
}
/** Clears the currently registered callback. */
public void clearCallback() {
- // TODO(b/220730550): synchronize with the callers of the callback
- mCallback = null;
- mCallbackExecutor = null;
+ synchronized (mLock) {
+ mCallback = null;
+ mCallbackExecutor = null;
+ }
}
- /** Returns the currently registered callback. */
- public @Nullable VirtualMachineCallback getCallback() {
- return mCallback;
+ /** Executes a callback on the callback executor. */
+ private void executeCallback(Consumer<VirtualMachineCallback> fn) {
+ final VirtualMachineCallback callback;
+ final Executor executor;
+ synchronized (mLock) {
+ callback = mCallback;
+ executor = mCallbackExecutor;
+ }
+ if (callback == null || executor == null) {
+ return;
+ }
+ final long restoreToken = Binder.clearCallingIdentity();
+ try {
+ executor.execute(() -> fn.accept(callback));
+ } finally {
+ Binder.restoreCallingIdentity(restoreToken);
+ }
}
/**
@@ -376,14 +402,15 @@
android.system.virtualizationservice.VirtualMachineConfig vmConfigParcel =
android.system.virtualizationservice.VirtualMachineConfig.appConfig(appConfig);
+ // The VM should only be observed to die once
+ AtomicBoolean onDiedCalled = new AtomicBoolean(false);
+
IBinder.DeathRecipient deathRecipient = new IBinder.DeathRecipient() {
@Override
public void binderDied() {
- final VirtualMachineCallback cb = mCallback;
- if (cb != null) {
- // TODO(b/220730550): don't call if the VM already died
- cb.onDied(VirtualMachine.this, VirtualMachineCallback
- .DEATH_REASON_VIRTUALIZATIONSERVICE_DIED);
+ if (onDiedCalled.compareAndSet(false, true)) {
+ executeCallback((cb) -> cb.onDied(VirtualMachine.this,
+ VirtualMachineCallback.DEATH_REASON_VIRTUALIZATIONSERVICE_DIED));
}
}
};
@@ -393,80 +420,32 @@
new IVirtualMachineCallback.Stub() {
@Override
public void onPayloadStarted(int cid, ParcelFileDescriptor stream) {
- final VirtualMachineCallback cb = mCallback;
- if (cb == null) {
- return;
- }
- final long restoreToken = Binder.clearCallingIdentity();
- try {
- mCallbackExecutor.execute(
- () -> cb.onPayloadStarted(VirtualMachine.this, stream));
- } finally {
- Binder.restoreCallingIdentity(restoreToken);
- }
+ executeCallback(
+ (cb) -> cb.onPayloadStarted(VirtualMachine.this, stream));
}
-
@Override
public void onPayloadReady(int cid) {
- final VirtualMachineCallback cb = mCallback;
- if (cb == null) {
- return;
- }
- final long restoreToken = Binder.clearCallingIdentity();
- try {
- mCallbackExecutor.execute(
- () -> cb.onPayloadReady(VirtualMachine.this));
- } finally {
- Binder.restoreCallingIdentity(restoreToken);
- }
+ executeCallback((cb) -> cb.onPayloadReady(VirtualMachine.this));
}
-
@Override
public void onPayloadFinished(int cid, int exitCode) {
- final VirtualMachineCallback cb = mCallback;
- if (cb == null) {
- return;
- }
- final long restoreToken = Binder.clearCallingIdentity();
- try {
- mCallbackExecutor.execute(
- () -> cb.onPayloadFinished(VirtualMachine.this, exitCode));
- } finally {
- Binder.restoreCallingIdentity(restoreToken);
- }
+ executeCallback(
+ (cb) -> cb.onPayloadFinished(VirtualMachine.this, exitCode));
}
-
@Override
public void onError(int cid, int errorCode, String message) {
- final VirtualMachineCallback cb = mCallback;
- if (cb == null) {
- return;
- }
- final long restoreToken = Binder.clearCallingIdentity();
- try {
- mCallbackExecutor.execute(
- () -> cb.onError(VirtualMachine.this, errorCode, message));
- } finally {
- Binder.restoreCallingIdentity(restoreToken);
- }
+ executeCallback(
+ (cb) -> cb.onError(VirtualMachine.this, errorCode, message));
}
-
@Override
public void onDied(int cid, int reason) {
service.asBinder().unlinkToDeath(deathRecipient, 0);
- final VirtualMachineCallback cb = mCallback;
- if (cb == null) {
- return;
- }
- final long restoreToken = Binder.clearCallingIdentity();
- try {
- mCallbackExecutor.execute(
- () -> cb.onDied(VirtualMachine.this, reason));
- } finally {
- Binder.restoreCallingIdentity(restoreToken);
+ if (onDiedCalled.compareAndSet(false, true)) {
+ executeCallback((cb) -> cb.onDied(VirtualMachine.this, reason));
}
}
- });
+ }
+ );
service.asBinder().linkToDeath(deathRecipient, 0);
mVirtualMachine.start();
} catch (IOException e) {
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index 3ae4b1a..0ca7036 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -59,7 +59,6 @@
"libstdc++",
"logcat",
"logd",
- "run-as",
"secilc",
// "com.android.adbd" requires these,
diff --git a/microdroid/bootconfig.normal b/microdroid/bootconfig.normal
index 708d64b..ea83287 100644
--- a/microdroid/bootconfig.normal
+++ b/microdroid/bootconfig.normal
@@ -7,7 +7,8 @@
# Console output is not redirect to the host-side.
# TODO(b/219743539) This doesn't successfully disable the console
kernel.printk.devkmsg=off
-kernel.console=null
+# TODO(b/219743539) Setting this to null makes everything slow
+kernel.console=hvc0
# ADB is not enabled.
androidboot.adb.enabled=0
diff --git a/microdroid/dice/service.rs b/microdroid/dice/service.rs
index 8cb5cc3..8199c7c 100644
--- a/microdroid/dice/service.rs
+++ b/microdroid/dice/service.rs
@@ -30,6 +30,7 @@
use std::slice;
use std::sync::Arc;
+const AVF_STRICT_BOOT: &str = "/sys/firmware/devicetree/base/chosen/avf,strict-boot";
const DICE_HAL_SERVICE_NAME: &str = "android.hardware.security.dice.IDiceDevice/default";
/// Artifacts that are mapped into the process address space from the driver.
@@ -135,16 +136,19 @@
#[derive(Clone, Serialize, Deserialize)]
enum DriverArtifactManager {
+ Invalid,
Driver(PathBuf),
Updated(RawArtifacts),
}
impl DriverArtifactManager {
fn new(driver_path: &Path) -> Self {
- // TODO(218793274): fail if driver is missing in protected mode
if driver_path.exists() {
log::info!("Using DICE values from driver");
Self::Driver(driver_path.to_path_buf())
+ } else if Path::new(AVF_STRICT_BOOT).exists() {
+ log::error!("Strict boot requires DICE value from driver but none were found");
+ Self::Invalid
} else {
log::warn!("Using sample DICE values");
let (cdi_attest, cdi_seal, bcc) = diced_sample_inputs::make_sample_bcc_and_cdis()
@@ -164,11 +168,15 @@
F: FnOnce(&dyn DiceArtifacts) -> Result<T>,
{
match self {
+ Self::Invalid => bail!("No DICE artifacts available."),
Self::Driver(driver_path) => f(&MappedDriverArtifacts::new(driver_path.as_path())?),
Self::Updated(raw_artifacts) => f(raw_artifacts),
}
}
fn update(self, new_artifacts: &impl DiceArtifacts) -> Result<Self> {
+ if let Self::Invalid = self {
+ bail!("Cannot update invalid DICE artifacts.");
+ }
if let Self::Driver(driver_path) = self {
// Writing to the device wipes the artifcates. The string is ignored
// by the driver but included for documentation.
diff --git a/microdroid/init.rc b/microdroid/init.rc
index 5f0001f..f6d5092 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -24,10 +24,8 @@
chmod 0755 /dev/binderfs
symlink /dev/binderfs/binder /dev/binder
- symlink /dev/binderfs/hwbinder /dev/hwbinder
symlink /dev/binderfs/vndbinder /dev/vndbinder
- chmod 0666 /dev/binderfs/hwbinder
chmod 0666 /dev/binderfs/binder
chmod 0666 /dev/binderfs/vndbinder
@@ -83,6 +81,10 @@
setprop ro.debuggable ${ro.boot.microdroid.debuggable:-0}
+on init && property:ro.boot.microdroid.debuggable=1
+ # Mount tracefs (with GID=AID_READTRACEFS)
+ mount tracefs tracefs /sys/kernel/tracing gid=3012
+
on init && property:ro.boot.logd.enabled=1
# Start logd before any other services run to ensure we capture all of their logs.
start logd
@@ -144,6 +146,12 @@
mkdir /data/misc/authfs 0700 root root
start authfs_service
+on late-fs && property:ro.debuggable=1
+ # Ensure that tracefs has the correct permissions.
+ # This does not work correctly if it is called in post-fs.
+ chmod 0755 /sys/kernel/tracing
+ chmod 0755 /sys/kernel/debug/tracing
+
on post-fs-data
mark_post_data
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index b644285..9e159d2 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -61,6 +61,8 @@
const DM_MOUNTED_APK_PATH: &str = "/dev/block/mapper/microdroid-apk";
const APKDMVERITY_BIN: &str = "/system/bin/apkdmverity";
const ZIPFUSE_BIN: &str = "/system/bin/zipfuse";
+const AVF_STRICT_BOOT: &str = "/sys/firmware/devicetree/base/chosen/avf,strict-boot";
+const AVF_NEW_INSTANCE: &str = "/sys/firmware/devicetree/base/chosen/avf,new-instance";
/// The CID representing the host VM
const VMADDR_CID_HOST: u32 = 2;
@@ -193,12 +195,35 @@
Ok(())
}
+fn is_strict_boot() -> bool {
+ Path::new(AVF_STRICT_BOOT).exists()
+}
+
+fn is_new_instance() -> bool {
+ Path::new(AVF_NEW_INSTANCE).exists()
+}
+
fn try_run_payload(service: &Strong<dyn IVirtualMachineService>) -> Result<i32> {
let metadata = load_metadata().context("Failed to load payload metadata")?;
let mut instance = InstanceDisk::new().context("Failed to load instance.img")?;
let saved_data = instance.read_microdroid_data().context("Failed to read identity data")?;
+ if is_strict_boot() {
+ // Provisioning must happen on the first boot and never again.
+ if is_new_instance() {
+ ensure!(
+ saved_data.is_none(),
+ MicrodroidError::InvalidConfig("Found instance data on first boot.".to_string())
+ );
+ } else {
+ ensure!(
+ saved_data.is_some(),
+ MicrodroidError::InvalidConfig("Instance data not found.".to_string())
+ );
+ };
+ }
+
// Verify the payload before using it.
let verified_data =
verify_payload(&metadata, saved_data.as_ref()).context("Payload verification failed")?;
diff --git a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
index f305372..5b71eba 100644
--- a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
+++ b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
@@ -445,5 +445,7 @@
archiveLogThenDelete(mTestLogs, getDevice(), LOG_PATH,
"vm.log-" + mTestName.getMethodName());
+
+ getDevice().uninstallPackage(PACKAGE_NAME);
}
}
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 8df853d..36bea72 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -544,8 +544,7 @@
.isNotEqualTo("vsoc_x86_64");
if (mProtectedVm) {
- // TODO(b/218461230): uncomment this after u-boot update
- // assertThatBootFailsAfterCompromisingPartition(U_BOOT_AVB_PARTITION_UUID);
+ assertThatBootFailsAfterCompromisingPartition(U_BOOT_AVB_PARTITION_UUID);
} else {
// non-protected VM shouldn't have u-boot avb data
assertThatPartitionIsMissing(U_BOOT_AVB_PARTITION_UUID);
@@ -560,8 +559,7 @@
.isNotEqualTo("vsoc_x86_64");
if (mProtectedVm) {
- // TODO(b/218461230): uncomment this after u-boot update
- // assertThatBootFailsAfterCompromisingPartition(U_BOOT_ENV_PARTITION_UUID);
+ assertThatBootFailsAfterCompromisingPartition(U_BOOT_ENV_PARTITION_UUID);
} else {
// non-protected VM shouldn't have u-boot env data
assertThatPartitionIsMissing(U_BOOT_ENV_PARTITION_UUID);