Merge "Use a wrapper around ANW to allow updating Surface" into main
diff --git a/docs/custom_vm.md b/docs/custom_vm.md
index 840acc3..1e15d16 100644
--- a/docs/custom_vm.md
+++ b/docs/custom_vm.md
@@ -289,6 +289,8 @@
 
 ```
 $ adb shell pm clear com.android.virtualization.vmlauncher
+# or
+$ adb shell pm clear com.google.android.virtualization.vmlauncher
 ```
 
 ### Inside guest OS (for ChromiumOS only)
@@ -305,8 +307,19 @@
 
 ### Debugging
 
-To see console log, check
+To open the serial console (interactive terminal):
+```shell
+$ adb shell -t /apex/com.android.virt/bin/vm console
+```
+
+To see console logs only, check
 `/data/data/com.android.virtualization.vmlauncher/files/console.log`
+Or
+`/data/data/com.google.android.virtualization.vmlauncher/files/console.log`
+
+```shell
+$ adb shell su root tail +0 -F /data/data/com{,.google}.android.virtualization.vmlauncher/files/console.log
+```
 
 For ChromiumOS, you can ssh-in. Use following commands after network setup.
 
diff --git a/java/framework/src/android/system/virtualmachine/VirtualMachine.java b/java/framework/src/android/system/virtualmachine/VirtualMachine.java
index 43f3db0..b6f811e 100644
--- a/java/framework/src/android/system/virtualmachine/VirtualMachine.java
+++ b/java/framework/src/android/system/virtualmachine/VirtualMachine.java
@@ -1214,6 +1214,9 @@
                         service.createVm(vmConfigParcel, consoleOutFd, consoleInFd, mLogWriter);
                 mVirtualMachine.registerCallback(new CallbackTranslator(service));
                 mContext.registerComponentCallbacks(mMemoryManagementCallbacks);
+                if (mConnectVmConsole) {
+                    mVirtualMachine.setHostConsoleName(getHostConsoleName());
+                }
                 mVirtualMachine.start();
             } catch (IOException e) {
                 throw new VirtualMachineException("failed to persist files", e);
@@ -1335,7 +1338,7 @@
      * @hide
      */
     @NonNull
-    public String getHostConsoleName() throws VirtualMachineException {
+    private String getHostConsoleName() throws VirtualMachineException {
         if (!mConnectVmConsole) {
             throw new VirtualMachineException("Host console is not enabled");
         }
diff --git a/microdroid_manager/src/verify.rs b/microdroid_manager/src/verify.rs
index 65c32b0..84feb68 100644
--- a/microdroid_manager/src/verify.rs
+++ b/microdroid_manager/src/verify.rs
@@ -14,7 +14,7 @@
 
 use crate::instance::{ApexData, ApkData, MicrodroidData};
 use crate::payload::{get_apex_data_from_payload, to_metadata};
-use crate::{is_strict_boot, is_verified_boot, MicrodroidError};
+use crate::{is_strict_boot, MicrodroidError};
 use anyhow::{anyhow, ensure, Context, Result};
 use apkmanifest::get_manifest_info;
 use apkverify::{extract_signed_data, verify, V4Signature};
@@ -130,11 +130,10 @@
     // APEX payload.
     let apex_data_from_payload = get_apex_data_from_payload(metadata)?;
 
-    // Writing /apex/vm-payload-metadata is to verify that the payload isn't changed.
-    // Skip writing it if the debug policy ignoring identity is on
-    if is_verified_boot() {
-        write_apex_payload_data(saved_data, &apex_data_from_payload)?;
-    }
+    // To prevent a TOCTOU attack, we need to make sure that when apexd verifies & mounts the
+    // APEXes it sees the same ones that we just read - so we write the metadata we just collected
+    // to a file (that the host can't access) that apexd will then verify against. See b/199371341.
+    write_apex_payload_data(saved_data, &apex_data_from_payload)?;
 
     if cfg!(not(dice_changes)) {
         // Start apexd to activate APEXes
@@ -222,16 +221,17 @@
             saved_apex_data == apex_data_from_payload,
             MicrodroidError::PayloadChanged(String::from("APEXes have changed."))
         );
-        let apex_metadata = to_metadata(apex_data_from_payload);
-        // Pass metadata(with public keys and root digests) to apexd so that it uses the passed
-        // metadata instead of the default one (/dev/block/by-name/payload-metadata)
-        OpenOptions::new()
-            .create_new(true)
-            .write(true)
-            .open("/apex/vm-payload-metadata")
-            .context("Failed to open /apex/vm-payload-metadata")
-            .and_then(|f| write_metadata(&apex_metadata, f))?;
     }
+    let apex_metadata = to_metadata(apex_data_from_payload);
+    // Pass metadata(with public keys and root digests) to apexd so that it uses the passed
+    // metadata instead of the default one (/dev/block/by-name/payload-metadata)
+    OpenOptions::new()
+        .create_new(true)
+        .write(true)
+        .open("/apex/vm-payload-metadata")
+        .context("Failed to open /apex/vm-payload-metadata")
+        .and_then(|f| write_metadata(&apex_metadata, f))?;
+
     Ok(())
 }
 
diff --git a/tests/ferrochrome/assets/vm_config.json b/tests/ferrochrome/assets/vm_config.json
index f8a3099..1d32463 100644
--- a/tests/ferrochrome/assets/vm_config.json
+++ b/tests/ferrochrome/assets/vm_config.json
@@ -1,6 +1,5 @@
 {
     "name": "cros",
-    "kernel": "/data/local/tmp/ferrochrome/vmlinuz",
     "disks": [
         {
             "image": "/data/local/tmp/ferrochrome/chromiumos_test_image.bin",
diff --git a/tests/ferrochrome/ferrochrome.sh b/tests/ferrochrome/ferrochrome.sh
index 4dde401..5638b34 100755
--- a/tests/ferrochrome/ferrochrome.sh
+++ b/tests/ferrochrome/ferrochrome.sh
@@ -102,16 +102,16 @@
 
     echo "Downloading ferrochrome image to ${fecr_dir}"
     fecr_version=${fecr_version:-${FECR_DEFAULT_VERSION}}
-    curl --output-dir ${fecr_dir} -O ${FECR_GS_URL}/${fecr_version}/image.zip
+    curl --output-dir ${fecr_dir} -O ${FECR_GS_URL}/${fecr_version}/chromiumos_test_image.tar.xz
   fi
   if [[ ! -f "${fecr_dir}/chromiumos_test_image.bin" ]]; then
-    unzip ${fecr_dir}/image.zip chromiumos_test_image.bin boot_images/vmlinuz* -d ${fecr_dir} > /dev/null
+    echo "Extrating ferrochrome image"
+    tar xvf ${fecr_dir}/chromiumos_test_image.tar.xz -C ${fecr_dir} > /dev/null
   fi
 
   echo "Pushing ferrochrome image to ${FECR_DEVICE_DIR}"
   adb shell mkdir -p ${FECR_DEVICE_DIR} > /dev/null || true
   adb push ${fecr_dir}/chromiumos_test_image.bin ${FECR_DEVICE_DIR}
-  adb push ${fecr_dir}/boot_images/vmlinuz ${FECR_DEVICE_DIR}
   adb push ${fecr_script_path}/assets/vm_config.json ${FECR_CONFIG_PATH}
 fi
 
diff --git a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
index e02db39..6474965 100644
--- a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
@@ -20,8 +20,8 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 
-import static org.junit.Assume.assumeTrue;
 import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
 
 import android.app.Instrumentation;
 import android.app.UiAutomation;
@@ -213,6 +213,7 @@
                 .isTrue();
         int vendorApiLevel = getVendorApiLevel();
         boolean isGsi = new File("/system/system_ext/etc/init/init.gsi.rc").exists();
+        Log.i(TAG, "isGsi = " + isGsi + ", vendor api level = " + vendorApiLevel);
         assume().withMessage("GSI with vendor API level < 202404 may not support AVF")
                 .that(isGsi && vendorApiLevel < 202404)
                 .isFalse();
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index ac70509..9df376a 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -1240,6 +1240,10 @@
             .or_service_specific_exception(-1)?;
         Ok(vsock_stream_to_pfd(stream))
     }
+
+    fn setHostConsoleName(&self, ptsname: &str) -> binder::Result<()> {
+        self.instance.vm_context.global_context.setHostConsoleName(ptsname)
+    }
 }
 
 impl Drop for VirtualMachine {
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 3722d4d..ee5f5cd 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -316,7 +316,7 @@
 #[derive(Debug)]
 pub struct VmContext {
     #[allow(dead_code)] // Keeps the global context alive
-    global_context: Strong<dyn IGlobalVmContext>,
+    pub(crate) global_context: Strong<dyn IGlobalVmContext>,
     #[allow(dead_code)] // Keeps the server alive
     vm_server: RpcServer,
 }
@@ -335,7 +335,7 @@
     pub vm_state: Mutex<VmState>,
     /// Global resources allocated for this VM.
     #[allow(dead_code)] // Keeps the context alive
-    vm_context: VmContext,
+    pub(crate) vm_context: VmContext,
     /// The CID assigned to the VM for vsock communication.
     pub cid: Cid,
     /// Path to crosvm control socket
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
index d76b586..d4001c8 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
@@ -47,4 +47,7 @@
 
     /** Open a vsock connection to the CID of the VM on the given port. */
     ParcelFileDescriptor connectVsock(int port);
+
+    /** Set the name of the peer end (ptsname) of the host console. */
+    void setHostConsoleName(in @utf8InCpp String pathname);
 }
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
index 870a342..9f033b1 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineDebugInfo.aidl
@@ -33,4 +33,7 @@
      * the PID may have been reused for a different process, so this should not be trusted.
      */
     int requesterPid;
+
+    /** The peer end (ptsname) of the host console. */
+    @nullable @utf8InCpp String hostConsoleName;
 }
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice_internal/IGlobalVmContext.aidl b/virtualizationservice/aidl/android/system/virtualizationservice_internal/IGlobalVmContext.aidl
index a4d5d19..ea52591 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice_internal/IGlobalVmContext.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice_internal/IGlobalVmContext.aidl
@@ -21,4 +21,7 @@
 
     /** Get the path to the temporary folder of the VM. */
     String getTemporaryDirectory();
+
+    /** Set the name of the peer end (ptsname) of the host console. */
+    void setHostConsoleName(@utf8InCpp String pathname);
 }
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index ae8d1da..70da37b 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -282,11 +282,15 @@
             .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,
+            .map(|vm| {
+                let vm = vm.lock().unwrap();
+                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,
+                    hostConsoleName: vm.host_console_name.clone(),
+                }
             })
             .collect();
         Ok(cids)
@@ -643,6 +647,8 @@
     requester_uid: uid_t,
     /// PID of the client who requested this VM instance.
     requester_debug_pid: pid_t,
+    /// Name of the host console.
+    host_console_name: Option<String>,
 }
 
 impl GlobalVmInstance {
@@ -657,7 +663,7 @@
 struct GlobalState {
     /// VM contexts currently allocated to running VMs. A CID is never recycled as long
     /// as there is a strong reference held by a GlobalVmContext.
-    held_contexts: HashMap<Cid, Weak<GlobalVmInstance>>,
+    held_contexts: HashMap<Cid, Weak<Mutex<GlobalVmInstance>>>,
 
     /// Cached read-only FD of VM DTBO file. Also serves as a lock for creating the file.
     dtbo_file: Mutex<Option<File>>,
@@ -737,8 +743,13 @@
         self.held_contexts.retain(|_, instance| instance.strong_count() > 0);
 
         let cid = self.get_next_available_cid()?;
-        let instance = Arc::new(GlobalVmInstance { cid, requester_uid, requester_debug_pid });
-        create_temporary_directory(&instance.get_temp_dir(), Some(requester_uid))?;
+        let instance = Arc::new(Mutex::new(GlobalVmInstance {
+            cid,
+            requester_uid,
+            requester_debug_pid,
+            ..Default::default()
+        }));
+        create_temporary_directory(&instance.lock().unwrap().get_temp_dir(), Some(requester_uid))?;
 
         self.held_contexts.insert(cid, Arc::downgrade(&instance));
         let binder = GlobalVmContext { instance, ..Default::default() };
@@ -818,7 +829,7 @@
 #[derive(Debug, Default)]
 struct GlobalVmContext {
     /// Strong reference to the context's instance data structure.
-    instance: Arc<GlobalVmInstance>,
+    instance: Arc<Mutex<GlobalVmInstance>>,
     /// Keeps our service process running as long as this VM context exists.
     #[allow(dead_code)]
     lazy_service_guard: LazyServiceGuard,
@@ -828,11 +839,16 @@
 
 impl IGlobalVmContext for GlobalVmContext {
     fn getCid(&self) -> binder::Result<i32> {
-        Ok(self.instance.cid as i32)
+        Ok(self.instance.lock().unwrap().cid as i32)
     }
 
     fn getTemporaryDirectory(&self) -> binder::Result<String> {
-        Ok(self.instance.get_temp_dir().to_string_lossy().to_string())
+        Ok(self.instance.lock().unwrap().get_temp_dir().to_string_lossy().to_string())
+    }
+
+    fn setHostConsoleName(&self, pathname: &str) -> binder::Result<()> {
+        self.instance.lock().unwrap().host_console_name = Some(pathname.to_string());
+        Ok(())
     }
 }
 
diff --git a/virtualizationservice/vmnic/src/aidl.rs b/virtualizationservice/vmnic/src/aidl.rs
index 69c37b8..03819b8 100644
--- a/virtualizationservice/vmnic/src/aidl.rs
+++ b/virtualizationservice/vmnic/src/aidl.rs
@@ -19,22 +19,20 @@
 use binder::{self, Interface, IntoBinderResult, ParcelFileDescriptor};
 use libc::{c_char, c_int, c_short, ifreq, IFF_NO_PI, IFF_TAP, IFF_UP, IFF_VNET_HDR, IFNAMSIZ};
 use log::info;
-use nix::{ioctl_write_int_bad, ioctl_write_ptr_bad};
+use nix::ioctl_write_ptr_bad;
 use nix::sys::ioctl::ioctl_num_type;
 use nix::sys::socket::{socket, AddressFamily, SockFlag, SockType};
 use std::ffi::{CStr, CString};
-use std::fs::File;
+use std::fs::OpenOptions;
 use std::os::fd::{AsRawFd, RawFd};
 use std::slice::from_raw_parts;
 
-const TUNGETIFF: ioctl_num_type = 0x800454d2u32 as c_int;
+const TUNGETIFF: ioctl_num_type = 0x800454d2u32 as ioctl_num_type;
 const TUNSETIFF: ioctl_num_type = 0x400454ca;
-const TUNSETPERSIST: ioctl_num_type = 0x400454cb;
 const SIOCSIFFLAGS: ioctl_num_type = 0x00008914;
 
 ioctl_write_ptr_bad!(ioctl_tungetiff, TUNGETIFF, ifreq);
 ioctl_write_ptr_bad!(ioctl_tunsetiff, TUNSETIFF, ifreq);
-ioctl_write_int_bad!(ioctl_tunsetpersist, TUNSETPERSIST);
 ioctl_write_ptr_bad!(ioctl_siocsifflags, SIOCSIFFLAGS, ifreq);
 
 fn validate_ifname(ifname: &[c_char]) -> Result<()> {
@@ -51,8 +49,6 @@
     ifr.ifr_name[..ifname.len()].copy_from_slice(ifname);
     // SAFETY: It modifies the state in the kernel, not the state of this process in any way.
     unsafe { ioctl_tunsetiff(fd, &ifr) }.context("Failed to ioctl TUNSETIFF")?;
-    // SAFETY: It modifies the state in the kernel, not the state of this process in any way.
-    unsafe { ioctl_tunsetpersist(fd, 1) }.context("Failed to ioctl TUNSETPERSIST")?;
     // SAFETY: ifr_ifru holds ifru_flags in its union field.
     unsafe { ifr.ifr_ifru.ifru_flags |= IFF_UP as c_short };
     // SAFETY: It modifies the state in the kernel, not the state of this process in any way.
@@ -69,13 +65,11 @@
     Ok(ifr)
 }
 
-fn delete_tap_interface(fd: RawFd, sockfd: c_int, ifr: &mut ifreq) -> Result<()> {
+fn delete_tap_interface(sockfd: c_int, ifr: &mut ifreq) -> Result<()> {
     // SAFETY: After calling TUNGETIFF, ifr_ifru holds ifru_flags in its union field.
     unsafe { ifr.ifr_ifru.ifru_flags &= !IFF_UP as c_short };
     // SAFETY: It modifies the state in the kernel, not the state of this process in any way.
     unsafe { ioctl_siocsifflags(sockfd, ifr) }.context("Failed to ioctl SIOCSIFFLAGS")?;
-    // SAFETY: It modifies the state in the kernel, not the state of this process in any way.
-    unsafe { ioctl_tunsetpersist(fd, 0) }.context("Failed to ioctl TUNSETPERSIST")?;
     Ok(())
 }
 
@@ -105,7 +99,10 @@
             .context(format!("Invalid interface name: {ifname:#?}"))
             .or_service_specific_exception(-1)?;
 
-        let tunfd = File::open("/dev/tun")
+        let tunfd = OpenOptions::new()
+            .read(true)
+            .write(true)
+            .open("/dev/tun")
             .context("Failed to open /dev/tun")
             .or_service_specific_exception(-1)?;
         let sock = socket(AddressFamily::Inet, SockType::Datagram, SockFlag::empty(), None)
@@ -120,8 +117,7 @@
     }
 
     fn deleteTapInterface(&self, tapfd: &ParcelFileDescriptor) -> binder::Result<()> {
-        let tap = tapfd.as_raw_fd();
-        let mut tap_ifreq = get_tap_ifreq(tap)
+        let mut tap_ifreq = get_tap_ifreq(tapfd.as_raw_fd())
             .context("Failed to get ifreq of TAP interface")
             .or_service_specific_exception(-1)?;
         // SAFETY: tap_ifreq.ifr_name is null-terminated within IFNAMSIZ, validated when creating
@@ -131,7 +127,7 @@
         let sock = socket(AddressFamily::Inet, SockType::Datagram, SockFlag::empty(), None)
             .context("Failed to create socket")
             .or_service_specific_exception(-1)?;
-        delete_tap_interface(tap, sock.as_raw_fd(), &mut tap_ifreq)
+        delete_tap_interface(sock.as_raw_fd(), &mut tap_ifreq)
             .context(format!("Failed to create TAP interface: {ifname:#?}"))
             .or_service_specific_exception(-1)?;
 
diff --git a/vm/src/main.rs b/vm/src/main.rs
index a250c35..3c0887c 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -24,15 +24,18 @@
 };
 #[cfg(not(llpvm_changes))]
 use anyhow::anyhow;
-use anyhow::{Context, Error};
+use anyhow::{bail, Context, Error};
 use binder::{ProcessState, Strong};
 use clap::{Args, Parser};
 use create_idsig::command_create_idsig;
 use create_partition::command_create_partition;
 use run::{command_run, command_run_app, command_run_microdroid};
 use serde::Serialize;
+use std::io::{self, IsTerminal};
 use std::num::NonZeroU16;
+use std::os::unix::process::CommandExt;
 use std::path::{Path, PathBuf};
+use std::process::Command;
 
 #[derive(Args, Default)]
 /// Collection of flags that are at VM level and therefore applicable to all subcommands
@@ -324,6 +327,11 @@
         /// Path to idsig of the APK
         path: PathBuf,
     },
+    /// Connect to the serial console of a VM
+    Console {
+        /// CID of the VM
+        cid: Option<i32>,
+    },
 }
 
 fn parse_debug_level(s: &str) -> Result<DebugLevel, String> {
@@ -386,6 +394,7 @@
         Opt::CreateIdsig { apk, path } => {
             command_create_idsig(get_service()?.as_ref(), &apk, &path)
         }
+        Opt::Console { cid } => command_console(cid),
     }
 }
 
@@ -450,6 +459,21 @@
     Ok(())
 }
 
+fn command_console(cid: Option<i32>) -> Result<(), Error> {
+    if !io::stdin().is_terminal() {
+        bail!("Stdin must be a terminal (tty). Use 'adb shell -t' to force allocate tty.");
+    }
+    let mut vms = get_service()?.debugListVms().context("Failed to get list of VMs")?;
+    if let Some(cid) = cid {
+        vms.retain(|vm_info| vm_info.cid == cid);
+    }
+    let host_console_name = vms
+        .into_iter()
+        .find_map(|vm_info| vm_info.hostConsoleName)
+        .context("Failed to get VM with console")?;
+    Err(Command::new("microcom").arg(host_console_name).exec().into())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;