Merge "Remove usages of select on defaults property" into main
diff --git a/android/virtmgr/fsfdt/src/lib.rs b/android/virtmgr/fsfdt/src/lib.rs
index e176b7b..ff15efa 100644
--- a/android/virtmgr/fsfdt/src/lib.rs
+++ b/android/virtmgr/fsfdt/src/lib.rs
@@ -76,7 +76,7 @@
stack.push(entry.path());
subnode_names.push(name);
} else if entry_type.is_file() {
- let value = fs::read(&entry.path())?;
+ let value = fs::read(entry.path())?;
node.setprop(&name, &value)
.map_err(|e| anyhow!("Failed to set FDT property, {e:?}"))?;
diff --git a/android/virtmgr/src/crosvm.rs b/android/virtmgr/src/crosvm.rs
index 37618c7..08a9e47 100644
--- a/android/virtmgr/src/crosvm.rs
+++ b/android/virtmgr/src/crosvm.rs
@@ -25,7 +25,6 @@
use log::{debug, error, info};
use semver::{Version, VersionReq};
use nix::{fcntl::OFlag, unistd::pipe2, unistd::Uid, unistd::User};
-use nix::unistd::dup;
use regex::{Captures, Regex};
use rustutils::system_properties;
use shared_child::SharedChild;
@@ -36,7 +35,6 @@
use std::io::{self, Read};
use std::mem;
use std::num::{NonZeroU16, NonZeroU32};
-use std::os::fd::FromRawFd;
use std::os::unix::io::{AsRawFd, OwnedFd};
use std::os::unix::process::ExitStatusExt;
use std::path::{Path, PathBuf};
@@ -59,7 +57,6 @@
use rpcbinder::RpcServer;
/// external/crosvm
-use base::AsRawDescriptor;
use base::UnixSeqpacketListener;
use vm_control::{BalloonControlCommand, VmRequest, VmResponse};
@@ -1042,14 +1039,7 @@
let control_sock = UnixSeqpacketListener::bind(crosvm_control_socket_path)
.context("failed to create control server")?;
- command.arg("--socket").arg(add_preserved_fd(&mut preserved_fds, {
- let dup_fd = dup(control_sock.as_raw_descriptor())?;
- // SAFETY: UnixSeqpacketListener doesn't provide a way to convert it into a RawFd or
- // OwnedFd. In order to provide a OwnedFd for add_preserved_fd, dup the control socket
- // and create a OwnedFd from the duped fd. This is fine as the original fd is still
- // closed when control_socket is dropped.
- unsafe { OwnedFd::from_raw_fd(dup_fd) }
- }));
+ command.arg("--socket").arg(add_preserved_fd(&mut preserved_fds, control_sock));
if let Some(dt_overlay) = config.device_tree_overlay {
command.arg("--device-tree-overlay").arg(add_preserved_fd(&mut preserved_fds, dt_overlay));
@@ -1103,9 +1093,9 @@
if cfg!(network) {
if let Some(tap) = config.tap {
- command
- .arg("--net")
- .arg(format!("tap-fd={}", add_preserved_fd(&mut preserved_fds, tap)));
+ add_preserved_fd(&mut preserved_fds, tap);
+ let tap_fd = preserved_fds.last().unwrap().as_raw_fd();
+ command.arg("--net").arg(format!("tap-fd={tap_fd}"));
}
}
diff --git a/guest/microdroid_manager/src/verify.rs b/guest/microdroid_manager/src/verify.rs
index 84feb68..90671a6 100644
--- a/guest/microdroid_manager/src/verify.rs
+++ b/guest/microdroid_manager/src/verify.rs
@@ -272,7 +272,7 @@
for argument in args {
cmd.arg("--apk").arg(argument.apk).arg(argument.idsig).arg(argument.name);
if let Some(root_hash) = argument.saved_root_hash {
- cmd.arg(&hex::encode(root_hash));
+ cmd.arg(hex::encode(root_hash));
} else {
cmd.arg("none");
}
diff --git a/libs/apkverify/src/sigutil.rs b/libs/apkverify/src/sigutil.rs
index 7d03bb2..a47b4c5 100644
--- a/libs/apkverify/src/sigutil.rs
+++ b/libs/apkverify/src/sigutil.rs
@@ -79,6 +79,7 @@
/// 2. The top-level digest is computed over the concatenation of byte 0x5a, the number of
/// chunks (little-endian uint32), and the concatenation of digests of the chunks in the
/// order the chunks appear in the APK.
+ ///
/// (see https://source.android.com/security/apksigning/v2#integrity-protected-contents)
pub(crate) fn compute_digest(
&mut self,
diff --git a/libs/framework-virtualization/Android.bp b/libs/framework-virtualization/Android.bp
index d3a2b54..d02eec6 100644
--- a/libs/framework-virtualization/Android.bp
+++ b/libs/framework-virtualization/Android.bp
@@ -9,7 +9,10 @@
jarjar_rules: "jarjar-rules.txt",
- srcs: ["src/**/*.java"],
+ srcs: [
+ "src/**/*.java",
+ ":avf-build-flags-java-gen",
+ ],
static_libs: [
"android.system.virtualizationservice-java",
"avf_aconfig_flags_java",
@@ -53,3 +56,15 @@
],
},
}
+
+gensrcs {
+ name: "avf-build-flags-java-gen",
+ srcs: ["src/**/BuildFlags.java_template"],
+ output_extension: "java",
+ cmd: "cp $(in) $(genDir)/tmp.java && " +
+ select(release_flag("RELEASE_AVF_ENABLE_VENDOR_MODULES"), {
+ true: "sed -ie 's/@vendor_modules_enabled_placeholder/true/g'",
+ default: "sed -ie 's/@vendor_modules_enabled_placeholder/false/g'",
+ }) + " $(genDir)/tmp.java && " +
+ " cp $(genDir)/tmp.java $(out)",
+}
diff --git a/libs/framework-virtualization/src/android/system/virtualmachine/BuildFlags.java_template b/libs/framework-virtualization/src/android/system/virtualmachine/BuildFlags.java_template
new file mode 100644
index 0000000..12b249c
--- /dev/null
+++ b/libs/framework-virtualization/src/android/system/virtualmachine/BuildFlags.java_template
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2024 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.
+ */
+
+package android.system.virtualmachine;
+
+/**
+ * Exposes AVF build flags (RELEASE_AVF_*) to java.
+ *
+ * @hide
+ */
+public final class BuildFlags {
+
+ /**
+ * Value of the {@code RELEASE_AVF_ENABLE_VENDOR_MODULES} build flag.
+ */
+ public static boolean VENDOR_MODULES_ENABLED = @vendor_modules_enabled_placeholder;
+
+ private BuildFlags() {};
+}
+
diff --git a/libs/framework-virtualization/src/android/system/virtualmachine/VirtualMachineManager.java b/libs/framework-virtualization/src/android/system/virtualmachine/VirtualMachineManager.java
index 242dc91..9295c6c 100644
--- a/libs/framework-virtualization/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/libs/framework-virtualization/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -371,10 +371,6 @@
private static final List<String> SUPPORTED_OS_LIST_FROM_CFG =
extractSupportedOSListFromConfig();
- private boolean isVendorModuleEnabled() {
- return VirtualizationService.nativeIsVendorModulesFlagEnabled();
- }
-
private static List<String> extractSupportedOSListFromConfig() {
List<String> supportedOsList = new ArrayList<>();
File directory = new File("/apex/com.android.virt/etc");
@@ -400,7 +396,7 @@
@FlaggedApi(Flags.FLAG_AVF_V_TEST_APIS)
@NonNull
public List<String> getSupportedOSList() throws VirtualMachineException {
- if (isVendorModuleEnabled()) {
+ if (BuildFlags.VENDOR_MODULES_ENABLED) {
return SUPPORTED_OS_LIST_FROM_CFG;
} else {
return Arrays.asList("microdroid");
diff --git a/libs/framework-virtualization/src/android/system/virtualmachine/VirtualizationService.java b/libs/framework-virtualization/src/android/system/virtualmachine/VirtualizationService.java
index 83b64ee..57990a9 100644
--- a/libs/framework-virtualization/src/android/system/virtualmachine/VirtualizationService.java
+++ b/libs/framework-virtualization/src/android/system/virtualmachine/VirtualizationService.java
@@ -51,12 +51,6 @@
private native boolean nativeIsOk(int clientFd);
/*
- * Retrieve boolean value whether RELEASE_AVF_ENABLE_VENDOR_MODULES build flag is enabled or
- * not.
- */
- static native boolean nativeIsVendorModulesFlagEnabled();
-
- /*
* Spawns a new virtmgr subprocess that will host a VirtualizationService
* AIDL service.
*/
diff --git a/libs/libsafe_ownedfd/Android.bp b/libs/libsafe_ownedfd/Android.bp
new file mode 100644
index 0000000..1f14578
--- /dev/null
+++ b/libs/libsafe_ownedfd/Android.bp
@@ -0,0 +1,37 @@
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+rust_defaults {
+ name: "libsafe_ownedfd.defaults",
+ crate_name: "safe_ownedfd",
+ srcs: ["src/lib.rs"],
+ edition: "2021",
+ rustlibs: [
+ "libnix",
+ "libthiserror",
+ ],
+}
+
+rust_library {
+ name: "libsafe_ownedfd",
+ defaults: ["libsafe_ownedfd.defaults"],
+ apex_available: [
+ "com.android.compos",
+ "com.android.virt",
+ ],
+}
+
+rust_test {
+ name: "libsafe_ownedfd.test",
+ defaults: ["libsafe_ownedfd.defaults"],
+ rustlibs: [
+ "libanyhow",
+ "libtempfile",
+ ],
+ host_supported: true,
+ test_suites: ["general-tests"],
+ test_options: {
+ unit_test: true,
+ },
+}
diff --git a/libs/libsafe_ownedfd/src/lib.rs b/libs/libsafe_ownedfd/src/lib.rs
new file mode 100644
index 0000000..52ae180
--- /dev/null
+++ b/libs/libsafe_ownedfd/src/lib.rs
@@ -0,0 +1,127 @@
+// Copyright 2024, 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.
+
+//! Library for a safer conversion from `RawFd` to `OwnedFd`
+
+use nix::fcntl::{fcntl, FdFlag, F_DUPFD, F_GETFD, F_SETFD};
+use nix::libc;
+use nix::unistd::close;
+use std::os::fd::FromRawFd;
+use std::os::fd::OwnedFd;
+use std::os::fd::RawFd;
+use std::sync::Mutex;
+use thiserror::Error;
+
+/// Errors that can occur while taking an ownership of `RawFd`
+#[derive(Debug, PartialEq, Error)]
+pub enum Error {
+ /// RawFd is not a valid file descriptor
+ #[error("{0} is not a file descriptor")]
+ Invalid(RawFd),
+
+ /// RawFd is either stdio, stdout, or stderr
+ #[error("standard IO descriptors cannot be owned")]
+ StdioNotAllowed,
+
+ /// Generic UNIX error
+ #[error("UNIX error")]
+ Errno(#[from] nix::errno::Errno),
+}
+
+static LOCK: Mutex<()> = Mutex::new(());
+
+/// Takes the ownership of `RawFd` and converts it to `OwnedFd`. It is important to know that
+/// `RawFd` is closed when this function successfully returns. The raw file descriptor of the
+/// returned `OwnedFd` is different from `RawFd`. The returned file descriptor is CLOEXEC set.
+pub fn take_fd_ownership(raw_fd: RawFd) -> Result<OwnedFd, Error> {
+ fcntl(raw_fd, F_GETFD).map_err(|_| Error::Invalid(raw_fd))?;
+
+ if [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO].contains(&raw_fd) {
+ return Err(Error::StdioNotAllowed);
+ }
+
+ // sync is needed otherwise we can create multiple OwnedFds out of the same RawFd
+ let lock = LOCK.lock().unwrap();
+ let new_fd = fcntl(raw_fd, F_DUPFD(raw_fd))?;
+ close(raw_fd)?;
+ drop(lock);
+
+ // This is not essential, but let's follow the common practice in the Rust ecosystem
+ fcntl(new_fd, F_SETFD(FdFlag::FD_CLOEXEC)).map_err(Error::Errno)?;
+
+ // SAFETY: In this function, we have checked that RawFd is actually an open file descriptor and
+ // this is the first time to claim its ownership because we just created it by duping.
+ Ok(unsafe { OwnedFd::from_raw_fd(new_fd) })
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use anyhow::Result;
+ use nix::fcntl::{fcntl, FdFlag, F_GETFD, F_SETFD};
+ use std::os::fd::AsRawFd;
+ use std::os::fd::IntoRawFd;
+ use tempfile::tempfile;
+
+ #[test]
+ fn good_fd() -> Result<()> {
+ let raw_fd = tempfile()?.into_raw_fd();
+ assert!(take_fd_ownership(raw_fd).is_ok());
+ Ok(())
+ }
+
+ #[test]
+ fn invalid_fd() -> Result<()> {
+ let raw_fd = 12345; // randomly chosen
+ assert_eq!(take_fd_ownership(raw_fd).unwrap_err(), Error::Invalid(raw_fd));
+ Ok(())
+ }
+
+ #[test]
+ fn original_fd_closed() -> Result<()> {
+ let raw_fd = tempfile()?.into_raw_fd();
+ let owned_fd = take_fd_ownership(raw_fd)?;
+ assert_ne!(raw_fd, owned_fd.as_raw_fd());
+ assert!(fcntl(raw_fd, F_GETFD).is_err());
+ Ok(())
+ }
+
+ #[test]
+ fn cannot_use_same_rawfd_multiple_times() -> Result<()> {
+ let raw_fd = tempfile()?.into_raw_fd();
+
+ let owned_fd = take_fd_ownership(raw_fd); // once
+ let owned_fd2 = take_fd_ownership(raw_fd); // twice
+
+ assert!(owned_fd.is_ok());
+ assert!(owned_fd2.is_err());
+ Ok(())
+ }
+
+ #[test]
+ fn cloexec() -> Result<()> {
+ let raw_fd = tempfile()?.into_raw_fd();
+
+ // intentionally clear cloexec to see if it is set by take_fd_ownership
+ fcntl(raw_fd, F_SETFD(FdFlag::empty()))?;
+ let flags = fcntl(raw_fd, F_GETFD)?;
+ assert_eq!(flags, FdFlag::empty().bits());
+
+ let owned_fd = take_fd_ownership(raw_fd)?;
+ let flags = fcntl(owned_fd.as_raw_fd(), F_GETFD)?;
+ assert_eq!(flags, FdFlag::FD_CLOEXEC.bits());
+ drop(owned_fd);
+ Ok(())
+ }
+}
diff --git a/libs/libservice_vm_requests/src/dice.rs b/libs/libservice_vm_requests/src/dice.rs
index 247c34e..ef9d894 100644
--- a/libs/libservice_vm_requests/src/dice.rs
+++ b/libs/libservice_vm_requests/src/dice.rs
@@ -76,7 +76,7 @@
///
/// - The first entry of the `client_vm_dice_chain` must be signed with the root public key.
/// - After the first entry, each entry of the `client_vm_dice_chain` must be signed with the
- /// subject public key of the previous entry.
+ /// subject public key of the previous entry.
///
/// Returns a partially decoded client VM's DICE chain if the verification succeeds.
pub(crate) fn validate_signatures_and_parse_dice_chain(
diff --git a/libs/libservice_vm_requests/src/rkp.rs b/libs/libservice_vm_requests/src/rkp.rs
index c62a36b..e2be11b 100644
--- a/libs/libservice_vm_requests/src/rkp.rs
+++ b/libs/libservice_vm_requests/src/rkp.rs
@@ -180,8 +180,8 @@
/// order as per RFC8949.
/// The CBOR ordering rules are:
/// 1. If two keys have different lengths, the shorter one sorts earlier;
- /// 2. If two keys have the same length, the one with the lower value in
- /// (bytewise) lexical order sorts earlier.
+ /// 2. If two keys have the same length, the one with the lower value in (bytewise) lexical
+ /// order sorts earlier.
#[test]
fn device_info_is_in_length_first_deterministic_order() {
let device_info = cbor!(device_info()).unwrap();
diff --git a/libs/libvirtualization_jni/android_system_virtualmachine_VirtualizationService.cpp b/libs/libvirtualization_jni/android_system_virtualmachine_VirtualizationService.cpp
index ced2079..0538c9e 100644
--- a/libs/libvirtualization_jni/android_system_virtualmachine_VirtualizationService.cpp
+++ b/libs/libvirtualization_jni/android_system_virtualmachine_VirtualizationService.cpp
@@ -108,9 +108,3 @@
}
return pfds[0].revents == 0;
}
-
-extern "C" JNIEXPORT jboolean JNICALL
-Java_android_system_virtualmachine_VirtualizationService_nativeIsVendorModulesFlagEnabled(
- [[maybe_unused]] JNIEnv* env, [[maybe_unused]] jobject obj) {
- return android::virtualization::IsVendorModulesFlagEnabled();
-}
diff --git a/libs/libvm_payload/src/lib.rs b/libs/libvm_payload/src/lib.rs
index 5cc4431..13c6e76 100644
--- a/libs/libvm_payload/src/lib.rs
+++ b/libs/libvm_payload/src/lib.rs
@@ -401,8 +401,8 @@
/// Behavior is undefined if any of the following conditions are violated:
///
/// * `data` must be [valid] for writes of `size` bytes, if size > 0.
-/// * The region of memory beginning at `data` with `size` bytes must not overlap with the
-/// region of memory `res` points to.
+/// * The region of memory beginning at `data` with `size` bytes must not overlap with the region of
+/// memory `res` points to.
///
/// [valid]: ptr#safety
/// [RFC 5915 s3]: https://datatracker.ietf.org/doc/html/rfc5915#section-3
@@ -439,8 +439,8 @@
///
/// * `message` must be [valid] for reads of `message_size` bytes.
/// * `data` must be [valid] for writes of `size` bytes, if size > 0.
-/// * The region of memory beginning at `data` with `size` bytes must not overlap with the
-/// region of memory `res` or `message` point to.
+/// * The region of memory beginning at `data` with `size` bytes must not overlap with the region of
+/// memory `res` or `message` point to.
///
///
/// [valid]: ptr#safety
@@ -507,8 +507,8 @@
/// * `data` must be [valid] for writes of `size` bytes, if size > 0.
/// * `index` must be within the range of [0, number of certificates). The number of certificates
/// can be obtained with `AVmAttestationResult_getCertificateCount`.
-/// * The region of memory beginning at `data` with `size` bytes must not overlap with the
-/// region of memory `res` points to.
+/// * The region of memory beginning at `data` with `size` bytes must not overlap with the region of
+/// memory `res` points to.
///
/// [valid]: ptr#safety
#[no_mangle]
diff --git a/libs/libvmclient/src/lib.rs b/libs/libvmclient/src/lib.rs
index fe86504..7b576e6 100644
--- a/libs/libvmclient/src/lib.rs
+++ b/libs/libvmclient/src/lib.rs
@@ -45,7 +45,6 @@
use shared_child::SharedChild;
use std::io::{self, Read};
use std::process::Command;
-use std::process::Stdio;
use std::{
fmt::{self, Debug, Formatter},
fs::File,
@@ -91,9 +90,6 @@
let (client_fd, server_fd) = posix_socketpair()?;
let mut command = Command::new(VIRTMGR_PATH);
- command.stdin(Stdio::null());
- command.stdout(Stdio::null());
- command.stderr(Stdio::null());
// Can't use BorrowedFd as it doesn't implement Display
command.arg("--rpc-server-fd").arg(format!("{}", server_fd.as_raw_fd()));
command.arg("--ready-fd").arg(format!("{}", ready_fd.as_raw_fd()));