From f61fc5c29878bd7a02c0efa0561054436dd8d526 Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Sun, 11 Aug 2019 13:28:47 -0600 Subject: [PATCH 1/2] Validate all initializers before finalizing --- lib/runtime-core/src/backing.rs | 91 +++++++++++++++++++++++--------- lib/spectests/tests/excludes.txt | 31 +++++------ 2 files changed, 78 insertions(+), 44 deletions(-) diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs index 0803013dc87..06581d1c8df 100644 --- a/lib/runtime-core/src/backing.rs +++ b/lib/runtime-core/src/backing.rs @@ -64,8 +64,11 @@ impl LocalBacking { let mut tables = Self::generate_tables(module); let mut globals = Self::generate_globals(module, imports); - let vm_memories = Self::finalize_memories(module, imports, &mut memories)?; - let vm_tables = Self::finalize_tables(module, imports, &mut tables, vmctx)?; + Self::validate_memories(module, imports)?; + Self::validate_tables(module, imports, &mut tables)?; + + let vm_memories = Self::finalize_memories(module, imports, &mut memories); + let vm_tables = Self::finalize_tables(module, imports, &mut tables, vmctx); let vm_globals = Self::finalize_globals(&mut globals); let dynamic_sigindices = Self::generate_sigindices(&module.info); @@ -121,16 +124,10 @@ impl LocalBacking { memories.into_boxed_map() } - /// Initialize each locally-defined memory in the Module. + /// Validate each locally-defined memory in the Module. /// /// This involves copying in the data initializers. - fn finalize_memories( - module: &ModuleInner, - imports: &ImportBacking, - memories: &mut SliceMap, - ) -> LinkResult> { - // For each init that has some data... - + fn validate_memories(module: &ModuleInner, imports: &ImportBacking) -> LinkResult<()> { // Validate data size fits for init in module.info.data_initializers.iter() { let init_base = match init.base { @@ -169,7 +166,18 @@ impl LocalBacking { } } } + Ok(()) + } + /// Initialize each locally-defined memory in the Module. + /// + /// This involves copying in the data initializers. + fn finalize_memories( + module: &ModuleInner, + imports: &ImportBacking, + memories: &mut SliceMap, + ) -> BoxedMap { + // For each init that has some data... // Initialize data for init in module.info.data_initializers.iter() { let init_base = match init.base { @@ -208,11 +216,11 @@ impl LocalBacking { } } - Ok(memories + memories .iter_mut() .map(|(_, mem)| mem.vm_local_memory()) .collect::>() - .into_boxed_map()) + .into_boxed_map() } fn generate_tables(module: &ModuleInner) -> BoxedMap { @@ -226,16 +234,13 @@ impl LocalBacking { tables.into_boxed_map() } - /// This initializes all of the locally-defined tables in the Module, e.g. - /// putting all the table elements (function pointers) - /// in the right places. + /// This validates all of the locally-defined tables in the Module. #[allow(clippy::cast_ptr_alignment)] - fn finalize_tables( + fn validate_tables( module: &ModuleInner, imports: &ImportBacking, tables: &mut SliceMap, - vmctx: *mut vm::Ctx, - ) -> LinkResult> { + ) -> LinkResult<()> { for init in &module.info.elem_initializers { let init_base = match init.base { Initializer::Const(Value::I32(offset)) => offset as u32, @@ -258,7 +263,47 @@ impl LocalBacking { message: "elements segment does not fit".to_string(), }]); } + } + LocalOrImport::Import(import_table_index) => { + let table = &imports.tables[import_table_index]; + + if (table.size() as usize) < init_base + init.elements.len() { + return Err(vec![LinkError::Generic { + message: "elements segment does not fit".to_string(), + }]); + } + } + } + } + Ok(()) + } + + /// This initializes all of the locally-defined tables in the Module, e.g. + /// putting all the table elements (function pointers) + /// in the right places. + #[allow(clippy::cast_ptr_alignment)] + fn finalize_tables( + module: &ModuleInner, + imports: &ImportBacking, + tables: &mut SliceMap, + vmctx: *mut vm::Ctx, + ) -> BoxedMap { + for init in &module.info.elem_initializers { + let init_base = match init.base { + Initializer::Const(Value::I32(offset)) => offset as u32, + Initializer::Const(_) => panic!("a const initializer must be the i32 type"), + Initializer::GetGlobal(import_global_index) => { + if let Value::I32(x) = imports.globals[import_global_index].get() { + x as u32 + } else { + panic!("unsupported global type for initializer") + } + } + } as usize; + match init.table_index.local_or_import(&module.info) { + LocalOrImport::Local(local_table_index) => { + let table = &tables[local_table_index]; table.anyfunc_direct_access_mut(|elements| { for (i, &func_index) in init.elements.iter().enumerate() { let sig_index = module.info.func_assoc[func_index]; @@ -292,12 +337,6 @@ impl LocalBacking { LocalOrImport::Import(import_table_index) => { let table = &imports.tables[import_table_index]; - if (table.size() as usize) < init_base + init.elements.len() { - return Err(vec![LinkError::Generic { - message: "elements segment does not fit".to_string(), - }]); - } - table.anyfunc_direct_access_mut(|elements| { for (i, &func_index) in init.elements.iter().enumerate() { let sig_index = module.info.func_assoc[func_index]; @@ -331,11 +370,11 @@ impl LocalBacking { } } - Ok(tables + tables .iter_mut() .map(|(_, table)| table.vm_local_table()) .collect::>() - .into_boxed_map()) + .into_boxed_map() } fn generate_globals( diff --git a/lib/spectests/tests/excludes.txt b/lib/spectests/tests/excludes.txt index 6ecad1303bb..8fea271c070 100644 --- a/lib/spectests/tests/excludes.txt +++ b/lib/spectests/tests/excludes.txt @@ -25,17 +25,14 @@ clif:skip:simd_binaryen.wast:* # SIMD not implemented # linking.wast:387,388 appear to be related to WABT issue: https://github.com/pepyakin/wabt-rs/issues/51 clif:fail:globals.wast:243 # AssertInvalid - Should be invalid -clif:fail:linking.wast:137 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:139 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:142 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:144 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:147 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635037 - illegal instruction" -clif:fail:linking.wast:149 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635037 - illegal instruction" -clif:fail:linking.wast:185 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:187 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x109635062 - illegal instruction" -clif:fail:linking.wast:236 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x10963b000 - segmentation violation" -clif:fail:linking.wast:248 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x10963b000 - segmentation violation" -clif:fail:linking.wast:354 # AssertReturn - result I32(97) ("0x61") does not match expected I32(0) ("0x0") +clif:fail:linking.wast:137 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" +clif:fail:linking.wast:139 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" +clif:fail:linking.wast:142 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" +clif:fail:linking.wast:144 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" +clif:fail:linking.wast:147 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3037 - illegal instruction" +clif:fail:linking.wast:149 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3037 - illegal instruction" +clif:fail:linking.wast:185 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" +clif:fail:linking.wast:187 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x1069f3062 - illegal instruction" clif:fail:linking.wast:387 # AssertReturn - result I32(0) ("0x0") does not match expected I32(104) ("0x68") clif:fail:linking.wast:388 # AssertReturn - Call failed RuntimeError: WebAssembly trap occurred during runtime: `call_indirect` out-of-bounds @@ -839,10 +836,10 @@ llvm:fail:binary-leb128.wast:86 # Module - caught panic Any llvm:fail:binary-leb128.wast:98 # Module - caught panic Any llvm:fail:binary.wast:446 # Module - caught panic Any llvm:fail:globals.wast:243 # AssertInvalid - caught panic Any -llvm:fail:i32.wast:243 # AssertReturn - result I32(305467727) ("0x1235114f") does not match expected I32(32) ("0x20") -llvm:fail:i32.wast:252 # AssertReturn - result I32(305467744) ("0x12351160") does not match expected I32(32) ("0x20") -llvm:fail:i64.wast:243 # AssertReturn - result I64(4600435183) ("0x1123511ef") does not match expected I64(64) ("0x40") -llvm:fail:i64.wast:252 # AssertReturn - result I64(4600435168) ("0x1123511e0") does not match expected I64(64) ("0x40") +llvm:fail:i32.wast:243 # AssertReturn - result I32(205242703) ("0xc3bc14f") does not match expected I32(32) ("0x20") +llvm:fail:i32.wast:252 # AssertReturn - result I32(205242720) ("0xc3bc160") does not match expected I32(32) ("0x20") +llvm:fail:i64.wast:243 # AssertReturn - result I64(4500210159) ("0x10c3bc1ef") does not match expected I64(64) ("0x40") +llvm:fail:i64.wast:252 # AssertReturn - result I64(4500210144) ("0x10c3bc1e0") does not match expected I64(64) ("0x40") llvm:fail:imports.wast:98 # Module - caught panic Any llvm:fail:imports.wast:99 # Module - caught panic Any llvm:fail:imports.wast:100 # Module - caught panic Any @@ -893,10 +890,9 @@ llvm:fail:linking.wast:319 # AssertReturn - No instance available: Some("$Pm") llvm:fail:linking.wast:320 # AssertReturn - No instance available: Some("$Pm") llvm:fail:linking.wast:321 # AssertReturn - No instance available: Some("$Pm") llvm:fail:linking.wast:324 # AssertUnlinkable - caught panic Any -llvm:fail:linking.wast:354 # AssertReturn - result I32(97) ("0x61") does not match expected I32(0) ("0x0") llvm:fail:linking.wast:387 # AssertReturn - result I32(0) ("0x0") does not match expected I32(104) ("0x68") llvm:fail:linking.wast:388 # AssertReturn - Call failed RuntimeError: WebAssembly trap occurred during runtime: incorrect `call_indirect` signature -llvm:fail:load.wast:201 # AssertReturn - result I32(305745951) ("0x1239501f") does not match expected I32(32) ("0x20") +llvm:fail:load.wast:201 # AssertReturn - result I32(205795359) ("0xc44301f") does not match expected I32(32) ("0x20") llvm:fail:start.wast:92 # Module - caught panic Any llvm:fail:type.wast:3 # Module - caught panic Any @@ -1785,7 +1781,6 @@ singlepass:fail:linking.wast:293 # Module - caught panic Any singlepass:fail:linking.wast:299 # AssertUnlinkable - caught panic Any singlepass:fail:linking.wast:324 # AssertUnlinkable - caught panic Any singlepass:fail:linking.wast:335 # AssertUnlinkable - caught panic Any -singlepass:fail:linking.wast:354 # AssertReturn - result I32(97) ("0x61") does not match expected I32(0) ("0x0") singlepass:fail:linking.wast:387 # AssertReturn - result I32(0) ("0x0") does not match expected I32(104) ("0x68") singlepass:fail:linking.wast:388 # AssertReturn - Call failed RuntimeError: unknown error singlepass:fail:load.wast:201 # AssertReturn - result I32(0) ("0x0") does not match expected I32(32) ("0x20") From 9b4ba66e1193ffe6c83a732f01070c0a3f9d4cf6 Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Sun, 11 Aug 2019 13:59:48 -0600 Subject: [PATCH 2/2] Add code comment explanation of validation --- lib/runtime-core/src/backing.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs index 06581d1c8df..8f9c66c737b 100644 --- a/lib/runtime-core/src/backing.rs +++ b/lib/runtime-core/src/backing.rs @@ -64,6 +64,7 @@ impl LocalBacking { let mut tables = Self::generate_tables(module); let mut globals = Self::generate_globals(module, imports); + // Ensure all initializers are valid before running finalizers Self::validate_memories(module, imports)?; Self::validate_tables(module, imports, &mut tables)?;