Merge "[avb] Add library libpvmfw_avb_nostd and test it in presubmit"
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index d14d83c..592a751 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -88,9 +88,6 @@
}
public class VirtualMachineException extends java.lang.Exception {
- ctor public VirtualMachineException(@Nullable String);
- ctor public VirtualMachineException(@Nullable String, @Nullable Throwable);
- ctor public VirtualMachineException(@Nullable Throwable);
}
public class VirtualMachineManager {
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineException.java b/javalib/src/android/system/virtualmachine/VirtualMachineException.java
index 985eb70..9948fda 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineException.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineException.java
@@ -26,15 +26,15 @@
*/
@SystemApi
public class VirtualMachineException extends Exception {
- public VirtualMachineException(@Nullable String message) {
+ VirtualMachineException(@Nullable String message) {
super(message);
}
- public VirtualMachineException(@Nullable String message, @Nullable Throwable cause) {
+ VirtualMachineException(@Nullable String message, @Nullable Throwable cause) {
super(message, cause);
}
- public VirtualMachineException(@Nullable Throwable cause) {
+ VirtualMachineException(@Nullable Throwable cause) {
super(cause);
}
}
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
index ea0a305..5b30617 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -150,6 +150,11 @@
* Returns an existing {@link VirtualMachine} with the given name. Returns null if there is no
* such virtual machine.
*
+ * <p>There is at most one {@code VirtualMachine} object corresponding to a given virtual
+ * machine instance. Multiple calls to get() passing the same name will get the same object
+ * returned, until the virtual machine is deleted (via {@link #delete}) and then recreated.
+ *
+ * @see #getOrCreate
* @throws VirtualMachineException if the virtual machine exists but could not be successfully
* retrieved.
* @hide
diff --git a/pvmfw/src/hvc.rs b/pvmfw/src/hvc.rs
new file mode 100644
index 0000000..66f7977
--- /dev/null
+++ b/pvmfw/src/hvc.rs
@@ -0,0 +1,66 @@
+// Copyright 2022, 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.
+
+//! Wrappers around calls to the hypervisor.
+
+use crate::smccc::{self, checked_hvc64, checked_hvc64_expect_zero};
+use log::info;
+
+const VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID: u32 = 0xc6000005;
+const VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID: u32 = 0xc6000006;
+const VENDOR_HYP_KVM_MMIO_GUARD_MAP_FUNC_ID: u32 = 0xc6000007;
+const VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID: u32 = 0xc6000008;
+
+pub fn mmio_guard_info() -> smccc::Result<u64> {
+ let args = [0u64; 17];
+
+ checked_hvc64(VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID, args)
+}
+
+pub fn mmio_guard_enroll() -> smccc::Result<()> {
+ let args = [0u64; 17];
+
+ checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID, args)
+}
+
+pub fn mmio_guard_map(ipa: u64) -> smccc::Result<()> {
+ let mut args = [0u64; 17];
+ args[0] = ipa;
+
+ // TODO(b/253586500): pKVM currently returns a i32 instead of a i64.
+ let is_i32_error_code = |n| u32::try_from(n).ok().filter(|v| (*v as i32) < 0).is_some();
+ match checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_MAP_FUNC_ID, args) {
+ Err(smccc::Error::Unexpected(e)) if is_i32_error_code(e) => {
+ info!("Handled a pKVM bug by interpreting the MMIO_GUARD_MAP return value as i32");
+ match e as u32 as i32 {
+ -1 => Err(smccc::Error::NotSupported),
+ -2 => Err(smccc::Error::NotRequired),
+ -3 => Err(smccc::Error::InvalidParameter),
+ ret => Err(smccc::Error::Unknown(ret as i64)),
+ }
+ }
+ res => res,
+ }
+}
+
+pub fn mmio_guard_unmap(ipa: u64) -> smccc::Result<()> {
+ let mut args = [0u64; 17];
+ args[0] = ipa;
+
+ // TODO(b/251426790): pKVM currently returns NOT_SUPPORTED for SUCCESS.
+ match checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID, args) {
+ Err(smccc::Error::NotSupported) | Ok(_) => Ok(()),
+ x => x,
+ }
+}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 039e674..59efca0 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -26,6 +26,7 @@
mod fdt;
mod heap;
mod helpers;
+mod hvc;
mod memory;
mod mmio_guard;
mod mmu;
diff --git a/pvmfw/src/mmio_guard.rs b/pvmfw/src/mmio_guard.rs
index 28f928f..e5f376e 100644
--- a/pvmfw/src/mmio_guard.rs
+++ b/pvmfw/src/mmio_guard.rs
@@ -15,9 +15,9 @@
//! Safe MMIO_GUARD support.
use crate::helpers;
+use crate::hvc::{mmio_guard_enroll, mmio_guard_info, mmio_guard_map, mmio_guard_unmap};
use crate::smccc;
use core::{fmt, result};
-use log::info;
#[derive(Debug, Clone)]
pub enum Error {
@@ -63,50 +63,3 @@
pub fn unmap(addr: usize) -> Result<()> {
mmio_guard_unmap(helpers::page_4kb_of(addr) as u64).map_err(Error::UnmapFailed)
}
-
-fn mmio_guard_info() -> smccc::Result<u64> {
- const VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID: u32 = 0xc6000005;
- let args = [0u64; 17];
-
- smccc::checked_hvc64(VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID, args)
-}
-
-fn mmio_guard_enroll() -> smccc::Result<()> {
- const VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID: u32 = 0xc6000006;
- let args = [0u64; 17];
-
- smccc::checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_ENROLL_FUNC_ID, args)
-}
-
-fn mmio_guard_map(ipa: u64) -> smccc::Result<()> {
- const VENDOR_HYP_KVM_MMIO_GUARD_MAP_FUNC_ID: u32 = 0xc6000007;
- let mut args = [0u64; 17];
- args[0] = ipa;
-
- // TODO(b/253586500): pKVM currently returns a i32 instead of a i64.
- let is_i32_error_code = |n| u32::try_from(n).ok().filter(|v| (*v as i32) < 0).is_some();
- match smccc::checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_MAP_FUNC_ID, args) {
- Err(smccc::Error::Unexpected(e)) if is_i32_error_code(e) => {
- info!("Handled a pKVM bug by interpreting the MMIO_GUARD_MAP return value as i32");
- match e as u32 as i32 {
- -1 => Err(smccc::Error::NotSupported),
- -2 => Err(smccc::Error::NotRequired),
- -3 => Err(smccc::Error::InvalidParameter),
- ret => Err(smccc::Error::Unknown(ret as i64)),
- }
- }
- res => res,
- }
-}
-
-fn mmio_guard_unmap(ipa: u64) -> smccc::Result<()> {
- const VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID: u32 = 0xc6000008;
- let mut args = [0u64; 17];
- args[0] = ipa;
-
- // TODO(b/251426790): pKVM currently returns NOT_SUPPORTED for SUCCESS.
- match smccc::checked_hvc64_expect_zero(VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID, args) {
- Err(smccc::Error::NotSupported) | Ok(_) => Ok(()),
- x => x,
- }
-}
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 5f24c4b..eb756d8 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -258,14 +258,7 @@
assertThrowsVmExceptionContaining(() -> vm.toDescriptor(), "deleted");
// This is indistinguishable from the VM having never existed, so the message
// is non-specific.
- assertThrows(
- VirtualMachineException.class, () -> getVirtualMachineManager().delete("test_vm"));
- }
-
- private void assertThrowsVmExceptionContaining(
- ThrowingRunnable runnable, String expectedContents) {
- Exception e = assertThrows(VirtualMachineException.class, runnable);
- assertThat(e).hasMessageThat().contains(expectedContents);
+ assertThrowsVmException(() -> getVirtualMachineManager().delete("test_vm"));
}
@Test
@@ -423,6 +416,48 @@
@Test
@CddTest(requirements = {"9.17/C-1-1"})
+ public void vmmGetAndCreate() throws Exception {
+ assumeSupportedKernel();
+
+ VirtualMachineConfig config =
+ newVmConfigBuilder()
+ .setPayloadBinaryPath("MicrodroidTestNativeLib.so")
+ .setMemoryMib(minMemoryRequired())
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .build();
+
+ VirtualMachineManager vmm = getVirtualMachineManager();
+ String vmName = "vmName";
+
+ // VM does not yet exist
+ assertThat(vmm.get(vmName)).isNull();
+
+ VirtualMachine vm1 = vmm.create(vmName, config);
+
+ // Now it does, and we should get the same instance back
+ assertThat(vmm.get(vmName)).isSameInstanceAs(vm1);
+ assertThat(vmm.getOrCreate(vmName, config)).isSameInstanceAs(vm1);
+
+ // Can't recreate it though
+ assertThrowsVmException(() -> vmm.create(vmName, config));
+
+ vmm.delete(vmName);
+ assertThat(vmm.get(vmName)).isNull();
+
+ // Now that we deleted the old one, this should create rather than get, and it should be a
+ // new instance.
+ VirtualMachine vm2 = vmm.getOrCreate(vmName, config);
+ assertThat(vm2).isNotSameInstanceAs(vm1);
+
+ // Subsequent gets should return this new one.
+ assertThat(vmm.get(vmName)).isSameInstanceAs(vm2);
+ assertThat(vmm.getOrCreate(vmName, config)).isSameInstanceAs(vm2);
+
+ vmm.delete(vmName);
+ }
+
+ @Test
+ @CddTest(requirements = {"9.17/C-1-1"})
public void vmFilesStoredInDeDirWhenCreatedFromDEContext() throws Exception {
final Context ctx = getContext().createDeviceProtectedStorageContext();
final int userId = ctx.getUserId();
@@ -511,10 +546,10 @@
assertThat(vmm.get("test_vm_delete")).isNull();
// Can't start the VM even with an existing reference
- assertThrows(VirtualMachineException.class, vm::run);
+ assertThrowsVmException(vm::run);
// Can't delete the VM since it no longer exists
- assertThrows(VirtualMachineException.class, () -> vmm.delete("test_vm_delete"));
+ assertThrowsVmException(() -> vmm.delete("test_vm_delete"));
}
@Test
@@ -1098,6 +1133,16 @@
return filePath.toFile();
}
+ private void assertThrowsVmException(ThrowingRunnable runnable) {
+ assertThrows(VirtualMachineException.class, runnable);
+ }
+
+ private void assertThrowsVmExceptionContaining(
+ ThrowingRunnable runnable, String expectedContents) {
+ Exception e = assertThrows(VirtualMachineException.class, runnable);
+ assertThat(e).hasMessageThat().contains(expectedContents);
+ }
+
private int minMemoryRequired() {
if (Build.SUPPORTED_ABIS.length > 0) {
String primaryAbi = Build.SUPPORTED_ABIS[0];
diff --git a/vm_payload/include-restricted/vm_payload_restricted.h b/vm_payload/include-restricted/vm_payload_restricted.h
index 0b78541..7f17cde 100644
--- a/vm_payload/include-restricted/vm_payload_restricted.h
+++ b/vm_payload/include-restricted/vm_payload_restricted.h
@@ -34,21 +34,21 @@
/**
* Get the VM's DICE attestation chain.
*
- * \param data pointer to size bytes where the chain is written.
+ * \param data pointer to size bytes where the chain is written (may be null if size is 0).
* \param size number of bytes that can be written to data.
*
* \return the total size of the chain
*/
-size_t AVmPayload_getDiceAttestationChain(void *data, size_t size);
+size_t AVmPayload_getDiceAttestationChain(void* _Nullable data, size_t size);
/**
* Get the VM's DICE attestation CDI.
*
- * \param data pointer to size bytes where the CDI is written.
+ * \param data pointer to size bytes where the CDI is written (may be null if size is 0).
* \param size number of bytes that can be written to data.
*
* \return the total size of the CDI
*/
-size_t AVmPayload_getDiceAttestationCdi(void *data, size_t size);
+size_t AVmPayload_getDiceAttestationCdi(void* _Nullable data, size_t size);
__END_DECLS
diff --git a/vm_payload/include/vm_payload.h b/vm_payload/include/vm_payload.h
index 7c224f6..e0c2613 100644
--- a/vm_payload/include/vm_payload.h
+++ b/vm_payload/include/vm_payload.h
@@ -18,6 +18,7 @@
#include <stdbool.h>
#include <stddef.h>
+#include <stdint.h>
#include <stdnoreturn.h>
#include <sys/cdefs.h>
@@ -52,12 +53,13 @@
*
* \param service the service to bind to the given port.
* \param port vsock port.
- * \param on_ready the callback to execute once the server is ready for connections. The callback
- * will be called at most once.
- * \param param param for the `on_ready` callback.
+ * \param on_ready the callback to execute once the server is ready for connections. If not null the
+ * callback will be called at most once.
+ * \param param parameter to be passed to the `on_ready` callback.
*/
-noreturn void AVmPayload_runVsockRpcServer(AIBinder *service, unsigned int port,
- void (*on_ready)(void *param), void *param);
+noreturn void AVmPayload_runVsockRpcServer(AIBinder* _Nonnull service, uint32_t port,
+ void (*_Nullable on_ready)(void* _Nullable param),
+ void* _Nullable param);
/**
* Get a secret that is uniquely bound to this VM instance. The secrets are
@@ -69,8 +71,8 @@
* \param secret pointer to size bytes where the secret is written.
* \param size number of bytes of the secret to get, <= 32.
*/
-void AVmPayload_getVmInstanceSecret(const void *identifier, size_t identifier_size, void *secret,
- size_t size);
+void AVmPayload_getVmInstanceSecret(const void* _Nonnull identifier, size_t identifier_size,
+ void* _Nonnull secret, size_t size);
/**
* Gets the path to the APK contents. It is a directory, under which are
@@ -81,7 +83,7 @@
* deleted or freed by the application. The string remains valid for the
* lifetime of the VM.
*/
-const char *AVmPayload_getApkContentsPath(void);
+const char* _Nonnull AVmPayload_getApkContentsPath(void);
/**
* Gets the path to the encrypted persistent storage for the VM, if any. This is
@@ -94,6 +96,6 @@
* be deleted or freed by the application and remains valid for the lifetime of
* the VM.
*/
-const char *AVmPayload_getEncryptedStoragePath(void);
+const char* _Nullable AVmPayload_getEncryptedStoragePath(void);
__END_DECLS
diff --git a/vm_payload/src/api.rs b/vm_payload/src/api.rs
index 66c8ef7..a9b3abb 100644
--- a/vm_payload/src/api.rs
+++ b/vm_payload/src/api.rs
@@ -206,7 +206,7 @@
///
/// Behavior is undefined if any of the following conditions are violated:
///
-/// * `data` must be [valid] for writes of `size` bytes.
+/// * `data` must be [valid] for writes of `size` bytes, if size > 0.
///
/// [valid]: ptr#safety
#[no_mangle]
@@ -214,9 +214,13 @@
initialize_logging();
let chain = unwrap_or_abort(try_get_dice_attestation_chain());
- // SAFETY: See the requirements on `data` above. The number of bytes copied doesn't exceed
- // the length of either buffer, and `chain` cannot overlap `data` because we just allocated it.
- unsafe { ptr::copy_nonoverlapping(chain.as_ptr(), data, std::cmp::min(chain.len(), size)) };
+ if size != 0 {
+ // SAFETY: See the requirements on `data` above. The number of bytes copied doesn't exceed
+ // the length of either buffer, and `chain` cannot overlap `data` because we just allocated
+ // it. We allow data to be null, which is never valid, but only if size == 0 which is
+ // checked above.
+ unsafe { ptr::copy_nonoverlapping(chain.as_ptr(), data, std::cmp::min(chain.len(), size)) };
+ }
chain.len()
}
@@ -231,7 +235,7 @@
///
/// Behavior is undefined if any of the following conditions are violated:
///
-/// * `data` must be [valid] for writes of `size` bytes.
+/// * `data` must be [valid] for writes of `size` bytes, if size > 0.
///
/// [valid]: ptr#safety
#[no_mangle]
@@ -239,9 +243,13 @@
initialize_logging();
let cdi = unwrap_or_abort(try_get_dice_attestation_cdi());
- // SAFETY: See the requirements on `data` above. The number of bytes copied doesn't exceed
- // the length of either buffer, and `cdi` cannot overlap `data` because we just allocated it.
- unsafe { ptr::copy_nonoverlapping(cdi.as_ptr(), data, std::cmp::min(cdi.len(), size)) };
+ if size != 0 {
+ // SAFETY: See the requirements on `data` above. The number of bytes copied doesn't exceed
+ // the length of either buffer, and `cdi` cannot overlap `data` because we just allocated
+ // it. We allow data to be null, which is never valid, but only if size == 0 which is
+ // checked above.
+ unsafe { ptr::copy_nonoverlapping(cdi.as_ptr(), data, std::cmp::min(cdi.len(), size)) };
+ }
cdi.len()
}