Merge "Don't use unsafe raw_from_fd" into main
diff --git a/android/TerminalApp/Android.bp b/android/TerminalApp/Android.bp
index f5f39e3..3ae014e 100644
--- a/android/TerminalApp/Android.bp
+++ b/android/TerminalApp/Android.bp
@@ -10,8 +10,10 @@
"vm_launcher_lib",
],
sdk_version: "system_current",
- product_specific: true,
optimize: {
shrink_resources: true,
},
+ apex_available: [
+ "com.android.virt",
+ ],
}
diff --git a/android/TerminalApp/AndroidManifest.xml b/android/TerminalApp/AndroidManifest.xml
index 07e6147..27b2b46 100644
--- a/android/TerminalApp/AndroidManifest.xml
+++ b/android/TerminalApp/AndroidManifest.xml
@@ -11,7 +11,8 @@
<activity android:name=".MainActivity"
android:screenOrientation="landscape"
android:configChanges="orientation|screenSize|keyboard|keyboardHidden|navigation|uiMode"
- android:exported="true">
+ android:exported="true"
+ android:enabled="false">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
<category android:name="android.intent.category.LAUNCHER" />
diff --git a/android/virtmgr/Android.bp b/android/virtmgr/Android.bp
index a21ee6c..4828057 100644
--- a/android/virtmgr/Android.bp
+++ b/android/virtmgr/Android.bp
@@ -55,6 +55,7 @@
"libregex",
"librpcbinder_rs",
"librustutils",
+ "libsafe_ownedfd",
"libsemver",
"libselinux_bindgen",
"libserde",
diff --git a/android/virtmgr/src/aidl.rs b/android/virtmgr/src/aidl.rs
index 144524f..7a357f3 100644
--- a/android/virtmgr/src/aidl.rs
+++ b/android/virtmgr/src/aidl.rs
@@ -73,6 +73,7 @@
use nix::unistd::pipe;
use rpcbinder::RpcServer;
use rustutils::system_properties;
+use safe_ownedfd::take_fd_ownership;
use semver::VersionReq;
use std::collections::HashSet;
use std::convert::TryInto;
@@ -82,7 +83,7 @@
use std::io::{BufRead, BufReader, Error, ErrorKind, Seek, SeekFrom, Write};
use std::iter;
use std::num::{NonZeroU16, NonZeroU32};
-use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
+use std::os::unix::io::{AsRawFd, IntoRawFd};
use std::os::unix::raw::pid_t;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex, Weak};
@@ -1274,7 +1275,7 @@
let stream = VsockStream::connect_with_cid_port(self.instance.cid, port)
.context("Failed to connect")
.or_service_specific_exception(-1)?;
- Ok(vsock_stream_to_pfd(stream))
+ vsock_stream_to_pfd(stream)
}
fn setHostConsoleName(&self, ptsname: &str) -> binder::Result<()> {
@@ -1433,10 +1434,12 @@
}
/// Converts a `VsockStream` to a `ParcelFileDescriptor`.
-fn vsock_stream_to_pfd(stream: VsockStream) -> ParcelFileDescriptor {
- // SAFETY: ownership is transferred from stream to f
- let f = unsafe { File::from_raw_fd(stream.into_raw_fd()) };
- ParcelFileDescriptor::new(f)
+fn vsock_stream_to_pfd(stream: VsockStream) -> binder::Result<ParcelFileDescriptor> {
+ let owned_fd = take_fd_ownership(stream.into_raw_fd())
+ .context("Failed to take ownership of the vsock stream")
+ .with_log()
+ .or_service_specific_exception(-1)?;
+ Ok(ParcelFileDescriptor::new(owned_fd))
}
/// Parses the platform version requirement string.
diff --git a/android/virtmgr/src/main.rs b/android/virtmgr/src/main.rs
index 445260f..7d29685 100644
--- a/android/virtmgr/src/main.rs
+++ b/android/virtmgr/src/main.rs
@@ -25,16 +25,16 @@
use crate::aidl::{GLOBAL_SERVICE, VirtualizationService};
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::IVirtualizationService::BnVirtualizationService;
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, Result};
use binder::{BinderFeatures, ProcessState};
use lazy_static::lazy_static;
use log::{info, LevelFilter};
use rpcbinder::{FileDescriptorTransportMode, RpcServer};
-use std::os::unix::io::{AsFd, FromRawFd, OwnedFd, RawFd};
+use std::os::unix::io::{AsFd, RawFd};
use clap::Parser;
-use nix::fcntl::{fcntl, F_GETFD, F_SETFD, FdFlag};
use nix::unistd::{write, Pid, Uid};
use std::os::unix::raw::{pid_t, uid_t};
+use safe_ownedfd::take_fd_ownership;
const LOG_TAG: &str = "virtmgr";
@@ -73,32 +73,6 @@
ready_fd: RawFd,
}
-fn take_fd_ownership(raw_fd: RawFd, owned_fds: &mut Vec<RawFd>) -> Result<OwnedFd, anyhow::Error> {
- // Basic check that the integer value does correspond to a file descriptor.
- fcntl(raw_fd, F_GETFD).with_context(|| format!("Invalid file descriptor {raw_fd}"))?;
-
- // The file descriptor had CLOEXEC disabled to be inherited from the parent.
- // Re-enable it to make sure it is not accidentally inherited further.
- fcntl(raw_fd, F_SETFD(FdFlag::FD_CLOEXEC))
- .with_context(|| format!("Could not set CLOEXEC on file descriptor {raw_fd}"))?;
-
- // Creating OwnedFd for stdio FDs is not safe.
- if [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO].contains(&raw_fd) {
- bail!("File descriptor {raw_fd} is standard I/O descriptor");
- }
-
- // Reject RawFds that already have a corresponding OwnedFd.
- if owned_fds.contains(&raw_fd) {
- bail!("File descriptor {raw_fd} already owned");
- }
- owned_fds.push(raw_fd);
-
- // SAFETY: Initializing OwnedFd for a RawFd provided in cmdline arguments.
- // We checked that the integer value corresponds to a valid FD and that this
- // is the first argument to claim its ownership.
- Ok(unsafe { OwnedFd::from_raw_fd(raw_fd) })
-}
-
fn check_vm_support() -> Result<()> {
if hypervisor_props::is_any_vm_supported()? {
Ok(())
@@ -122,11 +96,9 @@
let args = Args::parse();
- let mut owned_fds = vec![];
- let rpc_server_fd = take_fd_ownership(args.rpc_server_fd, &mut owned_fds)
- .expect("Failed to take ownership of rpc_server_fd");
- let ready_fd = take_fd_ownership(args.ready_fd, &mut owned_fds)
- .expect("Failed to take ownership of ready_fd");
+ let rpc_server_fd =
+ take_fd_ownership(args.rpc_server_fd).expect("Failed to take ownership of rpc_server_fd");
+ let ready_fd = take_fd_ownership(args.ready_fd).expect("Failed to take ownership of ready_fd");
// Start thread pool for kernel Binder connection to VirtualizationServiceInternal.
ProcessState::start_thread_pool();
diff --git a/build/apex/Android.bp b/build/apex/Android.bp
index 3a6ebb8..f493202 100644
--- a/build/apex/Android.bp
+++ b/build/apex/Android.bp
@@ -46,7 +46,10 @@
apps: [
"android.system.virtualmachine.res",
] + select(release_flag("RELEASE_AVF_SUPPORT_CUSTOM_VM_WITH_PARAVIRTUALIZED_DEVICES"), {
- true: ["VmLauncherApp"],
+ true: [
+ "VmLauncherApp",
+ "VmTerminalApp",
+ ],
default: [],
}),
diff --git a/guest/microdroid_manager/Android.bp b/guest/microdroid_manager/Android.bp
index 9c9a3d0..c4b6423 100644
--- a/guest/microdroid_manager/Android.bp
+++ b/guest/microdroid_manager/Android.bp
@@ -48,6 +48,7 @@
"libprotobuf",
"librpcbinder_rs",
"librustutils",
+ "libsafe_ownedfd",
"libsecretkeeper_client",
"libsecretkeeper_comm_nostd",
"libscopeguard",
diff --git a/guest/microdroid_manager/src/main.rs b/guest/microdroid_manager/src/main.rs
index 990d27a..a27d9b0 100644
--- a/guest/microdroid_manager/src/main.rs
+++ b/guest/microdroid_manager/src/main.rs
@@ -50,13 +50,14 @@
use rustutils::sockets::android_get_control_socket;
use rustutils::system_properties;
use rustutils::system_properties::PropertyWatcher;
+use safe_ownedfd::take_fd_ownership;
use secretkeeper_comm::data_types::ID_SIZE;
use std::borrow::Cow::{Borrowed, Owned};
use std::env;
use std::ffi::CString;
use std::fs::{self, create_dir, File, OpenOptions};
use std::io::{Read, Write};
-use std::os::unix::io::{FromRawFd, OwnedFd};
+use std::os::unix::io::OwnedFd;
use std::os::unix::process::CommandExt;
use std::os::unix::process::ExitStatusExt;
use std::path::Path;
@@ -494,15 +495,7 @@
/// of the file descriptor and it is called only once.
unsafe fn prepare_vm_payload_service_socket() -> Result<OwnedFd> {
let raw_fd = android_get_control_socket(VM_PAYLOAD_SERVICE_SOCKET_NAME)?;
-
- // Creating OwnedFd for stdio FDs is not safe.
- if [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO].contains(&raw_fd) {
- bail!("File descriptor {raw_fd} is standard I/O descriptor");
- }
- // SAFETY: Initializing OwnedFd for a RawFd created by the init.
- // We checked that the integer value corresponds to a valid FD and that the caller
- // ensures that this is the only place to claim its ownership.
- Ok(unsafe { OwnedFd::from_raw_fd(raw_fd) })
+ Ok(take_fd_ownership(raw_fd)?)
}
fn is_strict_boot() -> bool {
diff --git a/microfuchsia/OWNERS b/microfuchsia/OWNERS
new file mode 100644
index 0000000..8092be7
--- /dev/null
+++ b/microfuchsia/OWNERS
@@ -0,0 +1,2 @@
+awolter@google.com
+jamesr@google.com
diff --git a/tests/ferrochrome/ferrochrome.sh b/tests/ferrochrome/ferrochrome.sh
index c68e8a9..6e23204 100755
--- a/tests/ferrochrome/ferrochrome.sh
+++ b/tests/ferrochrome/ferrochrome.sh
@@ -131,9 +131,12 @@
fi
pkg_name=$(dirname ${resolved_activities})
+current_user=$(adb shell am get-current-user)
-adb shell pm grant ${pkg_name} android.permission.USE_CUSTOM_VIRTUAL_MACHINE > /dev/null
-adb shell pm clear ${pkg_name} > /dev/null
+echo "Reset app & granting permission"
+adb shell pm grant --user ${current_user} ${pkg_name} android.permission.RECORD_AUDIO
+adb shell pm grant --user ${current_user} ${pkg_name} android.permission.USE_CUSTOM_VIRTUAL_MACHINE > /dev/null
+adb shell pm clear --user ${current_user} ${pkg_name} > /dev/null
if [[ -z "${fecr_skip}" ]]; then
if [[ -z "${fecr_dir}" ]]; then
@@ -155,14 +158,10 @@
adb shell svc power stayon true
adb shell wm dismiss-keyguard
-echo "Granting runtime permissions to ensure VmLauncher is focused"
-adb shell pm grant ${pkg_name} android.permission.RECORD_AUDIO
-
echo "Starting ferrochrome"
adb shell am start-activity -a ${ACTION_NAME} > /dev/null
# HSUM aware log path
-current_user=$(adb shell am get-current-user)
log_path="/data/user/${current_user}/${pkg_name}/${FECR_CONSOLE_LOG_PATH}"
fecr_start_time=${EPOCHSECONDS}