Use new OdrefreshArgs in AIDL to simplify arguments

Using the parcelable directly simplies the arguments. Although it does
have two problems in the current form:
 * Naming convension (unless we can break the rule of AIDL's)
 * No Option<>.  Optional argument requires the old-school check for
   reserved/invalid value.

Bug: 248528901
Test: atest ComposHostTestCases
Change-Id: Ib87e9b1f998e803756d5a06aadcb92b8a3e2b8f4
This commit is contained in:
Victor Hsieh 2022-09-23 16:22:28 -07:00
parent 322a7b492e
commit e7698673d4
4 changed files with 99 additions and 137 deletions

View File

@ -42,28 +42,41 @@ interface ICompOsService {
TEST_COMPILE = 1,
}
/** Arguments to run odrefresh */
parcelable OdrefreshArgs {
/** The type of compilation to be performed */
CompilationMode compilationMode = CompilationMode.NORMAL_COMPILE;
/** An fd referring to /system */
int systemDirFd = -1;
/** An optional fd referring to /system_ext. Negative number means none. */
int systemExtDirFd = -1;
/** An fd referring to the output directory, ART_APEX_DATA */
int outputDirFd = -1;
/** An fd referring to the staging directory, e.g. ART_APEX_DATA/staging */
int stagingDirFd = -1;
/**
* The sub-directory of the output directory to which artifacts are to be written (e.g.
* dalvik-cache)
*/
String targetDirName;
/** The zygote architecture (ro.zygote) */
String zygoteArch;
/** The compiler filter used to compile system server */
String systemServerCompilerFilter;
}
/**
* Run odrefresh in the VM context.
*
* The execution is based on the VM's APEX mounts, files on Android's /system and optionally
* /system_ext (by accessing through systemDirFd and systemExtDirFd over AuthFS), and
* *CLASSPATH derived in the VM, to generate the same odrefresh output artifacts to the output
* directory (through outputDirFd).
* /system_ext (by accessing through OdrefreshArgs.systemDirFd and OdrefreshArgs.systemExtDirFd
* over AuthFS), and *CLASSPATH derived in the VM, to generate the same odrefresh output
* artifacts to the output directory (through OdrefreshArgs.outputDirFd).
*
* @param compilationMode The type of compilation to be performed
* @param systemDirFd An fd referring to /system
* @param systemExtDirFd An optional fd referring to /system_ext. Negative number means none.
* @param outputDirFd An fd referring to the output directory, ART_APEX_DATA
* @param stagingDirFd An fd referring to the staging directory, e.g. ART_APEX_DATA/staging
* @param targetDirName The sub-directory of the output directory to which artifacts are to be
* written (e.g. dalvik-cache)
* @param zygoteArch The zygote architecture (ro.zygote)
* @param systemServerCompilerFilter The compiler filter used to compile system server
* @param args Arguments to configure the odrefresh context
* @return odrefresh exit code
*/
byte odrefresh(CompilationMode compilation_mode, int systemDirFd, int systemExtDirFd,
int outputDirFd, int stagingDirFd, String targetDirName, String zygoteArch,
String systemServerCompilerFilter);
byte odrefresh(in OdrefreshArgs args);
/**
* Returns the current VM's signing key, as an Ed25519 public key

View File

@ -25,7 +25,7 @@ use android_system_composd::aidl::android::system::composd::{
use anyhow::{Context, Result};
use binder::{Interface, Result as BinderResult, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
CompilationMode::CompilationMode, ICompOsService,
CompilationMode::CompilationMode, ICompOsService, OdrefreshArgs::OdrefreshArgs,
};
use compos_common::odrefresh::{
is_system_property_interesting, ExitCode, ODREFRESH_OUTPUT_ROOT_DIR,
@ -180,16 +180,18 @@ fn run_in_vm(
let zygote_arch = system_properties::read("ro.zygote")?.context("ro.zygote not set")?;
let system_server_compiler_filter =
system_properties::read("dalvik.vm.systemservercompilerfilter")?.unwrap_or_default();
let exit_code = service.odrefresh(
compilation_mode,
system_dir_raw_fd,
system_ext_dir_raw_fd,
output_dir_raw_fd,
staging_dir_raw_fd,
target_dir_name,
&zygote_arch,
&system_server_compiler_filter,
)?;
let args = OdrefreshArgs {
compilationMode: compilation_mode,
systemDirFd: system_dir_raw_fd,
systemExtDirFd: system_ext_dir_raw_fd,
outputDirFd: output_dir_raw_fd,
stagingDirFd: staging_dir_raw_fd,
targetDirName: target_dir_name.to_string(),
zygoteArch: zygote_arch,
systemServerCompilerFilter: system_server_compiler_filter,
};
let exit_code = service.odrefresh(&args)?;
drop(fd_server_raii);
ExitCode::from_i32(exit_code.into())

View File

@ -33,95 +33,65 @@ use authfs_aidl_interface::aidl::com::android::virt::fs::{
IAuthFsService::IAuthFsService,
};
use binder::Strong;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::CompilationMode::CompilationMode;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
CompilationMode::CompilationMode, OdrefreshArgs::OdrefreshArgs,
};
use compos_common::odrefresh::ExitCode;
const FD_SERVER_PORT: i32 = 3264; // TODO: support dynamic port
pub struct OdrefreshContext<'a> {
compilation_mode: CompilationMode,
system_dir_fd: i32,
system_ext_dir_fd: Option<i32>,
output_dir_fd: i32,
staging_dir_fd: i32,
target_dir_name: &'a str,
zygote_arch: &'a str,
system_server_compiler_filter: &'a str,
}
impl<'a> OdrefreshContext<'a> {
#[allow(clippy::too_many_arguments)]
pub fn new(
compilation_mode: CompilationMode,
system_dir_fd: i32,
system_ext_dir_fd: Option<i32>,
output_dir_fd: i32,
staging_dir_fd: i32,
target_dir_name: &'a str,
zygote_arch: &'a str,
system_server_compiler_filter: &'a str,
) -> Result<Self> {
if compilation_mode != CompilationMode::NORMAL_COMPILE {
// Conservatively check debuggability.
let debuggable =
system_properties::read_bool("ro.boot.microdroid.app_debuggable", false)
.unwrap_or(false);
if !debuggable {
bail!("Requested compilation mode only available in debuggable VMs");
}
fn validate_args(args: &OdrefreshArgs) -> Result<()> {
if args.compilationMode != CompilationMode::NORMAL_COMPILE {
// Conservatively check debuggability.
let debuggable = system_properties::read_bool("ro.boot.microdroid.app_debuggable", false)
.unwrap_or(false);
if !debuggable {
bail!("Requested compilation mode only available in debuggable VMs");
}
if system_dir_fd < 0 || output_dir_fd < 0 || staging_dir_fd < 0 {
bail!("The remote FDs are expected to be non-negative");
}
if !matches!(zygote_arch, "zygote64" | "zygote64_32") {
bail!("Invalid zygote arch");
}
// Disallow any sort of path traversal
if target_dir_name.contains(path::MAIN_SEPARATOR) {
bail!("Invalid target directory {}", target_dir_name);
}
// We're not validating/allowlisting the compiler filter, and just assume the compiler will
// reject an invalid string. We need to accept "verify" filter anyway, and potential
// performance degration by the attacker is not currently in scope. This also allows ART to
// specify new compiler filter and configure through system property without change to
// CompOS.
Ok(Self {
compilation_mode,
system_dir_fd,
system_ext_dir_fd,
output_dir_fd,
staging_dir_fd,
target_dir_name,
zygote_arch,
system_server_compiler_filter,
})
}
if args.systemDirFd < 0 || args.outputDirFd < 0 || args.stagingDirFd < 0 {
bail!("The remote FDs are expected to be non-negative");
}
if !matches!(&args.zygoteArch[..], "zygote64" | "zygote64_32") {
bail!("Invalid zygote arch");
}
// Disallow any sort of path traversal
if args.targetDirName.contains(path::MAIN_SEPARATOR) {
bail!("Invalid target directory {}", args.targetDirName);
}
// We're not validating/allowlisting the compiler filter, and just assume the compiler will
// reject an invalid string. We need to accept "verify" filter anyway, and potential
// performance degration by the attacker is not currently in scope. This also allows ART to
// specify new compiler filter and configure through system property without change to
// CompOS.
Ok(())
}
pub fn odrefresh<F>(
odrefresh_path: &Path,
context: OdrefreshContext,
args: &OdrefreshArgs,
authfs_service: Strong<dyn IAuthFsService>,
success_fn: F,
) -> Result<ExitCode>
where
F: FnOnce(PathBuf) -> Result<()>,
{
validate_args(args)?;
// Mount authfs (via authfs_service). The authfs instance unmounts once the `authfs` variable
// is out of scope.
let mut input_dir_fd_annotations = vec![InputDirFdAnnotation {
fd: context.system_dir_fd,
fd: args.systemDirFd,
// Use the 0th APK of the extra_apks in compos/apk/assets/vm_config*.json
manifestPath: "/mnt/extra-apk/0/assets/build_manifest.pb".to_string(),
prefix: "system/".to_string(),
}];
if let Some(fd) = context.system_ext_dir_fd {
if args.systemExtDirFd >= 0 {
input_dir_fd_annotations.push(InputDirFdAnnotation {
fd,
fd: args.systemExtDirFd,
// Use the 1st APK of the extra_apks in compos/apk/assets/vm_config_system_ext_*.json
manifestPath: "/mnt/extra-apk/1/assets/build_manifest.pb".to_string(),
prefix: "system_ext/".to_string(),
@ -132,8 +102,8 @@ where
port: FD_SERVER_PORT,
inputDirFdAnnotations: input_dir_fd_annotations,
outputDirFdAnnotations: vec![
OutputDirFdAnnotation { fd: context.output_dir_fd },
OutputDirFdAnnotation { fd: context.staging_dir_fd },
OutputDirFdAnnotation { fd: args.outputDirFd },
OutputDirFdAnnotation { fd: args.stagingDirFd },
],
..Default::default()
};
@ -144,52 +114,50 @@ where
let mut odrefresh_vars = EnvMap::from_current_env();
let mut android_root = mountpoint.clone();
android_root.push(context.system_dir_fd.to_string());
android_root.push(args.systemDirFd.to_string());
android_root.push("system");
odrefresh_vars.set("ANDROID_ROOT", path_to_str(&android_root)?);
debug!("ANDROID_ROOT={:?}", &android_root);
if let Some(fd) = context.system_ext_dir_fd {
if args.systemExtDirFd >= 0 {
let mut system_ext_root = mountpoint.clone();
system_ext_root.push(fd.to_string());
system_ext_root.push(args.systemExtDirFd.to_string());
system_ext_root.push("system_ext");
odrefresh_vars.set("SYSTEM_EXT_ROOT", path_to_str(&system_ext_root)?);
debug!("SYSTEM_EXT_ROOT={:?}", &system_ext_root);
}
let art_apex_data = mountpoint.join(context.output_dir_fd.to_string());
let art_apex_data = mountpoint.join(args.outputDirFd.to_string());
odrefresh_vars.set("ART_APEX_DATA", path_to_str(&art_apex_data)?);
debug!("ART_APEX_DATA={:?}", &art_apex_data);
let staging_dir = mountpoint.join(context.staging_dir_fd.to_string());
let staging_dir = mountpoint.join(args.stagingDirFd.to_string());
set_classpaths(&mut odrefresh_vars, &android_root)?;
let mut args = vec![
let mut command_line_args = vec![
"odrefresh".to_string(),
"--compilation-os-mode".to_string(),
format!("--zygote-arch={}", context.zygote_arch),
format!("--dalvik-cache={}", context.target_dir_name),
format!("--zygote-arch={}", args.zygoteArch),
format!("--dalvik-cache={}", args.targetDirName),
format!("--staging-dir={}", staging_dir.display()),
"--no-refresh".to_string(),
];
if !context.system_server_compiler_filter.is_empty() {
args.push(format!(
"--system-server-compiler-filter={}",
context.system_server_compiler_filter
));
if !args.systemServerCompilerFilter.is_empty() {
command_line_args
.push(format!("--system-server-compiler-filter={}", args.systemServerCompilerFilter));
}
let compile_flag = match context.compilation_mode {
let compile_flag = match args.compilationMode {
CompilationMode::NORMAL_COMPILE => "--compile",
CompilationMode::TEST_COMPILE => "--force-compile",
other => bail!("Unknown compilation mode {:?}", other),
};
args.push(compile_flag.to_string());
command_line_args.push(compile_flag.to_string());
debug!("Running odrefresh with args: {:?}", &args);
let jail = spawn_jailed_task(odrefresh_path, &args, &odrefresh_vars.into_env())
debug!("Running odrefresh with args: {:?}", &command_line_args);
let jail = spawn_jailed_task(odrefresh_path, &command_line_args, &odrefresh_vars.into_env())
.context("Spawn odrefresh")?;
let exit_code = match jail.wait() {
Ok(_) => 0,
@ -201,7 +169,7 @@ where
info!("odrefresh exited with {:?}", exit_code);
if exit_code == ExitCode::CompilationSuccess {
let target_dir = art_apex_data.join(context.target_dir_name);
let target_dir = art_apex_data.join(&args.targetDirName);
success_fn(target_dir)?;
}

View File

@ -28,11 +28,11 @@ use std::path::{Path, PathBuf};
use std::sync::RwLock;
use crate::artifact_signer::ArtifactSigner;
use crate::compilation::{odrefresh, OdrefreshContext};
use crate::compilation::odrefresh;
use crate::compos_key;
use binder::{BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
BnCompOsService, CompilationMode::CompilationMode, ICompOsService,
BnCompOsService, ICompOsService, OdrefreshArgs::OdrefreshArgs,
};
use compos_common::binder::to_binder_result;
use compos_common::odrefresh::{is_system_property_interesting, ODREFRESH_PATH};
@ -98,17 +98,7 @@ impl ICompOsService for CompOsService {
Ok(())
}
fn odrefresh(
&self,
compilation_mode: CompilationMode,
system_dir_fd: i32,
system_ext_dir_fd: i32,
output_dir_fd: i32,
staging_dir_fd: i32,
target_dir_name: &str,
zygote_arch: &str,
system_server_compiler_filter: &str,
) -> BinderResult<i8> {
fn odrefresh(&self, args: &OdrefreshArgs) -> BinderResult<i8> {
let initialized = *self.initialized.read().unwrap();
if !initialized.unwrap_or(false) {
return Err(Status::new_exception_str(
@ -117,18 +107,7 @@ impl ICompOsService for CompOsService {
));
}
let context = OdrefreshContext::new(
compilation_mode,
system_dir_fd,
if system_ext_dir_fd >= 0 { Some(system_ext_dir_fd) } else { None },
output_dir_fd,
staging_dir_fd,
target_dir_name,
zygote_arch,
system_server_compiler_filter,
);
to_binder_result(context.and_then(|c| self.do_odrefresh(c)))
to_binder_result(self.do_odrefresh(args))
}
fn getPublicKey(&self) -> BinderResult<Vec<u8>> {
@ -147,10 +126,10 @@ impl ICompOsService for CompOsService {
}
impl CompOsService {
fn do_odrefresh(&self, context: OdrefreshContext) -> Result<i8> {
fn do_odrefresh(&self, args: &OdrefreshArgs) -> Result<i8> {
let authfs_service = binder::get_interface(AUTHFS_SERVICE_NAME)
.context("Unable to connect to AuthFS service")?;
let exit_code = odrefresh(&self.odrefresh_path, context, authfs_service, |output_dir| {
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.
let mut artifact_signer = ArtifactSigner::new(&output_dir);