From 0b73776c84b184d2a2b7f93f0ff1973d14c4e1a5 Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Sat, 10 Aug 2019 17:10:24 -0600 Subject: [PATCH 1/3] Validate all memory data initializers before writing --- lib/runtime-core/src/backing.rs | 44 +++++++++++++++++++++++++------- lib/spectests/tests/excludes.txt | 36 +++++++++++++------------- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs index d7b2a72352f..272dc225159 100644 --- a/lib/runtime-core/src/backing.rs +++ b/lib/runtime-core/src/backing.rs @@ -130,6 +130,8 @@ impl LocalBacking { memories: &mut SliceMap, ) -> LinkResult> { // For each init that has some data... + + // Validate data size fits for init in module.info.data_initializers.iter() { let init_base = match init.base { Initializer::Const(Value::I32(offset)) => offset as u32, @@ -143,17 +145,49 @@ impl LocalBacking { } } as usize; + // Validate data size fits match init.memory_index.local_or_import(&module.info) { LocalOrImport::Local(local_memory_index) => { let memory_desc = module.info.memories[local_memory_index]; let data_top = init_base + init.data.len(); - if memory_desc.minimum.bytes().0 < data_top || data_top < init_base { return Err(vec![LinkError::Generic { message: "data segment does not fit".to_string(), }]); } + } + LocalOrImport::Import(imported_memory_index) => { + // Write the initialization data to the memory that + // we think the imported memory is. + unsafe { + let local_memory = &*imports.vm_memories[imported_memory_index]; + let data_top = init_base + init.data.len(); + if local_memory.bound < data_top || data_top < init_base { + return Err(vec![LinkError::Generic { + message: "data segment does not fit".to_string(), + }]); + } + } + } + } + } + // Initialize data + for init in module.info.data_initializers.iter() { + 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.memory_index.local_or_import(&module.info) { + LocalOrImport::Local(local_memory_index) => { let mem = &memories[local_memory_index]; for (mem_byte, data_byte) in mem.view()[init_base..init_base + init.data.len()] .iter() @@ -167,14 +201,6 @@ impl LocalBacking { // we think the imported memory is. unsafe { let local_memory = &*imports.vm_memories[imported_memory_index]; - - let data_top = init_base + init.data.len(); - if local_memory.bound < data_top || data_top < init_base { - return Err(vec![LinkError::Generic { - message: "data segment does not fit".to_string(), - }]); - } - let memory_slice = slice::from_raw_parts_mut(local_memory.base, local_memory.bound); diff --git a/lib/spectests/tests/excludes.txt b/lib/spectests/tests/excludes.txt index 3b1beaf819a..49186db65ad 100644 --- a/lib/spectests/tests/excludes.txt +++ b/lib/spectests/tests/excludes.txt @@ -22,23 +22,24 @@ clif:skip:names.wast:* # Names file has parsing error? clif:skip:simd.wast:* # SIMD not implemented 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 0x108837062 - illegal instruction" -clif:fail:linking.wast:139 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837062 - illegal instruction" -clif:fail:linking.wast:142 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837062 - illegal instruction" -clif:fail:linking.wast:144 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837062 - illegal instruction" -clif:fail:linking.wast:147 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837037 - illegal instruction" -clif:fail:linking.wast:149 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837037 - illegal instruction" -clif:fail:linking.wast:185 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837062 - illegal instruction" -clif:fail:linking.wast:187 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x108837062 - illegal instruction" -clif:fail:linking.wast:236 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x10883d000 - segmentation violation" -clif:fail:linking.wast:248 # AssertTrap - expected trap, got Runtime:Error "unknown trap at 0x10883d000 - segmentation violation" -clif:fail:linking.wast:342 # 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 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: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 -# clif:skip:skip-stack-guard-page.wast:2 # Slow test +clif:skip:skip-stack-guard-page.wast:2 # Slow test # Cranelift Windows clif:skip:address.wast:*:windows @@ -838,10 +839,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(115568975) ("0x6e3714f") does not match expected I32(32) ("0x20") -llvm:fail:i32.wast:252 # AssertReturn - result I32(115568992) ("0x6e37160") does not match expected I32(32) ("0x20") -llvm:fail:i64.wast:243 # AssertReturn - result I64(4410536431) ("0x106e371ef") does not match expected I64(64) ("0x40") -llvm:fail:i64.wast:252 # AssertReturn - result I64(4410536416) ("0x106e371e0") does not match expected I64(64) ("0x40") +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: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 @@ -892,11 +893,10 @@ 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:342 # AssertReturn - result I32(97) ("0x61") does not match expected I32(0) ("0x0") 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(118083615) ("0x709d01f") does not match expected I32(32) ("0x20") +llvm:fail:load.wast:201 # AssertReturn - result I32(305745951) ("0x1239501f") 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 From b7970fb982ea98e2eb5eecc23c5c1ed06971ea3e Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Sat, 10 Aug 2019 17:11:32 -0600 Subject: [PATCH 2/3] Uncomment slow exclude test again --- lib/spectests/tests/excludes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spectests/tests/excludes.txt b/lib/spectests/tests/excludes.txt index 49186db65ad..6ecad1303bb 100644 --- a/lib/spectests/tests/excludes.txt +++ b/lib/spectests/tests/excludes.txt @@ -39,7 +39,7 @@ clif:fail:linking.wast:354 # AssertReturn - result I32(97) ("0x61") does not mat 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 -clif:skip:skip-stack-guard-page.wast:2 # Slow test +# clif:skip:skip-stack-guard-page.wast:2 # Slow test # Cranelift Windows clif:skip:address.wast:*:windows From 38a8a0eb01d2802f7a5899e7276653d075f21a2d Mon Sep 17 00:00:00 2001 From: Brandon Fish Date: Sat, 10 Aug 2019 17:20:27 -0600 Subject: [PATCH 3/3] Minimize unsafe block to unsafe code --- lib/runtime-core/src/backing.rs | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs index 272dc225159..0803013dc87 100644 --- a/lib/runtime-core/src/backing.rs +++ b/lib/runtime-core/src/backing.rs @@ -159,14 +159,12 @@ impl LocalBacking { LocalOrImport::Import(imported_memory_index) => { // Write the initialization data to the memory that // we think the imported memory is. - unsafe { - let local_memory = &*imports.vm_memories[imported_memory_index]; - let data_top = init_base + init.data.len(); - if local_memory.bound < data_top || data_top < init_base { - return Err(vec![LinkError::Generic { - message: "data segment does not fit".to_string(), - }]); - } + let local_memory = unsafe { &*imports.vm_memories[imported_memory_index] }; + let data_top = init_base + init.data.len(); + if local_memory.bound < data_top || data_top < init_base { + return Err(vec![LinkError::Generic { + message: "data segment does not fit".to_string(), + }]); } } } @@ -199,15 +197,13 @@ impl LocalBacking { LocalOrImport::Import(imported_memory_index) => { // Write the initialization data to the memory that // we think the imported memory is. - unsafe { + let memory_slice = unsafe { let local_memory = &*imports.vm_memories[imported_memory_index]; - let memory_slice = - slice::from_raw_parts_mut(local_memory.base, local_memory.bound); + slice::from_raw_parts_mut(local_memory.base, local_memory.bound) + }; - let mem_init_view = - &mut memory_slice[init_base..init_base + init.data.len()]; - mem_init_view.copy_from_slice(&init.data); - } + let mem_init_view = &mut memory_slice[init_base..init_base + init.data.len()]; + mem_init_view.copy_from_slice(&init.data); } } }