diff --git a/Cargo.lock b/Cargo.lock index ebfcda3..733c1ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -982,6 +982,15 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "memmap2" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "714098028fe011992e1c3962653c96b2d578c4b4bce9036e15ff220319b1e0e3" +dependencies = [ + "libc", +] + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -1852,6 +1861,7 @@ dependencies = [ "libc", "log", "log-panics", + "memmap2", "mock_instant", "mockall", "mockito", diff --git a/cspell.config.yaml b/cspell.config.yaml index 52f1174..8a6484e 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -55,6 +55,8 @@ words: - libflutter - libupdater - logcat + - memmap + - mmap - mockall - mocktail - msvc @@ -72,6 +74,7 @@ words: - rustup - serde - shorebirdtech + - SIGBUS - sigstore - staticlib - subosito diff --git a/library/Cargo.toml b/library/Cargo.toml index 5031787..3205cda 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -36,6 +36,10 @@ hex = "0.4.3" libc = "0.2.98" # For error!(), info!(), etc macros. `print` will not show up on Android. log = "0.4.14" +# For mmap'ing libapp.so out of the APK without buffering it in RAM. This +# matters on memory-constrained devices: reading the whole library into a +# Vec can cost tens of MB and trigger OOM kills mid-patch. +memmap2 = "0.9.10" # For implementing thread-local-storage of ResolvedConfig object. once_cell = "1.17.1" # Json serialization/de-serialization. diff --git a/library/src/android.rs b/library/src/android.rs index 7a6bdca..eb26e9b 100644 --- a/library/src/android.rs +++ b/library/src/android.rs @@ -1,10 +1,13 @@ // cspell:ignore rpkDZSLBRv2jWcc1gQpwdg use anyhow::Context; +use memmap2::{Mmap, MmapOptions}; use std::fs; use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; -use crate::InitError; +use crate::{InitError, ReadSeek}; + +impl ReadSeek for Cursor {} /// This function is a hack for Android. Android passes an array of paths, the /// first of which is `libapp.so` the second of which is a long (virtual) path @@ -84,10 +87,14 @@ pub(crate) fn get_relative_lib_path(lib_name: &str) -> PathBuf { // Ideally we'd just return the ZipFile itself, but I don't know how to set // up the references correctly, ZipFile contains a borrow into the ZipArchive. // And I'm not the right Rust to keep a reference to both with proper lifetimes. +// +// We also keep the path to the APK on disk so we can re-open it for mmap +// in `open_base_lib` without re-walking the apks_dir. #[derive(Debug)] struct ZipLocation { archive: zip::ZipArchive, internal_path: String, + zip_path: PathBuf, } /// Given a zip file, check if it contains the library we want. @@ -97,6 +104,7 @@ fn check_for_lib_path(zip_path: &Path, lib_path: &str) -> anyhow::Result anyhow::Result anyhow::Result>> { +pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result> { // As far as I can tell, Android provides no apis for reading per-platform // assets (e.g. libapp.so) from an APK. Both Facebook and Chromium // seem to have written their own code to do this: @@ -172,13 +180,64 @@ pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result allocation right after a patch download is a + // plausible OOM trigger on memory-constrained devices. + if zip_file.compression() == zip::CompressionMethod::Stored { + if let Some(data_start) = zip_file.data_start() { + let len = zip_file.size(); + // Drop the ZipFile reader so we release the borrow on the + // archive and on its underlying File handle before we re-open + // the APK for mapping. + drop(zip_file); + return mmap_zip_entry(&zip_location.zip_path, data_start, len); + } + } + + // Fallback: the entry is compressed (or the zip crate didn't surface a + // data offset for some reason). Decompress fully into RAM. + shorebird_debug!("libapp.so is not stored uncompressed; falling back to buffered read"); let mut buffer = Vec::new(); zip_file.read_to_end(&mut buffer)?; - Ok(Cursor::new(buffer)) + Ok(Box::new(Cursor::new(buffer))) +} + +/// Open `zip_path` and mmap a `len`-byte region starting at `data_start`. +/// +/// Safety: the mapping aliases the file's contents; concurrent writes by +/// another process would produce torn reads or SIGBUS. APKs on Android are +/// installed read-only and not modified at runtime, so this is sound for +/// our use. We also drop the File handle after mapping — the kernel keeps +/// the mapping alive independently. +fn mmap_zip_entry(zip_path: &Path, data_start: u64, len: u64) -> anyhow::Result> { + let len_usize = usize::try_from(len) + .with_context(|| format!("entry size {} does not fit in usize", len))?; + let file = fs::File::open(zip_path) + .with_context(|| format!("Failed to reopen APK for mmap: {:?}", zip_path))?; + // SAFETY: see function-level comment. + let mmap = unsafe { + MmapOptions::new() + .offset(data_start) + .len(len_usize) + .map(&file) + .with_context(|| { + format!( + "Failed to mmap {:?} at offset {} for {} bytes", + zip_path, data_start, len + ) + })? + }; + shorebird_debug!( + "Mapped libapp.so via mmap: {} bytes at offset {} of {:?}", + len, + data_start, + zip_path + ); + Ok(Box::new(Cursor::new(mmap))) } pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result { @@ -210,9 +269,11 @@ pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result isn't Debug, so unwrap_err() doesn't apply. + let error = super::open_base_lib(tmp_dir.path(), "libapp.so") + .err() + .expect("expected open_base_lib to fail with no APKs present"); assert_error_is_file_not_found(&error); } + + /// Build an APK in `apk_path` containing a single entry whose path is + /// `lib//` and whose contents are `data`, written with + /// the given compression method. + fn write_apk_with_lib( + apk_path: &Path, + lib_name: &str, + data: &[u8], + compression: CompressionMethod, + ) { + let arch = super::android_arch_names(); + let internal_path = Path::new("lib").join(arch.lib_dir).join(lib_name); + let file = File::create(apk_path).unwrap(); + let mut zip = ZipWriter::new(file); + zip.start_file( + internal_path.to_string_lossy(), + SimpleFileOptions::default().compression_method(compression), + ) + .unwrap(); + zip.write_all(data).unwrap(); + zip.finish().unwrap(); + } + + /// When the entry is STORED uncompressed (the modern AGP default), we + /// should mmap it and the returned reader should yield the original + /// bytes via Read + Seek. + #[test] + fn open_base_lib_stored_uses_mmap_and_reads_bytes() { + let tmp_dir = TempDir::new().unwrap(); + let apk_path = tmp_dir.path().join("base.apk"); + let payload: Vec = (0..2048u32).map(|i| (i % 251) as u8).collect(); + write_apk_with_lib(&apk_path, "libapp.so", &payload, CompressionMethod::Stored); + + let mut reader = super::open_base_lib(tmp_dir.path(), "libapp.so").unwrap(); + + // Full read. + let mut buf = Vec::new(); + reader.read_to_end(&mut buf).unwrap(); + assert_eq!(buf, payload); + + // Seek + partial read — proves the returned reader satisfies + // Read + Seek (which is what bipatch needs). + reader.seek(SeekFrom::Start(100)).unwrap(); + let mut chunk = vec![0u8; 16]; + reader.read_exact(&mut chunk).unwrap(); + assert_eq!(chunk, payload[100..116]); + } + + /// When the entry is DEFLATEd, we fall back to a buffered read. The + /// returned reader should still produce the uncompressed bytes. + #[test] + fn open_base_lib_deflated_falls_back_to_buffered_read() { + let tmp_dir = TempDir::new().unwrap(); + let apk_path = tmp_dir.path().join("base.apk"); + let payload: Vec = (0..1024u32).map(|i| (i % 17) as u8).collect(); + write_apk_with_lib( + &apk_path, + "libapp.so", + &payload, + CompressionMethod::Deflated, + ); + + let mut reader = super::open_base_lib(tmp_dir.path(), "libapp.so").unwrap(); + let mut buf = Vec::new(); + reader.read_to_end(&mut buf).unwrap(); + assert_eq!(buf, payload); + } } diff --git a/library/src/updater.rs b/library/src/updater.rs index fb0ca86..908e570 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -344,8 +344,7 @@ impl ReadSeek for fs::File {} // FIXME: these patch_base functions should move to platform-specific modules where they can all be tested. #[cfg(any(target_os = "android", test))] fn patch_base(config: &UpdateConfig) -> anyhow::Result> { - let base_r = crate::android::open_base_lib(&config.libapp_path, "libapp.so")?; - Ok(Box::new(base_r)) + crate::android::open_base_lib(&config.libapp_path, "libapp.so") } #[cfg(target_os = "ios")]