Merge changes from topic "microdroid_selinux_denial_fix"
* changes:
Fix vm_payload_service bind address
Don't inherit vm payload socket and stdio from MM
Use console for SELinux denial check
diff --git a/authfs/aidl/com/android/virt/fs/IAuthFsService.aidl b/authfs/aidl/com/android/virt/fs/IAuthFsService.aidl
index b349db2..30cc281 100644
--- a/authfs/aidl/com/android/virt/fs/IAuthFsService.aidl
+++ b/authfs/aidl/com/android/virt/fs/IAuthFsService.aidl
@@ -21,6 +21,8 @@
/** @hide */
interface IAuthFsService {
+ const String AUTHFS_SERVICE_SOCKET_NAME = "authfs_service";
+
/**
* Creates an AuthFS mount given the config. Returns the binder object that represent the AuthFS
* instance. The AuthFS setup is deleted once the lifetime of the returned binder object ends.
diff --git a/authfs/service/Android.bp b/authfs/service/Android.bp
index e9eec1e..de6326d 100644
--- a/authfs/service/Android.bp
+++ b/authfs/service/Android.bp
@@ -16,6 +16,7 @@
"liblibc",
"liblog_rust",
"libnix",
+ "librpcbinder_rs",
"libshared_child",
],
prefer_rlib: true,
diff --git a/authfs/service/authfs_service.rc b/authfs/service/authfs_service.rc
index 9ad0ce6..7edb1ca 100644
--- a/authfs/service/authfs_service.rc
+++ b/authfs/service/authfs_service.rc
@@ -1,2 +1,3 @@
service authfs_service /system/bin/authfs_service
disabled
+ socket authfs_service stream 0666 root system
diff --git a/authfs/service/src/main.rs b/authfs/service/src/main.rs
index 77cac9a..671c06a 100644
--- a/authfs/service/src/main.rs
+++ b/authfs/service/src/main.rs
@@ -22,8 +22,9 @@
mod authfs;
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, Result};
use log::*;
+use rpcbinder::run_init_unix_domain_rpc_server;
use std::ffi::OsString;
use std::fs::{create_dir, read_dir, remove_dir_all, remove_file};
use std::sync::atomic::{AtomicUsize, Ordering};
@@ -31,13 +32,10 @@
use authfs_aidl_interface::aidl::com::android::virt::fs::AuthFsConfig::AuthFsConfig;
use authfs_aidl_interface::aidl::com::android::virt::fs::IAuthFs::IAuthFs;
use authfs_aidl_interface::aidl::com::android::virt::fs::IAuthFsService::{
- BnAuthFsService, IAuthFsService,
+ BnAuthFsService, IAuthFsService, AUTHFS_SERVICE_SOCKET_NAME,
};
-use binder::{
- self, add_service, BinderFeatures, ExceptionCode, Interface, ProcessState, Status, Strong,
-};
+use binder::{self, BinderFeatures, ExceptionCode, Interface, Status, Strong};
-const SERVICE_NAME: &str = "authfs_service";
const SERVICE_ROOT: &str = "/data/misc/authfs";
/// Implementation of `IAuthFsService`.
@@ -117,15 +115,17 @@
clean_up_working_directory()?;
- ProcessState::start_thread_pool();
-
let service = AuthFsService::new_binder(debuggable).as_binder();
- add_service(SERVICE_NAME, service)
- .with_context(|| format!("Failed to register service {}", SERVICE_NAME))?;
- debug!("{} is running", SERVICE_NAME);
-
- ProcessState::join_thread_pool();
- bail!("Unexpected exit after join_thread_pool")
+ debug!("{} is starting as a rpc service.", AUTHFS_SERVICE_SOCKET_NAME);
+ let retval = run_init_unix_domain_rpc_server(service, AUTHFS_SERVICE_SOCKET_NAME, || {
+ info!("The RPC server '{}' is running.", AUTHFS_SERVICE_SOCKET_NAME);
+ });
+ if retval {
+ info!("The RPC server at '{}' has shut down gracefully.", AUTHFS_SERVICE_SOCKET_NAME);
+ Ok(())
+ } else {
+ bail!("Premature termination of the RPC server '{}'.", AUTHFS_SERVICE_SOCKET_NAME)
+ }
}
fn main() {
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index 0e8b9f5..40d14d8 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -30,14 +30,16 @@
use crate::artifact_signer::ArtifactSigner;
use crate::compilation::odrefresh;
use crate::compos_key;
+use authfs_aidl_interface::aidl::com::android::virt::fs::IAuthFsService::{
+ IAuthFsService, AUTHFS_SERVICE_SOCKET_NAME,
+};
use binder::{BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
BnCompOsService, ICompOsService, OdrefreshArgs::OdrefreshArgs,
};
use compos_common::binder::to_binder_result;
use compos_common::odrefresh::{is_system_property_interesting, ODREFRESH_PATH};
-
-const AUTHFS_SERVICE_NAME: &str = "authfs_service";
+use rpcbinder::get_unix_domain_rpc_interface;
/// Constructs a binder object that implements ICompOsService.
pub fn new_binder() -> Result<Strong<dyn ICompOsService>> {
@@ -127,8 +129,10 @@
impl CompOsService {
fn do_odrefresh(&self, args: &OdrefreshArgs) -> Result<i8> {
- let authfs_service = binder::get_interface(AUTHFS_SERVICE_NAME)
- .context("Unable to connect to AuthFS service")?;
+ log::debug!("Prepare to connect to {}", AUTHFS_SERVICE_SOCKET_NAME);
+ let authfs_service: Strong<dyn IAuthFsService> =
+ get_unix_domain_rpc_interface(AUTHFS_SERVICE_SOCKET_NAME)
+ .with_context(|| format!("Failed to connect to {}", AUTHFS_SERVICE_SOCKET_NAME))?;
let exit_code = odrefresh(&self.odrefresh_path, args, authfs_service, |output_dir| {
// authfs only shows us the files we created, so it's ok to just sign everything
// under the output directory.
diff --git a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
index 43fe615..9bcd1d3 100644
--- a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
+++ b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
@@ -100,6 +100,9 @@
public static void testIfDeviceIsCapable(ITestDevice androidDevice) throws Exception {
assumeTrue("Need an actual TestDevice", androidDevice instanceof TestDevice);
TestDevice testDevice = (TestDevice) androidDevice;
+ assumeTrue(
+ "Requires VM support",
+ testDevice.hasFeature("android.software.virtualization_framework"));
assumeTrue("Requires VM support", testDevice.supportsMicrodroid());
}
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 5e86798..eb719b8 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -601,6 +601,36 @@
}
@Test
+ public void importedVmAndOriginalVmHaveTheSameCdi() throws Exception {
+ assumeSupportedKernel();
+ // Arrange
+ grantPermission(VirtualMachine.USE_CUSTOM_VIRTUAL_MACHINE_PERMISSION);
+ VirtualMachineConfig config =
+ mInner.newVmConfigBuilder()
+ .setPayloadConfigPath("assets/vm_config.json")
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .build();
+ String vmNameOrig = "test_vm_orig";
+ String vmNameImport = "test_vm_import";
+ VirtualMachine vmOrig = mInner.forceCreateNewVirtualMachine(vmNameOrig, config);
+ VmCdis origCdis = launchVmAndGetCdis(vmNameOrig);
+ assertThat(origCdis.instanceSecret).isNotNull();
+ VirtualMachineDescriptor descriptor = vmOrig.toDescriptor();
+ VirtualMachineManager vmm = mInner.getVirtualMachineManager();
+ if (vmm.get(vmNameImport) != null) {
+ vmm.delete(vmNameImport);
+ }
+
+ // Action
+ // The imported VM will be fetched by name later.
+ VirtualMachine unusedVmImport = vmm.importFromDescriptor(vmNameImport, descriptor);
+
+ // Asserts
+ VmCdis importCdis = launchVmAndGetCdis(vmNameImport);
+ assertThat(origCdis.instanceSecret).isEqualTo(importCdis.instanceSecret);
+ }
+
+ @Test
public void importedVmIsEqualToTheOriginalVm() throws Exception {
// Arrange
VirtualMachineConfig config =
@@ -608,7 +638,8 @@
.setPayloadBinaryPath("MicrodroidTestNativeLib.so")
.setDebugLevel(DEBUG_LEVEL_NONE)
.build();
- String vmNameOrig = "test_vm_orig", vmNameImport = "test_vm_import";
+ String vmNameOrig = "test_vm_orig";
+ String vmNameImport = "test_vm_import";
VirtualMachine vmOrig = mInner.forceCreateNewVirtualMachine(vmNameOrig, config);
// Run something to make the instance.img different with the initialized one.
TestResults origTestResults = runVmTestService(vmOrig);
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index a8f68bc..30b89da 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -1049,8 +1049,8 @@
})?;
vm.callbacks.notify_payload_started(cid);
- let vm_start_timestamp = vm.vm_start_timestamp.lock().unwrap();
- write_vm_booted_stats(vm.requester_uid as i32, &vm.name, *vm_start_timestamp);
+ let vm_start_timestamp = vm.vm_metric.lock().unwrap().start_timestamp;
+ write_vm_booted_stats(vm.requester_uid as i32, &vm.name, vm_start_timestamp);
Ok(())
} else {
error!("notifyPayloadStarted is called from an unknown CID {}", cid);
diff --git a/virtualizationservice/src/atom.rs b/virtualizationservice/src/atom.rs
index 20f88e7..9c74d1e 100644
--- a/virtualizationservice/src/atom.rs
+++ b/virtualizationservice/src/atom.rs
@@ -15,6 +15,7 @@
//! Functions for creating and collecting atoms.
use crate::aidl::clone_file;
+use crate::crosvm::VmMetric;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
DeathReason::DeathReason,
IVirtualMachine::IVirtualMachine,
@@ -164,15 +165,18 @@
uid: i32,
vm_identifier: &str,
reason: DeathReason,
- vm_start_timestamp: Option<SystemTime>,
+ vm_metric: &VmMetric,
) {
let vm_identifier = vm_identifier.to_owned();
- let duration = get_duration(vm_start_timestamp);
+ let elapsed_time_millis = get_duration(vm_metric.start_timestamp).as_millis() as i64;
+ let guest_time_millis = vm_metric.cpu_guest_time.unwrap_or_default();
+ let rss = vm_metric.rss.unwrap_or_default();
+
thread::spawn(move || {
let vm_exited = vm_exited::VmExited {
uid,
vm_identifier: &vm_identifier,
- elapsed_time_millis: duration.as_millis() as i64,
+ elapsed_time_millis,
death_reason: match reason {
DeathReason::INFRASTRUCTURE_ERROR => vm_exited::DeathReason::InfrastructureError,
DeathReason::KILLED => vm_exited::DeathReason::Killed,
@@ -211,6 +215,9 @@
DeathReason::HANGUP => vm_exited::DeathReason::Hangup,
_ => vm_exited::DeathReason::Unknown,
},
+ guest_time_millis,
+ rss_vm_kb: rss.vm,
+ rss_crosvm_kb: rss.crosvm,
};
wait_for_statsd().unwrap_or_else(|e| warn!("failed to wait for statsd with error: {}", e));
match vm_exited.stats_write() {
diff --git a/virtualizationservice/src/crosvm.rs b/virtualizationservice/src/crosvm.rs
index 29040b7..db6da43 100644
--- a/virtualizationservice/src/crosvm.rs
+++ b/virtualizationservice/src/crosvm.rs
@@ -16,16 +16,18 @@
use crate::aidl::{Cid, VirtualMachineCallbacks};
use crate::atom::write_vm_exited_stats;
-use anyhow::{anyhow, bail, Context, Error};
+use anyhow::{anyhow, bail, Context, Error, Result};
use command_fds::CommandFdExt;
use lazy_static::lazy_static;
+use libc::{sysconf, _SC_CLK_TCK};
use log::{debug, error, info};
use semver::{Version, VersionReq};
use nix::{fcntl::OFlag, unistd::pipe2};
use regex::{Captures, Regex};
use shared_child::SharedChild;
use std::borrow::Cow;
-use std::fs::{remove_dir_all, File};
+use std::cmp::max;
+use std::fs::{read_to_string, remove_dir_all, File};
use std::io::{self, Read};
use std::mem;
use std::num::NonZeroU32;
@@ -60,6 +62,8 @@
/// The exit status which crosvm returns when vcpu is stalled.
const CROSVM_WATCHDOG_REBOOT_STATUS: i32 = 36;
+const MILLIS_PER_SEC: i64 = 1000;
+
lazy_static! {
/// If the VM doesn't move to the Started state within this amount time, a hang-up error is
/// triggered.
@@ -132,6 +136,24 @@
Failed,
}
+/// RSS values of VM and CrosVM process itself.
+#[derive(Copy, Clone, Debug, Default)]
+pub struct Rss {
+ pub vm: i64,
+ pub crosvm: i64,
+}
+
+/// Metrics regarding the VM.
+#[derive(Debug, Default)]
+pub struct VmMetric {
+ /// Recorded timestamp when the VM is started.
+ pub start_timestamp: Option<SystemTime>,
+ /// Update most recent guest_time periodically from /proc/[crosvm pid]/stat while VM is running.
+ pub cpu_guest_time: Option<i64>,
+ /// Update maximum RSS values periodically from /proc/[crosvm pid]/smaps while VM is running.
+ pub rss: Option<Rss>,
+}
+
impl VmState {
/// Tries to start the VM, if it is in the `NotStarted` state.
///
@@ -146,6 +168,12 @@
let child =
Arc::new(run_vm(config, &instance.temporary_directory, failure_pipe_write)?);
+ let instance_monitor_status = instance.clone();
+ let child_monitor_status = child.clone();
+ thread::spawn(move || {
+ instance_monitor_status.clone().monitor_vm_status(child_monitor_status);
+ });
+
let child_clone = child.clone();
let instance_clone = instance.clone();
thread::spawn(move || {
@@ -191,8 +219,8 @@
pub callbacks: VirtualMachineCallbacks,
/// VirtualMachineService binder object for the VM.
pub vm_service: Mutex<Option<Strong<dyn IVirtualMachineService>>>,
- /// Recorded timestamp when the VM is started.
- pub vm_start_timestamp: Mutex<Option<SystemTime>>,
+ /// Recorded metrics of VM such as timestamp or cpu / memory usage.
+ pub vm_metric: Mutex<VmMetric>,
/// The latest lifecycle state which the payload reported itself to be in.
payload_state: Mutex<PayloadState>,
/// Represents the condition that payload_state was updated
@@ -221,7 +249,7 @@
requester_debug_pid,
callbacks: Default::default(),
vm_service: Mutex::new(None),
- vm_start_timestamp: Mutex::new(None),
+ vm_metric: Mutex::new(Default::default()),
payload_state: Mutex::new(PayloadState::Starting),
payload_state_updated: Condvar::new(),
})
@@ -230,7 +258,8 @@
/// Starts an instance of `crosvm` to manage the VM. The `crosvm` instance will be killed when
/// the `VmInstance` is dropped.
pub fn start(self: &Arc<Self>) -> Result<(), Error> {
- *self.vm_start_timestamp.lock().unwrap() = Some(SystemTime::now());
+ let mut vm_metric = self.vm_metric.lock().unwrap();
+ vm_metric.start_timestamp = Some(SystemTime::now());
self.vm_state.lock().unwrap().start(self.clone())
}
@@ -279,13 +308,8 @@
let death_reason = death_reason(&result, &failure_reason);
self.callbacks.callback_on_died(self.cid, death_reason);
- let vm_start_timestamp = self.vm_start_timestamp.lock().unwrap();
- write_vm_exited_stats(
- self.requester_uid as i32,
- &self.name,
- death_reason,
- *vm_start_timestamp,
- );
+ let vm_metric = self.vm_metric.lock().unwrap();
+ write_vm_exited_stats(self.requester_uid as i32, &self.name, death_reason, &*vm_metric);
// Delete temporary files.
if let Err(e) = remove_dir_all(&self.temporary_directory) {
@@ -317,6 +341,42 @@
}
}
+ fn monitor_vm_status(&self, child: Arc<SharedChild>) {
+ let pid = child.id();
+
+ loop {
+ {
+ // Check VM state
+ let vm_state = &*self.vm_state.lock().unwrap();
+ if let VmState::Dead = vm_state {
+ break;
+ }
+
+ let mut vm_metric = self.vm_metric.lock().unwrap();
+
+ // Get CPU Information
+ // TODO: Collect it once right before VM dies using SIGCHLD
+ if let Ok(guest_time) = get_guest_time(pid) {
+ vm_metric.cpu_guest_time = Some(guest_time);
+ } else {
+ error!("Failed to parse /proc/[pid]/stat");
+ }
+
+ // Get Memory Information
+ if let Ok(rss) = get_rss(pid) {
+ vm_metric.rss = match &vm_metric.rss {
+ Some(x) => Some(Rss::extract_max(x, &rss)),
+ None => Some(rss),
+ }
+ } else {
+ error!("Failed to parse /proc/[pid]/smaps");
+ }
+ }
+
+ thread::sleep(Duration::from_secs(1));
+ }
+ }
+
/// Returns the last reported state of the VM payload.
pub fn payload_state(&self) -> PayloadState {
*self.payload_state.lock().unwrap()
@@ -387,6 +447,60 @@
}
}
+impl Rss {
+ fn extract_max(x: &Rss, y: &Rss) -> Rss {
+ Rss { vm: max(x.vm, y.vm), crosvm: max(x.crosvm, y.crosvm) }
+ }
+}
+
+// Get guest time from /proc/[crosvm pid]/stat
+fn get_guest_time(pid: u32) -> Result<i64> {
+ let file = read_to_string(format!("/proc/{}/stat", pid))?;
+ let data_list: Vec<_> = file.split_whitespace().collect();
+
+ // Information about guest_time is at 43th place of the file split with the whitespace.
+ // Example of /proc/[pid]/stat :
+ // 6603 (kworker/104:1H-kblockd) I 2 0 0 0 -1 69238880 0 0 0 0 0 88 0 0 0 -20 1 0 1845 0 0
+ // 18446744073709551615 0 0 0 0 0 0 0 2147483647 0 0 0 0 17 104 0 0 0 0 0 0 0 0 0 0 0 0 0
+ if data_list.len() < 43 {
+ bail!("Failed to parse command result for getting guest time : {}", file);
+ }
+
+ let guest_time_ticks = data_list[42].parse::<i64>()?;
+ // SAFETY : It just returns an integer about CPU tick information.
+ let ticks_per_sec = unsafe { sysconf(_SC_CLK_TCK) } as i64;
+ Ok(guest_time_ticks * MILLIS_PER_SEC / ticks_per_sec)
+}
+
+// Get rss from /proc/[crosvm pid]/smaps
+fn get_rss(pid: u32) -> Result<Rss> {
+ let file = read_to_string(format!("/proc/{}/smaps", pid))?;
+ let lines: Vec<_> = file.split('\n').collect();
+
+ let mut rss_vm_total = 0i64;
+ let mut rss_crosvm_total = 0i64;
+ let mut is_vm = false;
+ for line in lines {
+ if line.contains("crosvm_guest") {
+ is_vm = true;
+ } else if line.contains("Rss:") {
+ let data_list: Vec<_> = line.split_whitespace().collect();
+ if data_list.len() < 2 {
+ bail!("Failed to parse command result for getting rss :\n{}", line);
+ }
+ let rss = data_list[1].parse::<i64>()?;
+
+ if is_vm {
+ rss_vm_total += rss;
+ is_vm = false;
+ }
+ rss_crosvm_total += rss;
+ }
+ }
+
+ Ok(Rss { vm: rss_vm_total, crosvm: rss_crosvm_total })
+}
+
fn death_reason(result: &Result<ExitStatus, io::Error>, mut failure_reason: &str) -> DeathReason {
if let Some(position) = failure_reason.find('|') {
// Separator indicates extra context information is present after the failure name.
diff --git a/vmbase/example/Android.bp b/vmbase/example/Android.bp
index e9a3f98..0f1e66a 100644
--- a/vmbase/example/Android.bp
+++ b/vmbase/example/Android.bp
@@ -54,10 +54,11 @@
edition: "2021",
rustlibs: [
"android.system.virtualizationservice-rust",
+ "libandroid_logger",
"libanyhow",
- "libenv_logger",
"liblibc",
"liblog_rust",
+ "libnix",
"libvmclient",
],
data: [
diff --git a/vmbase/example/tests/test.rs b/vmbase/example/tests/test.rs
index 85e0213..57b68ed 100644
--- a/vmbase/example/tests/test.rs
+++ b/vmbase/example/tests/test.rs
@@ -25,8 +25,9 @@
use log::info;
use std::{
fs::File,
- io,
- os::unix::io::{AsRawFd, FromRawFd},
+ io::{self, BufRead, BufReader},
+ os::unix::io::FromRawFd,
+ panic, thread,
};
use vmclient::{DeathReason, VmInstance};
@@ -36,7 +37,14 @@
/// Runs the vmbase_example VM as an unprotected VM via VirtualizationService.
#[test]
fn test_run_example_vm() -> Result<(), Error> {
- env_logger::init();
+ android_logger::init_once(
+ android_logger::Config::default().with_tag("vmbase").with_min_level(log::Level::Debug),
+ );
+
+ // Redirect panic messages to logcat.
+ panic::set_hook(Box::new(|panic_info| {
+ log::error!("{}", panic_info);
+ }));
// We need to start the thread pool for Binder to work properly, especially link_to_death.
ProcessState::start_thread_pool();
@@ -62,8 +70,8 @@
platformVersion: "~1.0".to_string(),
taskProfiles: vec![],
});
- let console = duplicate_stdout()?;
- let log = duplicate_stdout()?;
+ let console = android_log_fd()?;
+ let log = android_log_fd()?;
let vm = VmInstance::create(service.as_ref(), &config, Some(console), Some(log), None)
.context("Failed to create VM")?;
vm.start().context("Failed to start VM")?;
@@ -76,17 +84,17 @@
Ok(())
}
-/// Safely duplicate the standard output file descriptor.
-fn duplicate_stdout() -> io::Result<File> {
- let stdout_fd = io::stdout().as_raw_fd();
- // Safe because this just duplicates a file descriptor which we know to be valid, and we check
- // for an error.
- let dup_fd = unsafe { libc::dup(stdout_fd) };
- if dup_fd < 0 {
- Err(io::Error::last_os_error())
- } else {
- // Safe because we have just duplicated the file descriptor so we own it, and `from_raw_fd`
- // takes ownership of it.
- Ok(unsafe { File::from_raw_fd(dup_fd) })
- }
+fn android_log_fd() -> io::Result<File> {
+ let (reader_fd, writer_fd) = nix::unistd::pipe()?;
+
+ // SAFETY: These are new FDs with no previous owner.
+ let reader = unsafe { File::from_raw_fd(reader_fd) };
+ let writer = unsafe { File::from_raw_fd(writer_fd) };
+
+ thread::spawn(|| {
+ for line in BufReader::new(reader).lines() {
+ info!("{}", line.unwrap());
+ }
+ });
+ Ok(writer)
}