Native API fixes
Port is always 32 bits, move it to uint32_t.
Add _Nullable / _Nonnull annotations in the headers. That initially
triggered a warning because we deliberately pass null to the
getDiceAttestation* functions to get the size needed. Which led me to
notice that our Rust code technically had undefined behavior, because
a null pointer is never valid even for size 0. Added a guard for that,
and documented that null is only allowed for size 0.
Bug: 262415211
Test: atest MicrodroidTests
Change-Id: I44bb2946989f7254e581d885542b41399c3ee059
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()
}