virtmgr: Delegate debugVmList to virtualizationservice
The shell user has permission to list all VMs in the system (called from
'vm list'). This stopped working after virtmgr was introduced, as it
only has visibility of its own VMs. Add a new method on
VirtuliazationService_Internal to retrieve this information from
the global VS, which does keep a list of the allocated VM contexts.
Extend VS's info with the requestor's UID and PID to match the original
functionality. The one piece of information missing is the VM's running
state.
Bug: 245727626
Bug: 264826962
Test: adb shell /apex/com.android.virt/vm list
Test: vm/vm_shell.sh start-microdroid --auto-connect
Test: atest MicrodroidHostTestCases
Change-Id: I4cf5d1a9792aca995b63a13c2ee47c6033d9a5d6
diff --git a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
index 617c300..2ee33e6 100644
--- a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
+++ b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
@@ -652,6 +652,8 @@
@Test
@CddTest(requirements = {"9.17/C-1-1", "9.17/C-1-2", "9.17/C/1-3"})
public void testMicrodroidBoots() throws Exception {
+ CommandRunner android = new CommandRunner(getDevice());
+
final String configPath = "assets/vm_config.json"; // path inside the APK
mMicrodroidDevice =
MicrodroidBuilder.fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
@@ -662,6 +664,9 @@
mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT);
CommandRunner microdroid = new CommandRunner(mMicrodroidDevice);
+ String vmList = android.run("/apex/com.android.virt/bin/vm list");
+ assertThat(vmList).contains("requesterUid: " + android.run("id -u"));
+
// Test writing to /data partition
microdroid.run("echo MicrodroidTest > /data/local/tmp/test.txt");
assertThat(microdroid.run("cat /data/local/tmp/test.txt")).isEqualTo("MicrodroidTest");
@@ -680,7 +685,6 @@
assertThat(abis).hasLength(1);
// Check that no denials have happened so far
- CommandRunner android = new CommandRunner(getDevice());
assertThat(android.tryRun("egrep", "'avc:[[:space:]]{1,2}denied'", LOG_PATH)).isNull();
assertThat(android.tryRun("egrep", "'avc:[[:space:]]{1,2}denied'", CONSOLE_PATH)).isNull();
diff --git a/virtualizationservice/aidl/Android.bp b/virtualizationservice/aidl/Android.bp
index f028c0f..91d91aa 100644
--- a/virtualizationservice/aidl/Android.bp
+++ b/virtualizationservice/aidl/Android.bp
@@ -36,7 +36,10 @@
aidl_interface {
name: "android.system.virtualizationservice_internal",
srcs: ["android/system/virtualizationservice_internal/**/*.aidl"],
- imports: ["android.system.virtualizationcommon"],
+ imports: [
+ "android.system.virtualizationcommon",
+ "android.system.virtualizationservice",
+ ],
unstable: true,
backend: {
java: {
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
index 424eec1..870a342 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
@@ -33,7 +33,4 @@
* the PID may have been reused for a different process, so this should not be trusted.
*/
int requesterPid;
-
- /** The current lifecycle state of the VM. */
- VirtualMachineState state = VirtualMachineState.NOT_STARTED;
}
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl b/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
index d6b3536..5422a48 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
@@ -15,6 +15,7 @@
*/
package android.system.virtualizationservice_internal;
+import android.system.virtualizationservice.VirtualMachineDebugInfo;
import android.system.virtualizationservice_internal.AtomVmBooted;
import android.system.virtualizationservice_internal.AtomVmCreationRequested;
import android.system.virtualizationservice_internal.AtomVmExited;
@@ -35,7 +36,7 @@
* The resources will not be recycled as long as there is a strong reference
* to the returned object.
*/
- IGlobalVmContext allocateGlobalVmContext();
+ IGlobalVmContext allocateGlobalVmContext(int requesterDebugPid);
/** Forwards a VmBooted atom to statsd. */
void atomVmBooted(in AtomVmBooted atom);
@@ -45,4 +46,7 @@
/** Forwards a VmExited atom to statsd. */
void atomVmExited(in AtomVmExited atom);
+
+ /** Get a list of all currently running VMs. */
+ VirtualMachineDebugInfo[] debugListVms();
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index f5ada71..8f28289 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -74,7 +74,7 @@
use std::num::NonZeroU32;
use std::os::unix::fs::PermissionsExt;
use std::os::unix::io::{FromRawFd, IntoRawFd};
-use std::os::unix::raw::uid_t;
+use std::os::unix::raw::{pid_t, uid_t};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex, Weak};
use tombstoned_client::{DebuggerdDumpType, TombstonedConnection};
@@ -189,10 +189,14 @@
}
}
- fn allocateGlobalVmContext(&self) -> binder::Result<Strong<dyn IGlobalVmContext>> {
+ fn allocateGlobalVmContext(
+ &self,
+ requester_debug_pid: i32,
+ ) -> binder::Result<Strong<dyn IGlobalVmContext>> {
let requester_uid = get_calling_uid();
+ let requester_debug_pid = requester_debug_pid as pid_t;
let state = &mut *self.state.lock().unwrap();
- state.allocate_vm_context(requester_uid).map_err(|e| {
+ state.allocate_vm_context(requester_uid, requester_debug_pid).map_err(|e| {
Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
})
}
@@ -211,12 +215,32 @@
forward_vm_exited_atom(atom);
Ok(())
}
+
+ fn debugListVms(&self) -> binder::Result<Vec<VirtualMachineDebugInfo>> {
+ let state = &mut *self.state.lock().unwrap();
+ let cids = state
+ .held_contexts
+ .iter()
+ .filter_map(|(_, inst)| Weak::upgrade(inst))
+ .map(|vm| VirtualMachineDebugInfo {
+ cid: vm.cid as i32,
+ temporaryDirectory: vm.get_temp_dir().to_string_lossy().to_string(),
+ requesterUid: vm.requester_uid as i32,
+ requesterPid: vm.requester_debug_pid as i32,
+ })
+ .collect();
+ Ok(cids)
+ }
}
#[derive(Debug, Default)]
struct GlobalVmInstance {
/// The unique CID assigned to the VM for vsock communication.
cid: Cid,
+ /// UID of the client who requested this VM instance.
+ requester_uid: uid_t,
+ /// PID of the client who requested this VM instance.
+ requester_debug_pid: pid_t,
}
impl GlobalVmInstance {
@@ -289,12 +313,13 @@
fn allocate_vm_context(
&mut self,
requester_uid: uid_t,
+ requester_debug_pid: pid_t,
) -> Result<Strong<dyn IGlobalVmContext>> {
// Garbage collect unused VM contexts.
self.held_contexts.retain(|_, instance| instance.strong_count() > 0);
let cid = self.get_next_available_cid()?;
- let instance = Arc::new(GlobalVmInstance { cid });
+ let instance = Arc::new(GlobalVmInstance { cid, requester_uid, requester_debug_pid });
create_temporary_directory(&instance.get_temp_dir(), requester_uid)?;
self.held_contexts.insert(cid, Arc::downgrade(&instance));
@@ -475,20 +500,7 @@
/// and as such is only permitted from the shell user.
fn debugListVms(&self) -> binder::Result<Vec<VirtualMachineDebugInfo>> {
check_debug_access()?;
-
- let state = &mut *self.state.lock().unwrap();
- let vms = state.vms();
- let cids = vms
- .into_iter()
- .map(|vm| VirtualMachineDebugInfo {
- cid: vm.cid as i32,
- temporaryDirectory: vm.temporary_directory.to_string_lossy().to_string(),
- requesterUid: vm.requester_uid as i32,
- requesterPid: vm.requester_debug_pid,
- state: get_state(&vm),
- })
- .collect();
- Ok(cids)
+ GLOBAL_SERVICE.debugListVms()
}
/// Hold a strong reference to a VM in VirtualizationService. This method is only intended for
@@ -570,13 +582,13 @@
VirtualizationService::default()
}
- fn create_vm_context(&self) -> Result<(VmContext, Cid, PathBuf)> {
+ fn create_vm_context(&self, requester_debug_pid: pid_t) -> Result<(VmContext, Cid, PathBuf)> {
const NUM_ATTEMPTS: usize = 5;
for _ in 0..NUM_ATTEMPTS {
- let global_context = GLOBAL_SERVICE.allocateGlobalVmContext()?;
- let cid = global_context.getCid()? as Cid;
- let temp_dir: PathBuf = global_context.getTemporaryDirectory()?.into();
+ let vm_context = GLOBAL_SERVICE.allocateGlobalVmContext(requester_debug_pid as i32)?;
+ let cid = vm_context.getCid()? as Cid;
+ let temp_dir: PathBuf = vm_context.getTemporaryDirectory()?.into();
let service = VirtualMachineService::new_binder(self.state.clone(), cid).as_binder();
// Start VM service listening for connections from the new CID on port=CID.
@@ -584,7 +596,7 @@
match RpcServer::new_vsock(service, cid, port) {
Ok(vm_server) => {
vm_server.start();
- return Ok((VmContext::new(global_context, vm_server), cid, temp_dir));
+ return Ok((VmContext::new(vm_context, vm_server), cid, temp_dir));
}
Err(err) => {
warn!("Could not start RpcServer on port {}: {}", port, err);
@@ -617,19 +629,21 @@
check_use_custom_virtual_machine()?;
}
- let (vm_context, cid, temporary_directory) = self.create_vm_context().map_err(|e| {
- error!("Failed to create VmContext: {:?}", e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to create VmContext: {:?}", e)),
- )
- })?;
+ let requester_uid = get_calling_uid();
+ let requester_debug_pid = get_calling_pid();
+
+ let (vm_context, cid, temporary_directory) =
+ self.create_vm_context(requester_debug_pid).map_err(|e| {
+ error!("Failed to create VmContext: {:?}", e);
+ Status::new_service_specific_error_str(
+ -1,
+ Some(format!("Failed to create VmContext: {:?}", e)),
+ )
+ })?;
let state = &mut *self.state.lock().unwrap();
let console_fd = console_fd.map(clone_file).transpose()?;
let log_fd = log_fd.map(clone_file).transpose()?;
- let requester_uid = get_calling_uid();
- let requester_debug_pid = get_calling_pid();
// Counter to generate unique IDs for temporary image files.
let mut next_temporary_image_id = 0;