Merge "Document valid port range."
diff --git a/demo/java/com/android/microdroid/demo/MainActivity.java b/demo/java/com/android/microdroid/demo/MainActivity.java
index 77f2ee7..54d7420 100644
--- a/demo/java/com/android/microdroid/demo/MainActivity.java
+++ b/demo/java/com/android/microdroid/demo/MainActivity.java
@@ -238,13 +238,6 @@
mService.shutdownNow();
mStatus.postValue(VirtualMachine.STATUS_STOPPED);
}
-
- @Override
- public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {
- if (!mService.isShutdown()) {
- mPayloadOutput.postValue("(Kernel panic. Ramdump created)");
- }
- }
};
try {
diff --git a/docs/debug/ramdump.md b/docs/debug/ramdump.md
index a0d9bf2..771c608 100644
--- a/docs/debug/ramdump.md
+++ b/docs/debug/ramdump.md
@@ -1,6 +1,6 @@
# Doing RAM dump of a Microdroid VM and analyzing it
-A Microdroid VM creates a RAM dump of itself when the kernel panics. This
+A debuggable Microdroid VM creates a RAM dump of itself when the kernel panics. This
document explains how the dump can be obtained and analyzed.
## Force triggering a RAM dump
@@ -49,7 +49,7 @@
## Obtaining the RAM dump
-By default, RAM dumps are sent to tombstone. To see which tombstone file is for
+RAM dumps are sent to tombstone. To see which tombstone file is for
the RAM dump, look into the log.
```shell
@@ -64,15 +64,6 @@
$ adb root && adb pull /data/tombstones/tombstone_47 ramdump && adb unroot
```
-Alternatively, you can specify the path to where RAM dump is stored when
-launching the VM using the `--ramdump` option of the `vm` tool.
-
-```shell
-$ adb shelll /apex/com.android.virt/bin/vm run-app --ramdump /data/local/tmp/virt/ramdump ...
-```
-
-In the above example, the RAM dump is saved to `/data/local/tmp/virt/ramdump`.
-
## Analyzing the RAM dump
### Building the crash(8) tool
@@ -151,9 +142,3 @@
actually triggered a crash in the kernel.
For more commands of crash(8), refer to the man page, or embedded `help` command.
-
-
-
-
-
-
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index 0fb83ac..d14d83c 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -30,7 +30,6 @@
method public void onPayloadFinished(@NonNull android.system.virtualmachine.VirtualMachine, int);
method public void onPayloadReady(@NonNull android.system.virtualmachine.VirtualMachine);
method public void onPayloadStarted(@NonNull android.system.virtualmachine.VirtualMachine);
- method public void onRamdump(@NonNull android.system.virtualmachine.VirtualMachine, @NonNull android.os.ParcelFileDescriptor);
method public void onStopped(@NonNull android.system.virtualmachine.VirtualMachine, int);
field public static final int ERROR_PAYLOAD_CHANGED = 2; // 0x2
field public static final int ERROR_PAYLOAD_INVALID_CONFIG = 3; // 0x3
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 87f9f6a..1ea6714 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -829,11 +829,6 @@
VirtualMachine.this, translatedReason));
}
}
-
- @Override
- public void onRamdump(int cid, ParcelFileDescriptor ramdump) {
- executeCallback((cb) -> cb.onRamdump(VirtualMachine.this, ramdump));
- }
});
mContext.registerComponentCallbacks(mMemoryManagementCallbacks);
service.asBinder().linkToDeath(deathRecipient, 0);
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
index fad2fa9..9aaecf0 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
@@ -20,7 +20,6 @@
import android.annotation.NonNull;
import android.annotation.SuppressLint;
import android.annotation.SystemApi;
-import android.os.ParcelFileDescriptor;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -155,7 +154,4 @@
/** Called when the VM has stopped. */
void onStopped(@NonNull VirtualMachine vm, @StopReason int reason);
-
- /** Called when kernel panic occurs and as a result ramdump is generated from the VM. */
- void onRamdump(@NonNull VirtualMachine vm, @NonNull ParcelFileDescriptor ramdump);
}
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 602f8b8..75e5414 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -123,7 +123,7 @@
*/
@Nullable private final String mPayloadBinaryPath;
- /** The size of storage in KB. 0 indicates that encryptedStorage is not required */
+ /** The size of storage in KiB. 0 indicates that encryptedStorage is not required */
private final long mEncryptedStorageKib;
private VirtualMachineConfig(
@@ -336,7 +336,7 @@
}
/**
- * Returns the size of encrypted storage (in KB) available in the VM, or 0 if encrypted storage
+ * Returns the size of encrypted storage (in KiB) available in the VM, or 0 if encrypted storage
* is not enabled
*
* @hide
@@ -608,7 +608,7 @@
}
/**
- * Sets the size (in KB) of encrypted storage available to the VM. If not set, no encrypted
+ * Sets the size (in KiB) of encrypted storage available to the VM. If not set, no encrypted
* storage is provided.
*
* <p>The storage is encrypted with a key deterministically derived from the VM identity
diff --git a/libs/apkverify/src/v4.rs b/libs/apkverify/src/v4.rs
index 6c085f6..94abf99 100644
--- a/libs/apkverify/src/v4.rs
+++ b/libs/apkverify/src/v4.rs
@@ -146,6 +146,11 @@
/// Read a stream for an APK file and creates a corresponding `V4Signature` struct that digests
/// the APK file. Note that the signing is not done.
+ /// Important: callers of this function are expected to verify the validity of the passed |apk|.
+ /// To be more specific, they should check that |apk| corresponds to a regular file, as calling
+ /// lseek on directory fds is not defined in the standard, and on ext4 it will return (off_t)-1
+ /// (see: https://bugzilla.kernel.org/show_bug.cgi?id=200043), which will result in this
+ /// function OOMing.
pub fn create(
mut apk: &mut R,
block_size: usize,
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 72a0090..9aed34d 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
@@ -248,9 +248,6 @@
vm.clearCallback();
mExecutorService.shutdown();
}
-
- @Override
- public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {}
}
public static class BootResult {
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
index a329fa6..34b6fa5 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
@@ -50,9 +50,4 @@
* also use `link_to_death` to handle that.
*/
void onDied(int cid, in DeathReason reason);
-
- /**
- * Called when kernel panic occurs and as a result ramdump is generated from the VM.
- */
- void onRamdump(int cid, in ParcelFileDescriptor ramdump);
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 186052d..a35c2ac 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -115,6 +115,24 @@
}
}
+fn create_or_update_idsig_file(
+ input_fd: &ParcelFileDescriptor,
+ idsig_fd: &ParcelFileDescriptor,
+) -> Result<()> {
+ let mut input = clone_file(input_fd)?;
+ let metadata = input.metadata().context("failed to get input metadata")?;
+ if !metadata.is_file() {
+ bail!("input is not a regular file");
+ }
+ let mut sig = V4Signature::create(&mut input, 4096, &[], HashAlgorithm::SHA256)
+ .context("failed to create idsig")?;
+
+ let mut output = clone_file(idsig_fd)?;
+ output.set_len(0).context("failed to set_len on the idsig output")?;
+ sig.write_into(&mut output).context("failed to write idsig")?;
+ Ok(())
+}
+
/// Singleton service for allocating globally-unique VM resources, such as the CID, and running
/// singleton servers, like tombstone receiver.
#[derive(Debug, Default)]
@@ -345,12 +363,8 @@
check_manage_access()?;
- let mut input = clone_file(input_fd)?;
- let mut sig = V4Signature::create(&mut input, 4096, &[], HashAlgorithm::SHA256).unwrap();
-
- let mut output = clone_file(idsig_fd)?;
- output.set_len(0).unwrap();
- sig.write_into(&mut output).unwrap();
+ create_or_update_idsig_file(input_fd, idsig_fd)
+ .map_err(|e| Status::new_service_specific_error_str(-1, Some(format!("{:?}", e))))?;
Ok(())
}
@@ -1052,17 +1066,6 @@
}
}
- /// Call all registered callbacks to say that there was a ramdump to download.
- pub fn callback_on_ramdump(&self, cid: Cid, ramdump: File) {
- let callbacks = &*self.0.lock().unwrap();
- let pfd = ParcelFileDescriptor::new(ramdump);
- for callback in callbacks {
- if let Err(e) = callback.onRamdump(cid as i32, &pfd) {
- error!("Error notifying ramdump of VM CID {}: {:?}", cid, e);
- }
- }
- }
-
/// Add a new callback to the set.
fn add(&self, callback: Strong<dyn IVirtualMachineCallback>) {
self.0.lock().unwrap().push(callback);
@@ -1305,4 +1308,50 @@
}
Ok(())
}
+
+ #[test]
+ fn test_create_or_update_idsig_file_empty_apk() -> Result<()> {
+ let apk = tempfile::tempfile().unwrap();
+ let idsig = tempfile::tempfile().unwrap();
+
+ let ret = create_or_update_idsig_file(
+ &ParcelFileDescriptor::new(apk),
+ &ParcelFileDescriptor::new(idsig),
+ );
+ assert!(ret.is_err(), "should fail");
+ Ok(())
+ }
+
+ #[test]
+ fn test_create_or_update_idsig_dir_instead_of_file_for_apk() -> Result<()> {
+ let tmp_dir = tempfile::TempDir::new().unwrap();
+ let apk = File::open(tmp_dir.path()).unwrap();
+ let idsig = tempfile::tempfile().unwrap();
+
+ let ret = create_or_update_idsig_file(
+ &ParcelFileDescriptor::new(apk),
+ &ParcelFileDescriptor::new(idsig),
+ );
+ assert!(ret.is_err(), "should fail");
+ Ok(())
+ }
+
+ /// Verifies that create_or_update_idsig_file won't oom if a fd that corresponds to a directory
+ /// on ext4 filesystem is passed.
+ /// On ext4 lseek on a directory fd will return (off_t)-1 (see:
+ /// https://bugzilla.kernel.org/show_bug.cgi?id=200043), which will result in
+ /// create_or_update_idsig_file ooming while attempting to allocate petabytes of memory.
+ #[test]
+ fn test_create_or_update_idsig_does_not_crash_dir_on_ext4() -> Result<()> {
+ // APEXes are backed by the ext4.
+ let apk = File::open("/apex/com.android.virt/").unwrap();
+ let idsig = tempfile::tempfile().unwrap();
+
+ let ret = create_or_update_idsig_file(
+ &ParcelFileDescriptor::new(apk),
+ &ParcelFileDescriptor::new(idsig),
+ );
+ assert!(ret.is_err(), "should fail");
+ Ok(())
+ }
}
diff --git a/virtualizationservice/src/crosvm.rs b/virtualizationservice/src/crosvm.rs
index fc85ca5..5125f19 100644
--- a/virtualizationservice/src/crosvm.rs
+++ b/virtualizationservice/src/crosvm.rs
@@ -520,15 +520,10 @@
Ok(())
}
- /// Checks if ramdump has been created. If so, send a notification to the user with the handle
- /// to read the ramdump.
+ /// Checks if ramdump has been created. If so, send it to tombstoned.
fn handle_ramdump(&self) -> Result<(), Error> {
let ramdump_path = self.temporary_directory.join("ramdump");
if std::fs::metadata(&ramdump_path)?.len() > 0 {
- let ramdump = File::open(&ramdump_path)
- .context(format!("Failed to open ramdump {:?} for reading", &ramdump_path))?;
- self.callbacks.callback_on_ramdump(self.cid, ramdump);
-
Self::send_ramdump_to_tombstoned(&ramdump_path)?;
}
Ok(())
@@ -536,7 +531,7 @@
fn send_ramdump_to_tombstoned(ramdump_path: &Path) -> Result<(), Error> {
let mut input = File::open(ramdump_path)
- .context(format!("Failed to open raudmp {:?} for reading", ramdump_path))?;
+ .context(format!("Failed to open ramdump {:?} for reading", ramdump_path))?;
let pid = std::process::id() as i32;
let conn = TombstonedConnection::connect(pid, DebuggerdDumpType::Tombstone)
diff --git a/vm/src/main.rs b/vm/src/main.rs
index 32b165b..3d2fc00 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -81,10 +81,6 @@
#[clap(long)]
log: Option<PathBuf>,
- /// Path to file where ramdump is recorded on kernel panic
- #[clap(long)]
- ramdump: Option<PathBuf>,
-
/// Debug level of the VM. Supported values: "none" (default), and "full".
#[clap(long, default_value = "none", value_parser = parse_debug_level)]
debug: DebugLevel,
@@ -144,10 +140,6 @@
#[clap(long)]
log: Option<PathBuf>,
- /// Path to file where ramdump is recorded on kernel panic
- #[clap(long)]
- ramdump: Option<PathBuf>,
-
/// Debug level of the VM. Supported values: "none" (default), and "full".
#[clap(long, default_value = "full", value_parser = parse_debug_level)]
debug: DebugLevel,
@@ -268,7 +260,6 @@
daemonize,
console,
log,
- ramdump,
debug,
protected,
mem,
@@ -288,7 +279,6 @@
daemonize,
console.as_deref(),
log.as_deref(),
- ramdump.as_deref(),
debug,
protected,
mem,
@@ -304,7 +294,6 @@
daemonize,
console,
log,
- ramdump,
debug,
protected,
mem,
@@ -319,7 +308,6 @@
daemonize,
console.as_deref(),
log.as_deref(),
- ramdump.as_deref(),
debug,
protected,
mem,
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 3f25bba..6096913 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -52,7 +52,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
debug_level: DebugLevel,
protected: bool,
mem: Option<u32>,
@@ -144,7 +143,7 @@
numCpus: cpus.unwrap_or(1) as i32,
taskProfiles: task_profiles,
});
- run(service, &config, &payload_config_str, daemonize, console_path, log_path, ramdump_path)
+ run(service, &config, &payload_config_str, daemonize, console_path, log_path)
}
const EMPTY_PAYLOAD_APK: &str = "com.android.microdroid.empty_payload";
@@ -182,7 +181,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
debug_level: DebugLevel,
protected: bool,
mem: Option<u32>,
@@ -214,7 +212,6 @@
daemonize,
console_path,
log_path,
- ramdump_path,
debug_level,
protected,
mem,
@@ -259,7 +256,6 @@
daemonize,
console_path,
log_path,
- /* ramdump_path */ None,
)
}
@@ -282,7 +278,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
) -> Result<(), Error> {
let console = if let Some(console_path) = console_path {
Some(
@@ -325,27 +320,12 @@
// Wait until the VM or VirtualizationService dies. If we just returned immediately then the
// IVirtualMachine Binder object would be dropped and the VM would be killed.
let death_reason = vm.wait_for_death();
-
- if let Some(path) = ramdump_path {
- save_ramdump_if_available(path, &vm)?;
- }
println!("VM ended: {:?}", death_reason);
}
Ok(())
}
-fn save_ramdump_if_available(path: &Path, vm: &VmInstance) -> Result<(), Error> {
- if let Some(mut ramdump) = vm.get_ramdump() {
- let mut file =
- File::create(path).context(format!("Failed to create ramdump file {:?}", path))?;
- let size = std::io::copy(&mut ramdump, &mut file)
- .context(format!("Failed to save ramdump to file {:?}", path))?;
- eprintln!("Ramdump ({} bytes) saved to {:?}", size, path);
- }
- Ok(())
-}
-
fn parse_extra_apk_list(apk: &Path, config_path: &str) -> Result<Vec<String>, Error> {
let mut archive = ZipArchive::new(File::open(apk)?)?;
let config_file = archive.by_name(config_path)?;
diff --git a/vmclient/src/lib.rs b/vmclient/src/lib.rs
index 20b7f02..7c05545 100644
--- a/vmclient/src/lib.rs
+++ b/vmclient/src/lib.rs
@@ -190,11 +190,6 @@
}
})
}
-
- /// Get ramdump
- pub fn get_ramdump(&self) -> Option<File> {
- self.state.get_ramdump()
- }
}
impl Debug for VmInstance {
@@ -222,7 +217,6 @@
struct VmState {
death_reason: Option<DeathReason>,
reported_state: VirtualMachineState,
- ramdump: Option<File>,
}
impl Monitor<VmState> {
@@ -239,14 +233,6 @@
self.state.lock().unwrap().reported_state = state;
self.cv.notify_all();
}
-
- fn set_ramdump(&self, ramdump: File) {
- self.state.lock().unwrap().ramdump = Some(ramdump);
- }
-
- fn get_ramdump(&self) -> Option<File> {
- self.state.lock().unwrap().ramdump.as_ref().and_then(|f| f.try_clone().ok())
- }
}
struct VirtualMachineCallback {
@@ -302,12 +288,6 @@
Ok(())
}
- fn onRamdump(&self, _cid: i32, ramdump: &ParcelFileDescriptor) -> BinderResult<()> {
- let ramdump: File = ramdump.as_ref().try_clone().unwrap();
- self.state.set_ramdump(ramdump);
- Ok(())
- }
-
fn onDied(&self, cid: i32, reason: AidlDeathReason) -> BinderResult<()> {
let reason = reason.into();
self.state.notify_death(reason);