Merge "put constraints on vendor VM instance IDs and allocated instance IDs" into main am: f540a3732e am: fc7b7ea5fd
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Virtualization/+/3494939
Change-Id: I51b410043a44c21d6af97d781989e24ea8d62d6c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/android/virtmgr/src/aidl.rs b/android/virtmgr/src/aidl.rs
index d7f68b8..190acc7 100644
--- a/android/virtmgr/src/aidl.rs
+++ b/android/virtmgr/src/aidl.rs
@@ -641,6 +641,24 @@
let calling_partition = find_partition(CALLING_EXE_PATH.as_deref())?;
+ let instance_id = extract_instance_id(config);
+ // Require vendor instance IDs to start with a specific prefix so that they don't conflict
+ // with system instance IDs.
+ //
+ // We should also make sure that non-vendor VMs do not use the vendor prefix, but there are
+ // already system VMs in the wild that may have randomly generated IDs with the prefix, so,
+ // for now, we only check in one direction.
+ const INSTANCE_ID_VENDOR_PREFIX: &[u8] = &[0xFF, 0xFF, 0xFF, 0xFF];
+ if matches!(calling_partition, CallingPartition::Vendor | CallingPartition::Odm)
+ && !instance_id.starts_with(INSTANCE_ID_VENDOR_PREFIX)
+ {
+ return Err(anyhow!(
+ "vendor initiated VMs must have instance IDs starting with 0xFFFFFFFF, got {}",
+ hex::encode(instance_id)
+ ))
+ .or_service_specific_exception(-1);
+ }
+
check_config_features(config)?;
if cfg!(early) {
@@ -668,7 +686,6 @@
check_gdb_allowed(config)?;
}
- let instance_id = extract_instance_id(config);
let mut device_tree_overlays = vec![];
if let Some(dt_overlay) =
maybe_create_reference_dt_overlay(config, &instance_id, &temporary_directory)?
diff --git a/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl b/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
index 5193e21..7db5135 100644
--- a/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
+++ b/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
@@ -23,7 +23,11 @@
/** Name of VM */
String name;
- /** Id of the VM instance */
+ /**
+ * Id of the VM instance
+ *
+ * See AVirtualMachineRawConfig_setInstanceId for details.
+ */
byte[64] instanceId;
/** Main APK */
diff --git a/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl b/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
index c5fe982..1e4fe03 100644
--- a/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
+++ b/android/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
@@ -31,7 +31,11 @@
/** Name of VM */
String name;
- /** Id of the VM instance */
+ /**
+ * Id of the VM instance
+ *
+ * See AVirtualMachineRawConfig_setInstanceId for details.
+ */
byte[64] instanceId;
/** The kernel image, if any. */
diff --git a/android/virtualizationservice/src/aidl.rs b/android/virtualizationservice/src/aidl.rs
index 1646117..e26cd4f 100644
--- a/android/virtualizationservice/src/aidl.rs
+++ b/android/virtualizationservice/src/aidl.rs
@@ -489,6 +489,9 @@
id.try_fill(&mut rand::thread_rng())
.context("Failed to allocate instance_id")
.or_service_specific_exception(-1)?;
+ // Randomly allocated IDs always start with all 7s to avoid colliding with statically
+ // assigned IDs.
+ id[..4].fill(0x77);
let uid = get_calling_uid();
info!("Allocated a VM's instance_id: {:?}..., for uid: {:?}", &hex::encode(id)[..8], uid);
self.try_updating_sk_state(&id);
diff --git a/libs/libavf/include/android/virtualization.h b/libs/libavf/include/android/virtualization.h
index 4bfe47a..e907ac4 100644
--- a/libs/libavf/include/android/virtualization.h
+++ b/libs/libavf/include/android/virtualization.h
@@ -78,6 +78,9 @@
* The `instanceId` is expected to be re-used for the VM instance with an associated state (secret,
* encrypted storage) - i.e., rebooting the VM must not change the instanceId.
*
+ * `instanceId` MUST start with 0xFFFFFFFF if and only if this library is being
+ * called from code in a vendor or odm partition,
+ *
* \param config a virtual machine config object.
* \param instanceId a pointer to a 64-byte buffer for the instance ID.
* \param instanceIdSize the number of bytes in `instanceId`.