From f19ead377bad466f5b0b16af71ced9f1f780e3b6 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 7 Jan 2022 17:14:11 +0100 Subject: [PATCH 1/2] Use a standard header for metadata in all serialized modules This header includes an ABI version which will reject any serialized modules from an incompatible version of Wasmer. Also, it replaces all of the custom per-engine code for encoding the metadata length. --- lib/engine-dylib/src/artifact.rs | 28 +++++----- lib/engine-staticlib/src/artifact.rs | 18 +++---- lib/engine-universal/src/artifact.rs | 77 ++++------------------------ lib/engine/src/artifact.rs | 70 ++++++++++++++++++++++++- lib/engine/src/lib.rs | 2 +- 5 files changed, 101 insertions(+), 94 deletions(-) diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs index e1a40cde4fe..248a2d04cd3 100644 --- a/lib/engine-dylib/src/artifact.rs +++ b/lib/engine-dylib/src/artifact.rs @@ -28,7 +28,7 @@ use wasmer_compiler::{ }; use wasmer_engine::{ register_frame_info, Artifact, DeserializeError, FunctionExtent, GlobalFrameInfoRegistration, - InstantiationError, SerializeError, + InstantiationError, MetadataHeader, SerializeError, }; #[cfg(feature = "compiler")] use wasmer_engine::{Engine, Tunables}; @@ -217,10 +217,8 @@ impl DylibArtifact { let serialized_data = metadata.serialize()?; - let mut metadata_binary = vec![0; 16]; - let mut writable = &mut metadata_binary[..]; - leb128::write::unsigned(&mut writable, serialized_data.len() as u64) - .expect("Should write number"); + let mut metadata_binary = vec![]; + metadata_binary.extend(MetadataHeader::new(serialized_data.len())); metadata_binary.extend(serialized_data); let (compile_info, symbol_registry) = metadata.split(); @@ -256,8 +254,13 @@ impl DylibArtifact { function_body_inputs, )?; let mut obj = get_object_for_target(&target_triple).map_err(to_compile_error)?; - emit_data(&mut obj, WASMER_METADATA_SYMBOL, &metadata_binary, 16) - .map_err(to_compile_error)?; + emit_data( + &mut obj, + WASMER_METADATA_SYMBOL, + &metadata_binary, + MetadataHeader::ALIGN as u64, + ) + .map_err(to_compile_error)?; emit_compilation(&mut obj, compilation, &symbol_registry, &target_triple) .map_err(to_compile_error)?; let file = tempfile::Builder::new() @@ -618,9 +621,7 @@ impl DylibArtifact { DeserializeError::CorruptedBinary(format!("Library loading failed: {}", e)) })?; let shared_path: PathBuf = PathBuf::from(path); - // We reserve 16 bytes for the length because the rest of the metadata - // needs to be aligned to 16 bytes. - let symbol: LibrarySymbol<*mut [u8; 16]> = + let symbol: LibrarySymbol<*mut [u8; MetadataHeader::LEN]> = lib.get(WASMER_METADATA_SYMBOL).map_err(|e| { DeserializeError::CorruptedBinary(format!( "The provided object file doesn't seem to be generated by Wasmer: {}", @@ -630,12 +631,9 @@ impl DylibArtifact { use std::slice; let metadata = &**symbol; - let mut readable = &metadata[..]; - let metadata_len = leb128::read::unsigned(&mut readable).map_err(|_e| { - DeserializeError::CorruptedBinary("Can't read metadata size".to_string()) - })?; + let metadata_len = MetadataHeader::parse(metadata)?; let metadata_slice: &'static [u8] = - slice::from_raw_parts(metadata.as_ptr().add(16), metadata_len as usize); + slice::from_raw_parts(metadata.as_ptr().add(MetadataHeader::LEN), metadata_len); let metadata = ModuleMetadata::deserialize(metadata_slice)?; diff --git a/lib/engine-staticlib/src/artifact.rs b/lib/engine-staticlib/src/artifact.rs index cf25fb4808f..400a4c91693 100644 --- a/lib/engine-staticlib/src/artifact.rs +++ b/lib/engine-staticlib/src/artifact.rs @@ -17,7 +17,9 @@ use wasmer_compiler::{ CompileModuleInfo, Compiler, FunctionBodyData, ModuleEnvironment, ModuleMiddlewareChain, ModuleTranslationState, }; -use wasmer_engine::{Artifact, DeserializeError, InstantiationError, SerializeError}; +use wasmer_engine::{ + Artifact, DeserializeError, InstantiationError, MetadataHeader, SerializeError, +}; #[cfg(feature = "compiler")] use wasmer_engine::{Engine, Tunables}; #[cfg(feature = "compiler")] @@ -206,10 +208,8 @@ impl StaticlibArtifact { */ let serialized_data = bincode::serialize(&metadata).map_err(to_compile_error)?; - let mut metadata_binary = vec![0; 10]; - let mut writable = &mut metadata_binary[..]; - leb128::write::unsigned(&mut writable, serialized_data.len() as u64) - .expect("Should write number"); + let mut metadata_binary = vec![]; + metadata_binary.extend(MetadataHeader::new(serialized_data.len())); metadata_binary.extend(serialized_data); let metadata_length = metadata_binary.len(); @@ -321,15 +321,15 @@ impl StaticlibArtifact { engine: &StaticlibEngine, bytes: &[u8], ) -> Result { - let mut reader = bytes; - let data_len = leb128::read::unsigned(&mut reader).unwrap() as usize; + let metadata_len = MetadataHeader::parse(bytes)?; - let metadata: ModuleMetadata = bincode::deserialize(&bytes[10..(data_len + 10)]).unwrap(); + let metadata: ModuleMetadata = + bincode::deserialize(&bytes[MetadataHeader::LEN..][..metadata_len]).unwrap(); const WORD_SIZE: usize = mem::size_of::(); let mut byte_buffer = [0u8; WORD_SIZE]; - let mut cur_offset = data_len + 10; + let mut cur_offset = MetadataHeader::LEN + metadata_len; byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]); cur_offset += WORD_SIZE; diff --git a/lib/engine-universal/src/artifact.rs b/lib/engine-universal/src/artifact.rs index 61a7aeabe0d..b9c700b0b2e 100644 --- a/lib/engine-universal/src/artifact.rs +++ b/lib/engine-universal/src/artifact.rs @@ -8,13 +8,14 @@ use crate::serialize::SerializableCompilation; use crate::serialize::SerializableModule; use enumset::EnumSet; use loupe::MemoryUsage; +use std::mem; use std::sync::{Arc, Mutex}; use wasmer_compiler::{CompileError, CpuFeature, Features, Triple}; #[cfg(feature = "compiler")] use wasmer_compiler::{CompileModuleInfo, ModuleEnvironment, ModuleMiddlewareChain}; use wasmer_engine::{ register_frame_info, Artifact, DeserializeError, FunctionExtent, GlobalFrameInfoRegistration, - SerializeError, + MetadataHeader, SerializeError, }; #[cfg(feature = "compiler")] use wasmer_engine::{Engine, Tunables}; @@ -28,9 +29,6 @@ use wasmer_vm::{ VMTrampoline, }; -const SERIALIZED_METADATA_LENGTH_OFFSET: usize = 22; -const SERIALIZED_METADATA_CONTENT_OFFSET: usize = 32; - /// A compiled wasm module, ready to be instantiated. #[derive(MemoryUsage)] pub struct UniversalArtifact { @@ -46,11 +44,9 @@ pub struct UniversalArtifact { } impl UniversalArtifact { - const MAGIC_HEADER: &'static [u8; 22] = b"\0wasmer-universal\0\0\0\0\0"; - /// Check if the provided bytes look like a serialized `UniversalArtifact`. pub fn is_deserializable(bytes: &[u8]) -> bool { - bytes.starts_with(Self::MAGIC_HEADER) + MetadataHeader::parse(bytes).is_ok() } /// Compile a data buffer into a `UniversalArtifact`, which may then be instantiated. @@ -151,22 +147,8 @@ impl UniversalArtifact { universal: &UniversalEngine, bytes: &[u8], ) -> Result { - if !Self::is_deserializable(bytes) { - return Err(DeserializeError::Incompatible( - "The provided bytes are not wasmer-universal".to_string(), - )); - } - - let mut inner_bytes = &bytes[SERIALIZED_METADATA_LENGTH_OFFSET..]; - - let metadata_len = leb128::read::unsigned(&mut inner_bytes).map_err(|_e| { - DeserializeError::CorruptedBinary("Can't read metadata size".to_string()) - })?; - let metadata_slice: &[u8] = std::slice::from_raw_parts( - &bytes[SERIALIZED_METADATA_CONTENT_OFFSET] as *const u8, - metadata_len as usize, - ); - + let metadata_len = MetadataHeader::parse(bytes)?; + let metadata_slice: &[u8] = &bytes[MetadataHeader::LEN..][..metadata_len]; let serializable = SerializableModule::deserialize(metadata_slice)?; Self::from_parts(&mut universal.inner_mut(), serializable) .map_err(DeserializeError::Compiler) @@ -345,51 +327,12 @@ impl Artifact for UniversalArtifact { &self.func_data_registry } fn serialize(&self) -> Result, SerializeError> { - // Prepend the header. - let mut serialized = Self::MAGIC_HEADER.to_vec(); - - serialized.resize(SERIALIZED_METADATA_CONTENT_OFFSET, 0); - let mut writable_leb = &mut serialized[SERIALIZED_METADATA_LENGTH_OFFSET..]; let serialized_data = self.serializable.serialize()?; - let length = serialized_data.len(); - leb128::write::unsigned(&mut writable_leb, length as u64).expect("Should write number"); - - let offset = pad_and_extend::(&mut serialized, &serialized_data); - assert_eq!(offset, SERIALIZED_METADATA_CONTENT_OFFSET); + assert!(mem::align_of::() <= MetadataHeader::ALIGN); - Ok(serialized) - } -} - -/// It pads the data with the desired alignment -pub fn pad_and_extend(prev_data: &mut Vec, data: &[u8]) -> usize { - let align = std::mem::align_of::(); - - let mut offset = prev_data.len(); - if offset & (align - 1) != 0 { - offset += align - (offset & (align - 1)); - prev_data.resize(offset, 0); - } - prev_data.extend(data); - offset -} - -#[cfg(test)] -mod tests { - use super::pad_and_extend; - - #[test] - fn test_pad_and_extend() { - let mut data: Vec = vec![]; - let offset = pad_and_extend::(&mut data, &[1, 0, 0, 0, 0, 0, 0, 0]); - assert_eq!(offset, 0); - let offset = pad_and_extend::(&mut data, &[2, 0, 0, 0]); - assert_eq!(offset, 8); - let offset = pad_and_extend::(&mut data, &[3, 0, 0, 0, 0, 0, 0, 0]); - assert_eq!(offset, 16); - assert_eq!( - data, - &[1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0] - ); + let mut metadata_binary = vec![]; + metadata_binary.extend(MetadataHeader::new(serialized_data.len())); + metadata_binary.extend(serialized_data); + Ok(metadata_binary) } } diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index 59356293c09..0a5c5df1797 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -1,12 +1,14 @@ use crate::{ - resolve_imports, InstantiationError, Resolver, RuntimeError, SerializeError, Tunables, + resolve_imports, DeserializeError, InstantiationError, Resolver, RuntimeError, SerializeError, + Tunables, }; use enumset::EnumSet; use loupe::MemoryUsage; use std::any::Any; -use std::fs; +use std::convert::TryInto; use std::path::Path; use std::sync::Arc; +use std::{fs, mem}; use wasmer_compiler::{CpuFeature, Features}; use wasmer_types::entity::{BoxedSlice, PrimaryMap}; use wasmer_types::{ @@ -227,3 +229,67 @@ impl dyn Artifact + 'static { self.upcast_any_mut().downcast_mut::() } } + +/// Metadata header which holds an ABI version and the length of the remaining +/// metadata. +#[repr(C)] +#[derive(Clone, Copy)] +pub struct MetadataHeader { + magic: [u8; 8], + version: u32, + len: u32, +} + +impl MetadataHeader { + /// Current ABI version. Increment this any time breaking changes are made + /// to the format of the serialized data. + const CURRENT_VERSION: u32 = 1; + + /// Magic number to identify wasmer metadata. + const MAGIC: [u8; 8] = *b"WASMER\0\0"; + + /// Length of the metadata header. + pub const LEN: usize = 16; + + /// Alignment of the metadata. + pub const ALIGN: usize = 16; + + /// Creates a new header for metadata of the given length. + pub fn new(len: usize) -> [u8; 16] { + let header = MetadataHeader { + magic: Self::MAGIC, + version: Self::CURRENT_VERSION, + len: len.try_into().expect("metadata exceeds maximum length"), + }; + unsafe { mem::transmute(header) } + } + + /// Parses the header and returns the length of the metadata following it. + pub fn parse(bytes: &[u8]) -> Result { + if bytes.as_ptr() as usize % 16 != 0 { + return Err(DeserializeError::CorruptedBinary( + "misaligned metadata".to_string(), + )); + } + let bytes: [u8; 16] = bytes + .get(..16) + .ok_or_else(|| { + DeserializeError::CorruptedBinary("invalid metadata header".to_string()) + })? + .try_into() + .unwrap(); + let header: MetadataHeader = unsafe { mem::transmute(bytes) }; + if header.magic != Self::MAGIC { + return Err(DeserializeError::Incompatible( + "The provided bytes were not serialized by Wasmer".to_string(), + )); + } + if header.version != Self::CURRENT_VERSION { + return Err(DeserializeError::Incompatible( + "The provided bytes were serialized by an incompatible version of Wasmer" + .to_string(), + )); + } + Ok(header.len as usize) + } +} diff --git a/lib/engine/src/lib.rs b/lib/engine/src/lib.rs index 9cc5f1a68b9..966cd03cfd9 100644 --- a/lib/engine/src/lib.rs +++ b/lib/engine/src/lib.rs @@ -28,7 +28,7 @@ mod resolver; mod trap; mod tunables; -pub use crate::artifact::Artifact; +pub use crate::artifact::{Artifact, MetadataHeader}; pub use crate::engine::{Engine, EngineId}; pub use crate::error::{ DeserializeError, ImportError, InstantiationError, LinkError, SerializeError, From fb73cf4f8be5d45fd75962bd7117e51e8d595a61 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 13 Jan 2022 23:43:26 +0000 Subject: [PATCH 2/2] Re-add a magic header for engine-universal --- lib/engine-universal/src/artifact.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/engine-universal/src/artifact.rs b/lib/engine-universal/src/artifact.rs index b9c700b0b2e..8163b8718d0 100644 --- a/lib/engine-universal/src/artifact.rs +++ b/lib/engine-universal/src/artifact.rs @@ -44,9 +44,11 @@ pub struct UniversalArtifact { } impl UniversalArtifact { + const MAGIC_HEADER: &'static [u8; 16] = b"wasmer-universal"; + /// Check if the provided bytes look like a serialized `UniversalArtifact`. pub fn is_deserializable(bytes: &[u8]) -> bool { - MetadataHeader::parse(bytes).is_ok() + bytes.starts_with(Self::MAGIC_HEADER) } /// Compile a data buffer into a `UniversalArtifact`, which may then be instantiated. @@ -147,6 +149,12 @@ impl UniversalArtifact { universal: &UniversalEngine, bytes: &[u8], ) -> Result { + if !Self::is_deserializable(bytes) { + return Err(DeserializeError::Incompatible( + "The provided bytes are not wasmer-universal".to_string(), + )); + } + let bytes = &bytes[Self::MAGIC_HEADER.len()..]; let metadata_len = MetadataHeader::parse(bytes)?; let metadata_slice: &[u8] = &bytes[MetadataHeader::LEN..][..metadata_len]; let serializable = SerializableModule::deserialize(metadata_slice)?; @@ -331,6 +339,7 @@ impl Artifact for UniversalArtifact { assert!(mem::align_of::() <= MetadataHeader::ALIGN); let mut metadata_binary = vec![]; + metadata_binary.extend(Self::MAGIC_HEADER); metadata_binary.extend(MetadataHeader::new(serialized_data.len())); metadata_binary.extend(serialized_data); Ok(metadata_binary)