diff --git a/compos/aidl/com/android/compos/ICompOsService.aidl b/compos/aidl/com/android/compos/ICompOsService.aidl index bfde6b68..df8c91ec 100644 --- a/compos/aidl/com/android/compos/ICompOsService.aidl +++ b/compos/aidl/com/android/compos/ICompOsService.aidl @@ -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 diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs index 9276fb16..3a699ab1 100644 --- a/compos/composd/src/odrefresh_task.rs +++ b/compos/composd/src/odrefresh_task.rs @@ -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()) diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs index d165599d..2872d95b 100644 --- a/compos/src/compilation.rs +++ b/compos/src/compilation.rs @@ -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, - 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, - output_dir_fd: i32, - staging_dir_fd: i32, - target_dir_name: &'a str, - zygote_arch: &'a str, - system_server_compiler_filter: &'a str, - ) -> Result { - 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( odrefresh_path: &Path, - context: OdrefreshContext, + args: &OdrefreshArgs, authfs_service: Strong, success_fn: F, ) -> Result 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)?; } diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs index 3dbb4daa..4330bbfb 100644 --- a/compos/src/compsvc.rs +++ b/compos/src/compsvc.rs @@ -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 { + fn odrefresh(&self, args: &OdrefreshArgs) -> BinderResult { 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> { @@ -147,10 +126,10 @@ impl ICompOsService for CompOsService { } impl CompOsService { - fn do_odrefresh(&self, context: OdrefreshContext) -> Result { + fn do_odrefresh(&self, args: &OdrefreshArgs) -> Result { 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);