Merge "Run VirtualizationTestCases for presubmit builds"
diff --git a/compos/aidl/com/android/compos/CompilationResult.aidl b/compos/aidl/com/android/compos/CompilationResult.aidl
new file mode 100644
index 0000000..7a50765
--- /dev/null
+++ b/compos/aidl/com/android/compos/CompilationResult.aidl
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.compos;
+
+/** {@hide} */
+parcelable CompilationResult {
+    /** Exit code of dex2oat */
+    byte exitCode;
+
+    /** raw signature of the output oat's fs-verity digest, may be empty */
+    byte[] oatSignature;
+
+    /** raw signature of the output vdex's fs-verity digest, may be empty */
+    byte[] vdexSignature;
+
+    /** raw signature of the output image's fs-verity digest, may be empty */
+    byte[] imageSignature;
+}
diff --git a/compos/aidl/com/android/compos/ICompOsService.aidl b/compos/aidl/com/android/compos/ICompOsService.aidl
index aaba86c..3a74940 100644
--- a/compos/aidl/com/android/compos/ICompOsService.aidl
+++ b/compos/aidl/com/android/compos/ICompOsService.aidl
@@ -17,6 +17,7 @@
 package com.android.compos;
 
 import com.android.compos.CompOsKeyData;
+import com.android.compos.CompilationResult;
 import com.android.compos.Metadata;
 
 /** {@hide} */
@@ -32,7 +33,7 @@
     void initializeSigningKey(in byte[] keyBlob);
 
     /**
-     * Execute a command composed of the args, in a context that may be specified in the Metadata,
+     * Run dex2oat command with provided args, in a context that may be specified in the Metadata,
      * e.g. with file descriptors pre-opened. The service is responsible to decide what executables
      * it may run.
      *
@@ -40,9 +41,9 @@
      *             which may not be used by the service. The service may be configured to always use
      *             a fixed executable, or possibly use the 0-th args are the executable lookup hint.
      * @param metadata Additional information of the execution
-     * @return exit code of the program
+     * @return a CompilationResult
      */
-    byte execute(in String[] args, in Metadata metadata);
+    CompilationResult compile(in String[] args, in Metadata metadata);
 
     /**
      * Generate a new public/private key pair suitable for signing CompOs output files.
diff --git a/compos/compos_key_cmd/compos_key_cmd.cpp b/compos/compos_key_cmd/compos_key_cmd.cpp
index e65324a..d847b6d 100644
--- a/compos/compos_key_cmd/compos_key_cmd.cpp
+++ b/compos/compos_key_cmd/compos_key_cmd.cpp
@@ -75,7 +75,7 @@
         "/apex/com.android.compos/etc/CompOSPayloadApp.apk.idsig";
 
 // This is a path inside the APK
-constexpr const char* kConfigFilePath = "assets/key_service_vm_config.json";
+constexpr const char* kConfigFilePath = "assets/vm_config.json";
 
 static bool writeBytesToFile(const std::vector<uint8_t>& bytes, const std::string& path) {
     std::string str(bytes.begin(), bytes.end());
@@ -434,6 +434,28 @@
     return {};
 }
 
+static Result<void> initializeKey(TargetVm& vm, const std::string& blob_file) {
+    auto cid = vm.resolveCid();
+    if (!cid.ok()) {
+        return cid.error();
+    }
+    auto service = getService(*cid);
+    if (!service) {
+        return Error() << "No service";
+    }
+
+    auto blob = readBytesFromFile(blob_file);
+    if (!blob.ok()) {
+        return blob.error();
+    }
+
+    auto status = service->initializeSigningKey(blob.value());
+    if (!status.isOk()) {
+        return Error() << "Failed to initialize signing key: " << status.getDescription();
+    }
+    return {};
+}
+
 static Result<void> makeInstanceImage(const std::string& image_path) {
     ndk::SpAIBinder binder(AServiceManager_waitForService("android.system.virtualizationservice"));
     auto service = IVirtualizationService::fromBinder(binder);
@@ -511,6 +533,13 @@
         } else {
             std::cerr << result.error() << '\n';
         }
+    } else if (argc == 3 && argv[1] == "init-key"sv) {
+        auto result = initializeKey(vm, argv[2]);
+        if (result.ok()) {
+            return 0;
+        } else {
+            std::cerr << result.error() << '\n';
+        }
     } else if (argc == 3 && argv[1] == "make-instance"sv) {
         auto result = makeInstanceImage(argv[2]);
         if (result.ok()) {
@@ -519,11 +548,12 @@
             std::cerr << result.error() << '\n';
         }
     } else {
-        std::cerr << "Usage: compos_key_cmd [OPTIONS] generate|verify|sign|make-instance\n"
+        std::cerr << "Usage: compos_key_cmd [OPTIONS] generate|verify|sign|make-instance|init-key\n"
                   << "  generate <blob file> <public key file> Generate new key pair and write\n"
                   << "    the private key blob and public key to the specified files.\n "
                   << "  verify <blob file> <public key file> Verify that the content of the\n"
                   << "    specified private key blob and public key files are valid.\n "
+                  << "  init-key <blob file> Initialize the service key.\n"
                   << "  sign <blob file> <files to be signed> Generate signatures for one or\n"
                   << "    more files using the supplied private key blob. Signature is stored in\n"
                   << "    <filename>.signature\n"
diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs
index 24266e6..0199eb5 100644
--- a/compos/src/compilation.rs
+++ b/compos/src/compilation.rs
@@ -15,15 +15,13 @@
  */
 
 use anyhow::{anyhow, bail, Context, Result};
-use libc::getxattr;
 use log::error;
 use minijail::{self, Minijail};
-use std::ffi::CString;
 use std::fs::File;
-use std::io;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::path::Path;
 
+use crate::fsverity;
 use authfs_aidl_interface::aidl::com::android::virt::fs::{
     AuthFsConfig::AuthFsConfig, IAuthFs::IAuthFs, IAuthFsService::IAuthFsService,
     InputFdAnnotation::InputFdAnnotation, OutputFdAnnotation::OutputFdAnnotation,
@@ -35,12 +33,13 @@
 /// meaningless in the current process.
 pub type PseudoRawFd = i32;
 
-const SHA256_HASH_SIZE: usize = 32;
-type Sha256Hash = [u8; SHA256_HASH_SIZE];
-
 pub enum CompilerOutput {
     /// Fs-verity digests of output files, if the compiler finishes successfully.
-    Digests { oat: Sha256Hash, vdex: Sha256Hash, image: Sha256Hash },
+    Digests {
+        oat: fsverity::Sha256Digest,
+        vdex: fsverity::Sha256Digest,
+        image: fsverity::Sha256Digest,
+    },
     /// Exit code returned by the compiler, if not 0.
     ExitCode(i8),
 }
@@ -82,9 +81,9 @@
 
     match jail_result {
         Ok(()) => Ok(CompilerOutput::Digests {
-            oat: fsverity_measure(oat_file.as_raw_fd())?,
-            vdex: fsverity_measure(vdex_file.as_raw_fd())?,
-            image: fsverity_measure(image_file.as_raw_fd())?,
+            oat: fsverity::measure(oat_file.as_raw_fd())?,
+            vdex: fsverity::measure(vdex_file.as_raw_fd())?,
+            image: fsverity::measure(image_file.as_raw_fd())?,
         }),
         Err(minijail::Error::ReturnCode(exit_code)) => {
             error!("dex2oat failed with exit code {}", exit_code);
@@ -186,22 +185,3 @@
     let _pid = jail.run_remap(executable, preserve_fds.as_slice(), args)?;
     Ok(jail)
 }
-
-fn fsverity_measure(fd: RawFd) -> Result<Sha256Hash> {
-    // TODO(b/196635431): Unfortunately, the FUSE API doesn't allow authfs to implement the standard
-    // fs-verity ioctls. Until the kernel allows, use the alternative xattr that authfs provides.
-    let path = CString::new(format!("/proc/self/fd/{}", fd).as_str()).unwrap();
-    let name = CString::new("authfs.fsverity.digest").unwrap();
-    let mut buf = [0u8; SHA256_HASH_SIZE];
-    // SAFETY: getxattr should not write beyond the given buffer size.
-    let size = unsafe {
-        getxattr(path.as_ptr(), name.as_ptr(), buf.as_mut_ptr() as *mut libc::c_void, buf.len())
-    };
-    if size < 0 {
-        bail!("Failed to getxattr: {}", io::Error::last_os_error());
-    } else if size != SHA256_HASH_SIZE as isize {
-        bail!("Unexpected hash size: {}", size);
-    } else {
-        Ok(buf)
-    }
-}
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index b55fb7c..4a19030 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -19,16 +19,19 @@
 //! actual compiler.
 
 use anyhow::Result;
-use log::{debug, warn};
+use log::warn;
+use std::default::Default;
 use std::ffi::CString;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 
 use crate::compilation::{compile, CompilerOutput};
 use crate::compos_key_service::CompOsKeyService;
+use crate::fsverity;
 use authfs_aidl_interface::aidl::com::android::virt::fs::IAuthFsService::IAuthFsService;
 use compos_aidl_interface::aidl::com::android::compos::{
     CompOsKeyData::CompOsKeyData,
+    CompilationResult::CompilationResult,
     ICompOsService::{BnCompOsService, ICompOsService},
     Metadata::Metadata,
 };
@@ -55,6 +58,20 @@
     key_blob: Arc<RwLock<Vec<u8>>>,
 }
 
+impl CompOsService {
+    fn generate_raw_fsverity_signature(
+        &self,
+        key_blob: &[u8],
+        fsverity_digest: &fsverity::Sha256Digest,
+    ) -> Vec<u8> {
+        let formatted_digest = fsverity::to_formatted_digest(fsverity_digest);
+        self.key_service.do_sign(key_blob, &formatted_digest[..]).unwrap_or_else(|e| {
+            warn!("Failed to sign the fsverity digest, returning empty signature.  Error: {}", e);
+            Vec::new()
+        })
+    }
+}
+
 impl Interface for CompOsService {}
 
 impl ICompOsService for CompOsService {
@@ -68,7 +85,7 @@
         }
     }
 
-    fn execute(&self, args: &[String], metadata: &Metadata) -> BinderResult<i8> {
+    fn compile(&self, args: &[String], metadata: &Metadata) -> BinderResult<CompilationResult> {
         let authfs_service = get_authfs_service()?;
         let output = compile(&self.dex2oat_path, args, authfs_service, metadata).map_err(|e| {
             new_binder_exception(
@@ -78,13 +95,27 @@
         })?;
         match output {
             CompilerOutput::Digests { oat, vdex, image } => {
-                // TODO(b/161471326): Sign the output on succeed.
-                debug!("oat fs-verity digest: {:02x?}", oat);
-                debug!("vdex fs-verity digest: {:02x?}", vdex);
-                debug!("image fs-verity digest: {:02x?}", image);
-                Ok(0)
+                let key = &*self.key_blob.read().unwrap();
+                if key.is_empty() {
+                    Err(new_binder_exception(
+                        ExceptionCode::ILLEGAL_STATE,
+                        "Key is not initialized",
+                    ))
+                } else {
+                    let oat_signature = self.generate_raw_fsverity_signature(key, &oat);
+                    let vdex_signature = self.generate_raw_fsverity_signature(key, &vdex);
+                    let image_signature = self.generate_raw_fsverity_signature(key, &image);
+                    Ok(CompilationResult {
+                        exitCode: 0,
+                        oatSignature: oat_signature,
+                        vdexSignature: vdex_signature,
+                        imageSignature: image_signature,
+                    })
+                }
             }
-            CompilerOutput::ExitCode(exit_code) => Ok(exit_code),
+            CompilerOutput::ExitCode(exit_code) => {
+                Ok(CompilationResult { exitCode: exit_code, ..Default::default() })
+            }
         }
     }
 
diff --git a/compos/src/compsvc_main.rs b/compos/src/compsvc_main.rs
index 48e37b6..6396556 100644
--- a/compos/src/compsvc_main.rs
+++ b/compos/src/compsvc_main.rs
@@ -21,6 +21,7 @@
 mod compilation;
 mod compos_key_service;
 mod compsvc;
+mod fsverity;
 mod signer;
 
 use crate::common::{SERVICE_NAME, VSOCK_PORT};
diff --git a/compos/src/fsverity.rs b/compos/src/fsverity.rs
new file mode 100644
index 0000000..a1e2314
--- /dev/null
+++ b/compos/src/fsverity.rs
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+use anyhow::{bail, Result};
+use libc::getxattr;
+use std::ffi::CString;
+use std::io;
+use std::os::unix::io::RawFd;
+
+/// Magic used in fs-verity digest
+const FS_VERITY_MAGIC: &[u8; 8] = b"FSVerity";
+
+/// Hash algorithm to use from linux/fsverity.h
+const FS_VERITY_HASH_ALG_SHA256: u8 = 1;
+
+const SHA256_HASH_SIZE: usize = 32;
+
+/// Size of `struct fsverity_formatted_digest` with SHA-256 in bytes.
+const FORMATTED_SHA256_DIGEST_SIZE: usize = 12 + SHA256_HASH_SIZE;
+
+/// Bytes of `struct fsverity_formatted_digest` in Linux with SHA-256.
+pub type FormattedSha256Digest = [u8; FORMATTED_SHA256_DIGEST_SIZE];
+
+/// Bytes of SHA256 digest
+pub type Sha256Digest = [u8; SHA256_HASH_SIZE];
+
+/// Returns the fs-verity measurement/digest. Currently only SHA256 is supported.
+pub fn measure(fd: RawFd) -> Result<Sha256Digest> {
+    // TODO(b/196635431): Unfortunately, the FUSE API doesn't allow authfs to implement the standard
+    // fs-verity ioctls. Until the kernel allows, use the alternative xattr that authfs provides.
+    let path = CString::new(format!("/proc/self/fd/{}", fd).as_str()).unwrap();
+    let name = CString::new("authfs.fsverity.digest").unwrap();
+    let mut buf = [0u8; SHA256_HASH_SIZE];
+    // SAFETY: getxattr should not write beyond the given buffer size.
+    let size = unsafe {
+        getxattr(path.as_ptr(), name.as_ptr(), buf.as_mut_ptr() as *mut libc::c_void, buf.len())
+    };
+    if size < 0 {
+        bail!("Failed to getxattr: {}", io::Error::last_os_error());
+    } else if size != SHA256_HASH_SIZE as isize {
+        bail!("Unexpected hash size: {}", size);
+    } else {
+        Ok(buf)
+    }
+}
+
+pub fn to_formatted_digest(digest: &Sha256Digest) -> FormattedSha256Digest {
+    let mut formatted_digest: FormattedSha256Digest = [0; FORMATTED_SHA256_DIGEST_SIZE];
+    formatted_digest[0..8].copy_from_slice(FS_VERITY_MAGIC);
+    formatted_digest[8..10].copy_from_slice(&(FS_VERITY_HASH_ALG_SHA256 as u16).to_le_bytes());
+    formatted_digest[10..12].copy_from_slice(&(SHA256_HASH_SIZE as u16).to_le_bytes());
+    formatted_digest[12..].copy_from_slice(digest);
+    formatted_digest
+}
diff --git a/compos/src/pvm_exec.rs b/compos/src/pvm_exec.rs
index 69eebbf..99eddfc 100644
--- a/compos/src/pvm_exec.rs
+++ b/compos/src/pvm_exec.rs
@@ -14,20 +14,22 @@
  * limitations under the License.
  */
 
-//! pvm_exec is a proxy/wrapper command to run a command remotely. It does not transport the
-//! program and just pass the command line arguments to compsvc to execute. The most important task
+//! pvm_exec is a proxy/wrapper command to run compilation task remotely. The most important task
 //! for this program is to run a `fd_server` that serves remote file read/write requests.
 //!
-//! Example:
-//! $ adb shell exec 3</dev/zero 4<>/dev/null pvm_exec --in-fd 3 --out-fd 4 -- sleep 10
+//! It currently works as a command line wrapper to make it easy to schedule an existing dex2oat
+//! task to run in the VM.
 //!
-//! Note the immediate argument right after "--" (e.g. "sleep" in the example above) is not really
-//! used. It is only for ergonomics.
+//! Example:
+//! $ adb shell exec 3</input/dex 4<>/output/oat ... pvm_exec --in-fd 3 --out-fd 4 -- dex2oat64 ...
+//!
+//! Note the immediate argument "dex2oat64" right after "--" is not really used. It is only for
+//! ergonomics.
 
 use anyhow::{bail, Context, Result};
 use binder::unstable_api::{new_spibinder, AIBinder};
 use binder::FromIBinder;
-use log::{error, warn};
+use log::{debug, error, warn};
 use minijail::Minijail;
 use nix::fcntl::{fcntl, FcntlArg::F_GETFD};
 use nix::sys::stat::fstat;
@@ -187,14 +189,22 @@
 
     // 3. Send the command line args to the remote to execute.
     let service = if let Some(cid) = cid { get_rpc_binder(cid) } else { get_local_service() }?;
-    let exit_code = service.execute(&args, &metadata).context("Binder call failed")?;
+    let result = service.compile(&args, &metadata).context("Binder call failed")?;
+
+    // TODO: store/use the signature
+    debug!(
+        "Signature length: oat {}, vdex {}, image {}",
+        result.oatSignature.len(),
+        result.vdexSignature.len(),
+        result.imageSignature.len()
+    );
 
     // Be explicit about the lifetime, which should last at least until the task is finished.
     drop(fd_server_lifetime);
 
-    if exit_code > 0 {
-        error!("remote execution failed with exit code {}", exit_code);
-        exit(exit_code as i32);
+    if result.exitCode > 0 {
+        error!("remote execution failed with exit code {}", result.exitCode);
+        exit(result.exitCode as i32);
     }
     Ok(())
 }
diff --git a/compos/tests/java/android/compos/test/ComposTestCase.java b/compos/tests/java/android/compos/test/ComposTestCase.java
index f69b7b7..8409f44 100644
--- a/compos/tests/java/android/compos/test/ComposTestCase.java
+++ b/compos/tests/java/android/compos/test/ComposTestCase.java
@@ -40,6 +40,9 @@
     /** Path to odrefresh on Microdroid */
     private static final String ODREFRESH_BIN = "/apex/com.android.art/bin/odrefresh";
 
+    /** Path to compos_key_cmd on Microdroid */
+    private static final String COMPOS_KEY_CMD_BIN = "/apex/com.android.compos/bin/compos_key_cmd";
+
     /** Output directory of odrefresh */
     private static final String ODREFRESH_OUTPUT_DIR =
             "/data/misc/apexdata/com.android.art/dalvik-cache";
@@ -100,6 +103,15 @@
                 android.runForResultWithTimeout(ODREFRESH_TIMEOUT_MS, ODREFRESH_BIN, "--check");
         assertThat(result.getExitCode()).isEqualTo(OKAY);
 
+        // Initialize the service with the generated key. Should succeed.
+        android.run(
+                COMPOS_KEY_CMD_BIN,
+                "--cid " + mCid,
+                "generate",
+                TEST_ROOT + "test_key.blob",
+                TEST_ROOT + "test_key.pubkey");
+        android.run(COMPOS_KEY_CMD_BIN, "--cid " + mCid, "init-key", TEST_ROOT + "test_key.blob");
+
         // Expect the compilation in Compilation OS to finish successfully.
         {
             long start = System.currentTimeMillis();
diff --git a/demo/java/com/android/microdroid/demo/MainActivity.java b/demo/java/com/android/microdroid/demo/MainActivity.java
index 044a55d..8974475 100644
--- a/demo/java/com/android/microdroid/demo/MainActivity.java
+++ b/demo/java/com/android/microdroid/demo/MainActivity.java
@@ -41,6 +41,7 @@
 import java.io.BufferedReader;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -120,16 +121,67 @@
         private final MutableLiveData<String> mConsoleOutput = new MutableLiveData<>();
         private final MutableLiveData<String> mPayloadOutput = new MutableLiveData<>();
         private final MutableLiveData<VirtualMachine.Status> mStatus = new MutableLiveData<>();
+        private ExecutorService mExecutorService;
 
         public VirtualMachineModel(Application app) {
             super(app);
             mStatus.setValue(VirtualMachine.Status.DELETED);
         }
 
+        private static void postOutput(MutableLiveData<String> output, InputStream stream)
+                throws IOException {
+            BufferedReader reader = new BufferedReader(new InputStreamReader(stream));
+            String line;
+            while ((line = reader.readLine()) != null && !Thread.interrupted()) {
+                output.postValue(line);
+            }
+        }
+
         /** Runs a VM */
         public void run(boolean debug) {
             // Create a VM and run it.
             // TODO(jiyong): remove the call to idsigPath
+            mExecutorService = Executors.newFixedThreadPool(2);
+
+            VirtualMachineCallback callback =
+                    new VirtualMachineCallback() {
+                        // store reference to ExecutorService to avoid race condition
+                        private final ExecutorService mService = mExecutorService;
+
+                        @Override
+                        public void onPayloadStarted(
+                                VirtualMachine vm, ParcelFileDescriptor stream) {
+                            if (stream == null) {
+                                mPayloadOutput.postValue("(no output available)");
+                                return;
+                            }
+
+                            mService.execute(
+                                    new Runnable() {
+                                        @Override
+                                        public void run() {
+                                            try {
+                                                postOutput(
+                                                        mPayloadOutput,
+                                                        new FileInputStream(
+                                                                stream.getFileDescriptor()));
+                                            } catch (IOException e) {
+                                                Log.e(
+                                                        TAG,
+                                                        "IOException while reading payload: "
+                                                                + e.getMessage());
+                                            }
+                                        }
+                                    });
+                        }
+
+                        @Override
+                        public void onDied(VirtualMachine vm) {
+                            mService.shutdownNow();
+                            mStatus.postValue(VirtualMachine.Status.STOPPED);
+                        }
+                    };
+
             try {
                 VirtualMachineConfig.Builder builder =
                         new VirtualMachineConfig.Builder(getApplication(), "assets/vm_config.json")
@@ -138,58 +190,25 @@
                 VirtualMachineManager vmm = VirtualMachineManager.getInstance(getApplication());
                 mVirtualMachine = vmm.getOrCreate("demo_vm", config);
                 mVirtualMachine.run();
-                mVirtualMachine.setCallback(
-                        new VirtualMachineCallback() {
-                            @Override
-                            public void onPayloadStarted(
-                                    VirtualMachine vm, ParcelFileDescriptor stream) {
-                                if (stream == null) {
-                                    mPayloadOutput.postValue("(no output available)");
-                                    return;
-                                }
-                                try {
-                                    BufferedReader reader =
-                                            new BufferedReader(
-                                                    new InputStreamReader(
-                                                            new FileInputStream(
-                                                                    stream.getFileDescriptor())));
-                                    String line;
-                                    while ((line = reader.readLine()) != null) {
-                                        mPayloadOutput.postValue(line);
-                                    }
-                                } catch (IOException e) {
-                                    Log.e(TAG, "IOException while reading payload: "
-                                            + e.getMessage());
-                                }
-                            }
-
-                            @Override
-                            public void onDied(VirtualMachine vm) {
-                                mStatus.postValue(VirtualMachine.Status.STOPPED);
-                            }
-                        });
+                mVirtualMachine.setCallback(callback);
                 mStatus.postValue(mVirtualMachine.getStatus());
             } catch (VirtualMachineException e) {
                 throw new RuntimeException(e);
             }
 
             // Read console output from the VM in the background
-            ExecutorService executorService = Executors.newFixedThreadPool(1);
-            executorService.execute(
+            mExecutorService.execute(
                     new Runnable() {
                         @Override
                         public void run() {
                             try {
-                                BufferedReader reader =
-                                        new BufferedReader(
-                                                new InputStreamReader(
-                                                        mVirtualMachine.getConsoleOutputStream()));
-                                while (true) {
-                                    String line = reader.readLine();
-                                    mConsoleOutput.postValue(line);
-                                }
+                                postOutput(
+                                        mConsoleOutput, mVirtualMachine.getConsoleOutputStream());
                             } catch (IOException | VirtualMachineException e) {
-                                // Consume
+                                Log.e(
+                                        TAG,
+                                        "Exception while posting console output: "
+                                                + e.getMessage());
                             }
                         }
                     });
@@ -203,6 +222,7 @@
                 // Consume
             }
             mVirtualMachine = null;
+            mExecutorService.shutdownNow();
             mStatus.postValue(VirtualMachine.Status.STOPPED);
         }
 
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index 4926e2c..47271a7 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -204,10 +204,10 @@
     ],
 }
 
-microdroid_boot_cmdline = "panic=-1 " +
-    "bootconfig " +
-    // TODO(b/181936135) make the ratelimiting conditional; ratelimiting on prod build
-    "printk.devkmsg=on "
+microdroid_boot_cmdline = [
+    "panic=-1",
+    "bootconfig",
+]
 
 bootimg {
     name: "microdroid_boot-5.10",
@@ -222,9 +222,15 @@
         },
         x86_64: {
             kernel_prebuilt: ":kernel_prebuilts-5.10-x86_64",
-            cmdline: microdroid_boot_cmdline + "acpi=noirq",
+            cmdline: microdroid_boot_cmdline + ["acpi=noirq"],
         },
     },
+    product_variables: {
+        debuggable: {
+            cmdline: ["printk.devkmsg=on"],
+        },
+    },
+
     dtb_prebuilt: "dummy_dtb.img",
     header_version: "4",
     partition_name: "boot",
diff --git a/zipfuse/Android.bp b/zipfuse/Android.bp
index 24cfaa0..46f4b5a 100644
--- a/zipfuse/Android.bp
+++ b/zipfuse/Android.bp
@@ -14,6 +14,8 @@
         "libfuse_rust",
         "liblibc",
         "libzip",
+        "libscopeguard",
+        "liblog_rust",
     ],
     // libfuse_rust, etc don't support 32-bit targets
     multilib: {
diff --git a/zipfuse/Cargo.toml b/zipfuse/Cargo.toml
index c8f2f3a..17fd293 100644
--- a/zipfuse/Cargo.toml
+++ b/zipfuse/Cargo.toml
@@ -12,7 +12,8 @@
 zip = "0.5"
 tempfile = "3.2"
 nix = "0.20"
+scopeguard = "1.1"
+log = "0.4"
 
 [dev-dependencies]
 loopdev = "0.2"
-scopeguard = "1.1"
diff --git a/zipfuse/src/main.rs b/zipfuse/src/main.rs
index 4ab934d..a91642c 100644
--- a/zipfuse/src/main.rs
+++ b/zipfuse/src/main.rs
@@ -87,20 +87,23 @@
 
 struct ZipFuse {
     zip_archive: Mutex<zip::ZipArchive<File>>,
+    raw_file: Mutex<File>,
     inode_table: InodeTable,
-    open_files: Mutex<HashMap<Handle, OpenFileBuf>>,
+    open_files: Mutex<HashMap<Handle, OpenFile>>,
     open_dirs: Mutex<HashMap<Handle, OpenDirBuf>>,
 }
 
-/// Holds the (decompressed) contents of a [`ZipFile`].
-///
-/// This buf is needed because `ZipFile` is in general not seekable due to the compression.
-///
-/// TODO(jiyong): do this only for compressed `ZipFile`s. Uncompressed (store) files don't need
-/// this; they can be directly read from `zip_archive`.
-struct OpenFileBuf {
+/// Represents a [`ZipFile`] that is opened.
+struct OpenFile {
     open_count: u32, // multiple opens share the buf because this is a read-only filesystem
-    buf: Box<[u8]>,
+    content: OpenFileContent,
+}
+
+/// Holds the content of a [`ZipFile`]. Depending on whether it is compressed or not, the
+/// entire content is stored, or only the zip index is stored.
+enum OpenFileContent {
+    Compressed(Box<[u8]>),
+    Uncompressed(usize), // zip index
 }
 
 /// Holds the directory entries in a directory opened by [`opendir`].
@@ -123,11 +126,15 @@
     fn new(zip_file: &Path) -> Result<ZipFuse> {
         // TODO(jiyong): Use O_DIRECT to avoid double caching.
         // `.custom_flags(nix::fcntl::OFlag::O_DIRECT.bits())` currently doesn't work.
-        let f = OpenOptions::new().read(true).open(zip_file)?;
+        let f = File::open(zip_file)?;
         let mut z = zip::ZipArchive::new(f)?;
+        // Open the same file again so that we can directly access it when accessing
+        // uncompressed zip_file entries in it. `ZipFile` doesn't implement `Seek`.
+        let raw_file = File::open(zip_file)?;
         let it = InodeTable::from_zip(&mut z)?;
         Ok(ZipFuse {
             zip_archive: Mutex::new(z),
+            raw_file: Mutex::new(raw_file),
             inode_table: it,
             open_files: Mutex::new(HashMap::new()),
             open_dirs: Mutex::new(HashMap::new()),
@@ -208,21 +215,37 @@
         // If the file is already opened, just increase the reference counter. If not, read the
         // entire file content to the buffer. When `read` is called, a portion of the buffer is
         // copied to the kernel.
-        // TODO(jiyong): do this only for compressed zip files. Files that are not compressed
-        // (store) can be directly read from zip_archive. That will help reduce the memory usage.
-        if let Some(ofb) = open_files.get_mut(&handle) {
-            if ofb.open_count == 0 {
+        if let Some(file) = open_files.get_mut(&handle) {
+            if file.open_count == 0 {
                 return Err(ebadf());
             }
-            ofb.open_count += 1;
+            file.open_count += 1;
         } else {
             let inode_data = self.find_inode(inode)?;
             let zip_index = inode_data.get_zip_index().ok_or_else(ebadf)?;
             let mut zip_archive = self.zip_archive.lock().unwrap();
             let mut zip_file = zip_archive.by_index(zip_index)?;
-            let mut buf = Vec::with_capacity(inode_data.size as usize);
-            zip_file.read_to_end(&mut buf)?;
-            open_files.insert(handle, OpenFileBuf { open_count: 1, buf: buf.into_boxed_slice() });
+            let content = match zip_file.compression() {
+                zip::CompressionMethod::Stored => OpenFileContent::Uncompressed(zip_index),
+                _ => {
+                    if let Some(mode) = zip_file.unix_mode() {
+                        let is_reg_file = zip_file.is_file();
+                        let is_executable =
+                            mode & (libc::S_IXUSR | libc::S_IXGRP | libc::S_IXOTH) != 0;
+                        if is_reg_file && is_executable {
+                            log::warn!(
+                                "Executable file {:?} is stored compressed. Consider \
+                                storing it uncompressed to save memory",
+                                zip_file.mangled_name()
+                            );
+                        }
+                    }
+                    let mut buf = Vec::with_capacity(inode_data.size as usize);
+                    zip_file.read_to_end(&mut buf)?;
+                    OpenFileContent::Compressed(buf.into_boxed_slice())
+                }
+            };
+            open_files.insert(handle, OpenFile { open_count: 1, content });
         }
         // Note: we don't return `DIRECT_IO` here, because then applications wouldn't be able to
         // mmap the files.
@@ -244,8 +267,8 @@
         // again when the same file is opened in the future.
         let mut open_files = self.open_files.lock().unwrap();
         let handle = inode as Handle;
-        if let Some(ofb) = open_files.get_mut(&handle) {
-            if ofb.open_count.checked_sub(1).ok_or_else(ebadf)? == 0 {
+        if let Some(file) = open_files.get_mut(&handle) {
+            if file.open_count.checked_sub(1).ok_or_else(ebadf)? == 0 {
                 open_files.remove(&handle);
             }
             Ok(())
@@ -266,15 +289,28 @@
         _flags: u32,
     ) -> io::Result<usize> {
         let open_files = self.open_files.lock().unwrap();
-        let ofb = open_files.get(&handle).ok_or_else(ebadf)?;
-        if ofb.open_count == 0 {
+        let file = open_files.get(&handle).ok_or_else(ebadf)?;
+        if file.open_count == 0 {
             return Err(ebadf());
         }
-        let start = offset as usize;
-        let end = start + size as usize;
-        let end = std::cmp::min(end, ofb.buf.len());
-        let read_len = w.write(&ofb.buf[start..end])?;
-        Ok(read_len)
+        Ok(match &file.content {
+            OpenFileContent::Uncompressed(zip_index) => {
+                let mut zip_archive = self.zip_archive.lock().unwrap();
+                let zip_file = zip_archive.by_index(*zip_index)?;
+                let start = zip_file.data_start() + offset;
+                let remaining_size = zip_file.size() - offset;
+                let size = std::cmp::min(remaining_size, size.into());
+
+                let mut raw_file = self.raw_file.lock().unwrap();
+                w.write_from(&mut raw_file, size as usize, start)?
+            }
+            OpenFileContent::Compressed(buf) => {
+                let start = offset as usize;
+                let end = start + size as usize;
+                let end = std::cmp::min(end, buf.len());
+                w.write(&buf[start..end])?
+            }
+        })
     }
 
     fn opendir(
@@ -672,6 +708,25 @@
         run_fuse_and_check_test_zip(test_dir.path(), &zip_path);
     }
 
+    #[test]
+    fn supports_store() {
+        run_test(
+            |zip| {
+                let data = vec![10; 2 << 20];
+                zip.start_file(
+                    "foo",
+                    FileOptions::default().compression_method(zip::CompressionMethod::Stored),
+                )
+                .unwrap();
+                zip.write_all(&data).unwrap();
+            },
+            |root| {
+                let data = vec![10; 2 << 20];
+                check_file(root, "foo", &data);
+            },
+        );
+    }
+
     #[cfg(not(target_os = "android"))] // Android doesn't have the loopdev crate
     #[test]
     fn supports_zip_on_block_device() {