Add notifyError/onError notification for VM errors
For now, microdroid_manager shuts down a VM instance when it fails to
verify the VM payload. This change adds a new "error" notification to
deliver the error to the VM app.
Fow now this doesn't change the workflow. In a follow-up change this
will be changed so that VM app can decide to shutdown or not "onError".
Bug: 205778374
Test: MicrodroidHostTestCases
Change-Id: If7fdac3996b69601be0eda17da3e4cf218b4d1d8
diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs
index 508423b..4908f94 100644
--- a/compos/common/compos_client.rs
+++ b/compos/common/compos_client.rs
@@ -297,6 +297,12 @@
log::warn!("VM payload finished, cid = {}, exit code = {}", cid, exit_code);
Ok(())
}
+
+ fn onError(&self, cid: i32, error_code: i32, message: &str) -> BinderResult<()> {
+ self.0.set_died();
+ log::warn!("VM error, cid = {}, error code = {}, message = {}", cid, error_code, message,);
+ Ok(())
+ }
}
fn start_logging(pfd: &ParcelFileDescriptor) -> Result<()> {
diff --git a/compos/compos_key_cmd/compos_key_cmd.cpp b/compos/compos_key_cmd/compos_key_cmd.cpp
index 3f431da..55803a0 100644
--- a/compos/compos_key_cmd/compos_key_cmd.cpp
+++ b/compos/compos_key_cmd/compos_key_cmd.cpp
@@ -145,6 +145,14 @@
return ScopedAStatus::ok();
}
+ ::ndk::ScopedAStatus onError(int32_t in_cid, int32_t in_error_code,
+ const std::string& in_message) override {
+ // For now, just log the error as onDied() will follow.
+ LOG(WARNING) << "VM error! cid = " << in_cid << ", error_code = " << in_error_code
+ << ", message = " << in_message;
+ return ScopedAStatus::ok();
+ }
+
::ndk::ScopedAStatus onDied(int32_t in_cid) override {
LOG(WARNING) << "VM died! cid = " << in_cid;
{
diff --git a/demo/java/com/android/microdroid/demo/MainActivity.java b/demo/java/com/android/microdroid/demo/MainActivity.java
index 15d9046..1a0e14d 100644
--- a/demo/java/com/android/microdroid/demo/MainActivity.java
+++ b/demo/java/com/android/microdroid/demo/MainActivity.java
@@ -263,6 +263,17 @@
}
@Override
+ public void onError(VirtualMachine vm, int errorCode, String message) {
+ // This check doesn't 100% prevent race condition, but is fine for demo.
+ if (!mService.isShutdown()) {
+ mPayloadOutput.postValue(
+ String.format(
+ "(Error occurred. code: %d, message: %s)",
+ errorCode, message));
+ }
+ }
+
+ @Override
public void onDied(VirtualMachine vm) {
mService.shutdownNow();
mStatus.postValue(VirtualMachine.Status.STOPPED);
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index d04da0e..c2a897b 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -401,6 +401,16 @@
}
@Override
+ public void onError(int cid, int errorCode, String message) {
+ final VirtualMachineCallback cb = mCallback;
+ if (cb == null) {
+ return;
+ }
+ mCallbackExecutor.execute(
+ () -> cb.onError(VirtualMachine.this, errorCode, message));
+ }
+
+ @Override
public void onDied(int cid) {
final VirtualMachineCallback cb = mCallback;
if (cb == null) {
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
index 988acd7..9dbed64 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
@@ -16,10 +16,14 @@
package android.system.virtualmachine;
+import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.ParcelFileDescriptor;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
/**
* Callback interface to get notified with the events from the virtual machine. The methods are
* executed on a binder thread. Implementations can make blocking calls in the methods.
@@ -27,6 +31,22 @@
* @hide
*/
public interface VirtualMachineCallback {
+ /** @hide */
+ @Retention(RetentionPolicy.SOURCE)
+ @IntDef({ERROR_UNKNOWN, ERROR_PAYLOAD_VERIFICATION_FAILED, ERROR_PAYLOAD_CHANGED})
+ @interface ErrorCode {}
+
+ /** Error code for all other errors not listed below. */
+ int ERROR_UNKNOWN = 0;
+
+ /**
+ * Error code indicating that the payload can't be verified due to various reasons (e.g invalid
+ * merkle tree, invalid formats, etc).
+ */
+ int ERROR_PAYLOAD_VERIFICATION_FAILED = 1;
+
+ /** Error code indicating that the payload is verified, but has changed since the last boot. */
+ int ERROR_PAYLOAD_CHANGED = 2;
/** Called when the payload starts in the VM. */
void onPayloadStarted(@NonNull VirtualMachine vm, @Nullable ParcelFileDescriptor stream);
@@ -37,6 +57,9 @@
/** Called when the payload has finished in the VM. */
void onPayloadFinished(@NonNull VirtualMachine vm, int exitCode);
+ /** Called when an error occurs in the VM. */
+ void onError(@NonNull VirtualMachine vm, @ErrorCode int errorCode, @NonNull String message);
+
/** Called when the VM died. */
void onDied(@NonNull VirtualMachine vm);
}
diff --git a/microdroid_manager/Android.bp b/microdroid_manager/Android.bp
index 721f9fa..f427966 100644
--- a/microdroid_manager/Android.bp
+++ b/microdroid_manager/Android.bp
@@ -30,6 +30,7 @@
"libserde",
"libserde_cbor",
"libserde_json",
+ "libthiserror",
"libuuid",
"libvsock",
"librand",
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index f666294..2e6fa36 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -19,7 +19,7 @@
mod payload;
use crate::instance::{ApkData, InstanceDisk, MicrodroidData, RootHash};
-use anyhow::{anyhow, bail, ensure, Context, Result};
+use anyhow::{anyhow, bail, ensure, Context, Error, Result};
use apkverify::{get_public_key_der, verify};
use binder::unstable_api::{new_spibinder, AIBinder};
use binder::{FromIBinder, Strong};
@@ -39,7 +39,7 @@
use vsock::VsockStream;
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
- VM_BINDER_SERVICE_PORT, VM_STREAM_SERVICE_PORT, IVirtualMachineService,
+ ERROR_PAYLOAD_CHANGED, ERROR_PAYLOAD_VERIFICATION_FAILED, ERROR_UNKNOWN, VM_BINDER_SERVICE_PORT, VM_STREAM_SERVICE_PORT, IVirtualMachineService,
};
const WAIT_TIMEOUT: Duration = Duration::from_secs(10);
@@ -51,6 +51,27 @@
const APEX_CONFIG_DONE_PROP: &str = "apex_config.done";
const LOGD_ENABLED_PROP: &str = "ro.boot.logd.enabled";
+#[derive(thiserror::Error, Debug)]
+enum MicrodroidError {
+ #[error("Payload has changed: {0}")]
+ PayloadChanged(String),
+ #[error("Payload verification has failed: {0}")]
+ PayloadVerificationFailed(String),
+}
+
+fn translate_error(err: &Error) -> (i32, String) {
+ if let Some(e) = err.downcast_ref::<MicrodroidError>() {
+ match e {
+ MicrodroidError::PayloadChanged(msg) => (ERROR_PAYLOAD_CHANGED, msg.to_string()),
+ MicrodroidError::PayloadVerificationFailed(msg) => {
+ (ERROR_PAYLOAD_VERIFICATION_FAILED, msg.to_string())
+ }
+ }
+ } else {
+ (ERROR_UNKNOWN, err.to_string())
+ }
+}
+
fn get_vms_rpc_binder() -> Result<Strong<dyn IVirtualMachineService>> {
// SAFETY: AIBinder returned by RpcClient has correct reference count, and the ownership can be
// safely taken by new_spibinder.
@@ -81,6 +102,17 @@
kernlog::init()?;
info!("started.");
+ let service = get_vms_rpc_binder().context("cannot connect to VirtualMachineService")?;
+ if let Err(err) = try_start_payload(&service) {
+ let (error_code, message) = translate_error(&err);
+ service.notifyError(error_code, &message)?;
+ Err(err)
+ } else {
+ Ok(())
+ }
+}
+
+fn try_start_payload(service: &Strong<dyn IVirtualMachineService>) -> Result<()> {
let metadata = load_metadata().context("Failed to load payload metadata")?;
let mut instance = InstanceDisk::new().context("Failed to load instance.img")?;
@@ -90,11 +122,13 @@
let verified_data =
verify_payload(&metadata, saved_data.as_ref()).context("Payload verification failed")?;
if let Some(saved_data) = saved_data {
- if saved_data == verified_data {
- info!("Saved data is verified.");
- } else {
- bail!("Detected an update of the payload which isn't supported yet.");
- }
+ ensure!(
+ saved_data == verified_data,
+ MicrodroidError::PayloadChanged(String::from(
+ "Detected an update of the payload which isn't supported yet."
+ ))
+ );
+ info!("Saved data is verified.");
} else {
info!("Saving verified data.");
instance.write_microdroid_data(&verified_data).context("Failed to write identity data")?;
@@ -103,7 +137,6 @@
// Before reading a file from the APK, start zipfuse
system_properties::write("ctl.start", "zipfuse")?;
- let service = get_vms_rpc_binder().expect("cannot connect to VirtualMachineService");
if !metadata.payload_config_path.is_empty() {
let config = load_config(Path::new(&metadata.payload_config_path))?;
@@ -117,7 +150,7 @@
wait_for_apex_config_done()?;
if let Some(main_task) = &config.task {
- exec_task(main_task, &service).map_err(|e| {
+ exec_task(main_task, service).map_err(|e| {
error!("failed to execute task: {}", e);
e
})?;
@@ -156,7 +189,10 @@
let apex_data_from_payload = get_apex_data_from_payload(metadata)?;
if let Some(saved_data) = saved_data.map(|d| &d.apex_data) {
// We don't support APEX updates. (assuming that update will change root digest)
- ensure!(saved_data == &apex_data_from_payload, "APEX payloads has changed.");
+ ensure!(
+ saved_data == &apex_data_from_payload,
+ MicrodroidError::PayloadChanged(String::from("APEXes have changed."))
+ );
let apex_metadata = to_metadata(&apex_data_from_payload);
// Pass metadata(with public keys and root digests) to apexd so that it uses the passed
// metadata instead of the default one (/dev/block/by-name/payload-metadata)
@@ -178,7 +214,10 @@
// of the VM or APK was updated in the host.
// TODO(jooyung): consider multithreading to make this faster
let apk_pubkey = if !root_hash_trustful {
- verify(DM_MOUNTED_APK_PATH).context(format!("failed to verify {}", DM_MOUNTED_APK_PATH))?
+ verify(DM_MOUNTED_APK_PATH).context(MicrodroidError::PayloadVerificationFailed(format!(
+ "failed to verify {}",
+ DM_MOUNTED_APK_PATH
+ )))?
} else {
get_public_key_der(DM_MOUNTED_APK_PATH)?
};
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 e0d6cc1..61c3edc 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -111,6 +111,9 @@
public void onPayloadFinished(VirtualMachine vm, int exitCode) {}
@Override
+ public void onError(VirtualMachine vm, int errorCode, String message) {}
+
+ @Override
public void onDied(VirtualMachine vm) {}
}
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
index 15354a3..d7f90a1 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
@@ -42,6 +42,11 @@
void onPayloadFinished(int cid, int exitCode);
/**
+ * Called when an error occurs in the VM.
+ */
+ void onError(int cid, int errorCode, in String message);
+
+ /**
* Called when the VM dies.
*
* Note that this will not be called if the VirtualizationService itself dies, so you should
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineState.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineState.aidl
index b1aebfd..d85b3c1 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineState.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineState.aidl
@@ -44,5 +44,5 @@
/**
* The VM has died.
*/
- DEAD = 5,
+ DEAD = 6,
}
diff --git a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
index 8611898..97f6ca3 100644
--- a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
+++ b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
@@ -43,4 +43,25 @@
* Notifies that the payload has finished.
*/
void notifyPayloadFinished(int exitCode);
+
+ /**
+ * Notifies that an error has occurred. See the ERROR_* constants.
+ */
+ void notifyError(int errorCode, in String message);
+
+ /**
+ * Error code for all other errors not listed below.
+ */
+ const int ERROR_UNKNOWN = 0;
+
+ /**
+ * Error code indicating that the payload can't be verified due to various reasons (e.g invalid
+ * merkle tree, invalid formats, etc).
+ */
+ const int ERROR_PAYLOAD_VERIFICATION_FAILED = 1;
+
+ /**
+ * Error code indicating that the payload is verified, but has changed since the last boot.
+ */
+ const int ERROR_PAYLOAD_CHANGED = 2;
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 5d64684..1c88174 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -714,6 +714,16 @@
}
}
+ /// Call all registered callbacks to say that the VM encountered an error.
+ pub fn notify_error(&self, cid: Cid, error_code: i32, message: &str) {
+ let callbacks = &*self.0.lock().unwrap();
+ for callback in callbacks {
+ if let Err(e) = callback.onError(cid as i32, error_code, message) {
+ error!("Error notifying error event from VM CID {}: {}", cid, e);
+ }
+ }
+ }
+
/// Call all registered callbacks to say that the VM has died.
pub fn callback_on_died(&self, cid: Cid) {
let callbacks = &*self.0.lock().unwrap();
@@ -879,10 +889,10 @@
vm.callbacks.notify_payload_started(cid, stream);
Ok(())
} else {
- error!("notifyPayloadStarted is called from an unknown cid {}", cid);
+ error!("notifyPayloadStarted is called from an unknown CID {}", cid);
Err(new_binder_exception(
ExceptionCode::SERVICE_SPECIFIC,
- format!("cannot find a VM with cid {}", cid),
+ format!("cannot find a VM with CID {}", cid),
))
}
}
@@ -896,10 +906,10 @@
vm.callbacks.notify_payload_ready(cid);
Ok(())
} else {
- error!("notifyPayloadReady is called from an unknown cid {}", cid);
+ error!("notifyPayloadReady is called from an unknown CID {}", cid);
Err(new_binder_exception(
ExceptionCode::SERVICE_SPECIFIC,
- format!("cannot find a VM with cid {}", cid),
+ format!("cannot find a VM with CID {}", cid),
))
}
}
@@ -913,10 +923,27 @@
vm.callbacks.notify_payload_finished(cid, exit_code);
Ok(())
} else {
- error!("notifyPayloadFinished is called from an unknown cid {}", cid);
+ error!("notifyPayloadFinished is called from an unknown CID {}", cid);
Err(new_binder_exception(
ExceptionCode::SERVICE_SPECIFIC,
- format!("cannot find a VM with cid {}", cid),
+ format!("cannot find a VM with CID {}", cid),
+ ))
+ }
+ }
+
+ fn notifyError(&self, error_code: i32, message: &str) -> binder::Result<()> {
+ let cid = self.cid;
+ if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
+ info!("VM having CID {} encountered an error", cid);
+ vm.update_payload_state(PayloadState::Finished)
+ .map_err(|e| new_binder_exception(ExceptionCode::ILLEGAL_STATE, e.to_string()))?;
+ vm.callbacks.notify_error(cid, error_code, message);
+ Ok(())
+ } else {
+ error!("notifyPayloadStarted is called from an unknown CID {}", cid);
+ Err(new_binder_exception(
+ ExceptionCode::SERVICE_SPECIFIC,
+ format!("cannot find a VM with CID {}", cid),
))
}
}
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 1cd51a1..7f5f9fc 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -271,6 +271,11 @@
Ok(())
}
+ fn onError(&self, _cid: i32, error_code: i32, message: &str) -> BinderResult<()> {
+ eprintln!("VM encountered an error: code={}, message={}", error_code, message);
+ Ok(())
+ }
+
fn onDied(&self, _cid: i32) -> BinderResult<()> {
// No need to explicitly report the event to the user (e.g. via println!) because this
// callback is registered only when the vm tool is invoked as interactive mode (e.g. not