Remove onPayloadStarted stream argument
microdroid_manager creates a vsock connection with the host and
redirects the payload's stdin/stdout/stderr streams over it. This may
not necessarily be a securiy issue if the app never writes any secrets
to its standard output, but it would be safer to not open up
a communication channel like that by default. If the payload wishes to
pass its logs to the host, it should open up the connection explicitly.
Remove the vsock connection, the virtualizationservice server and the
corresponding file descriptor argument of onPayloadStarted() callback.
Instead, provide onPayloadStdio() that the payload can optinally call
to set up the connection.
Bug: 245727626
Bug: 253221932
Test: atest -p packages/modules/Virtualization:avf-presubmit
Change-Id: I89fd3a52aae9272db7300224b88d87c6f4d8a8a7
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index c18dd26..762a149 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -26,7 +26,7 @@
use crate::vm_payload_service::register_vm_payload_service;
use android_system_virtualizationcommon::aidl::android::system::virtualizationcommon::ErrorCode::ErrorCode;
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
- IVirtualMachineService, VM_BINDER_SERVICE_PORT, VM_STREAM_SERVICE_PORT,
+ IVirtualMachineService, VM_BINDER_SERVICE_PORT,
};
use android_system_virtualization_payload::aidl::android::system::virtualization::payload::IVmPayloadService::VM_APK_CONTENTS_PATH;
use anyhow::{anyhow, bail, ensure, Context, Error, Result};
@@ -49,15 +49,13 @@
use std::borrow::Cow::{Borrowed, Owned};
use std::convert::TryInto;
use std::env;
-use std::fs::{self, create_dir, File, OpenOptions};
+use std::fs::{self, create_dir, OpenOptions};
use std::io::Write;
-use std::os::unix::io::{FromRawFd, IntoRawFd};
use std::os::unix::process::ExitStatusExt;
use std::path::Path;
use std::process::{Child, Command, Stdio};
use std::str;
use std::time::{Duration, SystemTime};
-use vsock::VsockStream;
const WAIT_TIMEOUT: Duration = Duration::from_secs(10);
const MAIN_APK_PATH: &str = "/dev/block/by-name/microdroid-apk";
@@ -732,7 +730,14 @@
/// virtualizationservice in the host side.
fn exec_task(task: &Task, service: &Strong<dyn IVirtualMachineService>) -> Result<i32> {
info!("executing main task {:?}...", task);
- let mut command = build_command(task)?;
+ let mut command = match task.type_ {
+ TaskType::Executable => Command::new(&task.command),
+ TaskType::MicrodroidLauncher => {
+ let mut command = Command::new("/system/bin/microdroid_launcher");
+ command.arg(find_library_path(&task.command)?);
+ command
+ }
+ };
info!("notifying payload started");
service.notifyPayloadStarted()?;
@@ -751,40 +756,6 @@
}
}
-fn build_command(task: &Task) -> Result<Command> {
- let mut command = match task.type_ {
- TaskType::Executable => Command::new(&task.command),
- TaskType::MicrodroidLauncher => {
- let mut command = Command::new("/system/bin/microdroid_launcher");
- command.arg(find_library_path(&task.command)?);
- command
- }
- };
-
- match VsockStream::connect_with_cid_port(VMADDR_CID_HOST, VM_STREAM_SERVICE_PORT as u32) {
- Ok(stream) => {
- // SAFETY: the ownership of the underlying file descriptor is transferred from stream
- // to the file object, and then into the Command object. When the command is finished,
- // the file descriptor is closed.
- let file = unsafe { File::from_raw_fd(stream.into_raw_fd()) };
- command
- .stdin(Stdio::from(file.try_clone()?))
- .stdout(Stdio::from(file.try_clone()?))
- .stderr(Stdio::from(file));
- }
- Err(e) => {
- error!("failed to connect to virtualization service: {}", e);
- // Don't fail hard here. Even if we failed to connect to the virtualizationservice,
- // we keep executing the task. This can happen if the owner of the VM doesn't register
- // callback to accept the stream. Use /dev/null as the stream so that the task can
- // make progress without waiting for someone to consume the output.
- command.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null());
- }
- }
-
- Ok(command)
-}
-
fn find_library_path(name: &str) -> Result<String> {
let mut watcher = PropertyWatcher::new("ro.product.cpu.abilist")?;
let value = watcher.read(|_name, value| Ok(value.trim().to_string()))?;