From 19070119a275ac4b4ebe62434ae29523c4ecf584 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Fri, 12 May 2023 08:55:15 +0200 Subject: [PATCH] Switch FileSystemCache to use checked artifact deserialization We recently introduced safe artifact deserialization methods, which are a much saner default than the unsafe variants, and only have small performance overhead. The CLI was switched to the the new, safe deserialization, but the cache was not updated. This commit and switches the FileSystemCache implementation to use the checked deserialization variants. NOTE: The Cache::load method should also be made safe, but that is a breaking change. --- lib/cache/Cargo.toml | 3 ++- lib/cache/src/cache.rs | 6 +----- lib/cache/src/filesystem.rs | 30 ++++++++++++++++++++++++------ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/cache/Cargo.toml b/lib/cache/Cargo.toml index 8c36e61ebba..b313d6df924 100644 --- a/lib/cache/Cargo.toml +++ b/lib/cache/Cargo.toml @@ -20,6 +20,7 @@ blake3 = "1.0" criterion = "0.3" tempfile = "3.4.0" rand = "0.8.3" +wasmer = { path = "../api", version = "=3.3.0", default-features = false, features = ["sys", "cranelift"] } wasmer-compiler-singlepass = { path = "../compiler-singlepass", version = "=3.3.0" } [features] @@ -28,4 +29,4 @@ filesystem = [] blake3-pure = ["blake3/pure"] [package.metadata.docs.rs] -features = ["wasmer/sys"] \ No newline at end of file +features = ["wasmer/sys"] diff --git a/lib/cache/src/cache.rs b/lib/cache/src/cache.rs index 8d34f9cfb62..1b65057b430 100644 --- a/lib/cache/src/cache.rs +++ b/lib/cache/src/cache.rs @@ -17,11 +17,7 @@ pub trait Cache { /// /// # Safety /// This function is unsafe as the cache store could be tampered with. - unsafe fn load( - &self, - engine: &impl AsEngineRef, - key: Hash, - ) -> Result; + unsafe fn load(&self, engine: &impl AsEngineRef, key: Hash) -> Result; /// Store a [`Module`] into the cache with the given [`crate::Hash`]. fn store(&mut self, key: Hash, module: &Module) -> Result<(), Self::SerializeError>; diff --git a/lib/cache/src/filesystem.rs b/lib/cache/src/filesystem.rs index 237390c61cc..fff07b4bad1 100644 --- a/lib/cache/src/filesystem.rs +++ b/lib/cache/src/filesystem.rs @@ -92,18 +92,14 @@ impl Cache for FileSystemCache { type DeserializeError = DeserializeError; type SerializeError = SerializeError; - unsafe fn load( - &self, - engine: &impl AsEngineRef, - key: Hash, - ) -> Result { + unsafe fn load(&self, engine: &impl AsEngineRef, key: Hash) -> Result { let filename = if let Some(ref ext) = self.ext { format!("{}.{}", key, ext) } else { key.to_string() }; let path = self.path.join(filename); - let ret = Module::deserialize_from_file(engine, path.clone()); + let ret = Module::deserialize_from_file_checked(engine, path.clone()); if ret.is_err() { // If an error occurs while deserializing then we can not trust it anymore // so delete the cache file @@ -127,3 +123,25 @@ impl Cache for FileSystemCache { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_fs_cache() { + let dir = tempfile::tempdir().unwrap(); + + let mut cache = FileSystemCache::new(dir.path()).unwrap(); + + let engine = wasmer::Engine::default(); + + let bytes = include_bytes!("../../wasi/tests/envvar.wasm"); + + let module = Module::from_binary(&engine, bytes).unwrap(); + let key = Hash::generate(bytes); + + cache.store(key, &module).unwrap(); + let _restored = unsafe { cache.load(&engine, key).unwrap() }; + } +}