From 2edc91cf6b77ff472ba9b08fa3dd58f0105450ea Mon Sep 17 00:00:00 2001 From: StackOverflowExcept1on <109800286+StackOverflowExcept1on@users.noreply.github.com> Date: Fri, 2 Feb 2024 15:16:30 +0300 Subject: [PATCH] feat(wasm-gen): increase fuzzer stack height limit (#3671) --- pallets/gear/src/schedule.rs | 19 ++++++++++---- sandbox/sandbox/src/embedded_executor.rs | 26 +++++++++++++++---- utils/wasm-gen/src/generator.rs | 2 -- utils/wasm-gen/src/utils.rs | 32 ------------------------ 4 files changed, 35 insertions(+), 44 deletions(-) diff --git a/pallets/gear/src/schedule.rs b/pallets/gear/src/schedule.rs index d27ee1f0dc0..281ee55ae6d 100644 --- a/pallets/gear/src/schedule.rs +++ b/pallets/gear/src/schedule.rs @@ -51,13 +51,19 @@ pub const API_BENCHMARK_BATCH_SIZE: u32 = 80; /// as for `API_BENCHMARK_BATCH_SIZE`. pub const INSTR_BENCHMARK_BATCH_SIZE: u32 = 500; -// Constant for `stack_height` is calculated via `calc-stack-height` utility to be small enough -// to avoid stack overflow in wasmer and wasmi executors. -// To avoid potential stack overflow problems we have a panic in sandbox in case, -// execution is ended with stack overflow error. So, process queue execution will be -// stopped and we will be able to investigate the problem and decrease this constant if needed. +/// Constant for `stack_height` is calculated via `calc-stack-height` utility to be small enough +/// to avoid stack overflow in wasmer and wasmi executors. +/// To avoid potential stack overflow problems we have a panic in sandbox in case, +/// execution is ended with stack overflow error. So, process queue execution will be +/// stopped and we will be able to investigate the problem and decrease this constant if needed. +#[cfg(not(feature = "fuzz"))] pub const STACK_HEIGHT_LIMIT: u32 = 36_743; +/// For the fuzzer, we take the maximum possible stack limit calculated by the `calc-stack-height` +/// utility, which would be suitable for Linux machines. This has a positive effect on code coverage. +#[cfg(feature = "fuzz")] +pub const FUZZER_STACK_HEIGHT_LIMIT: u32 = 65_000; + /// Definition of the cost schedule and other parameterization for the wasm vm. /// /// Its [`Default`] implementation is the designated way to initialize this type. It uses @@ -742,7 +748,10 @@ impl Default for Schedule { impl Default for Limits { fn default() -> Self { Self { + #[cfg(not(feature = "fuzz"))] stack_height: Some(STACK_HEIGHT_LIMIT), + #[cfg(feature = "fuzz")] + stack_height: Some(FUZZER_STACK_HEIGHT_LIMIT), globals: 256, locals: 1024, parameters: 128, diff --git a/sandbox/sandbox/src/embedded_executor.rs b/sandbox/sandbox/src/embedded_executor.rs index 9bbfd46ad22..ba2589bbcbc 100644 --- a/sandbox/sandbox/src/embedded_executor.rs +++ b/sandbox/sandbox/src/embedded_executor.rs @@ -24,12 +24,12 @@ use crate::{ }; use alloc::string::String; use gear_sandbox_env::GLOBAL_NAME_GAS; -use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; +use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, mem, prelude::*}; use sp_wasm_interface_common::HostPointer; use wasmi::{ - core::{Pages, Trap}, - Engine, ExternType, Linker, MemoryType, Module, StoreContext, StoreContextMut, - Value as RuntimeValue, + core::{Pages, Trap, UntypedValue}, + Config, Engine, ExternType, Linker, MemoryType, Module, StackLimits, StoreContext, + StoreContextMut, Value as RuntimeValue, }; /// [`AsContextExt`] extension. @@ -46,7 +46,23 @@ impl Store { impl SandboxStore for Store { fn new(state: T) -> Self { - let engine = Engine::default(); + let register_len = mem::size_of::(); + + const DEFAULT_MIN_VALUE_STACK_HEIGHT: usize = 1024; + const DEFAULT_MAX_VALUE_STACK_HEIGHT: usize = 1024 * DEFAULT_MIN_VALUE_STACK_HEIGHT; + const DEFAULT_MAX_RECURSION_DEPTH: usize = 16384; + + let mut config = Config::default(); + config.set_stack_limits( + StackLimits::new( + DEFAULT_MIN_VALUE_STACK_HEIGHT / register_len, + DEFAULT_MAX_VALUE_STACK_HEIGHT / register_len, + DEFAULT_MAX_RECURSION_DEPTH, + ) + .expect("infallible"), + ); + + let engine = Engine::new(&config); let store = wasmi::Store::new(&engine, state); Self(store) } diff --git a/utils/wasm-gen/src/generator.rs b/utils/wasm-gen/src/generator.rs index 4bfda9c3cf3..cc3c928c3eb 100644 --- a/utils/wasm-gen/src/generator.rs +++ b/utils/wasm-gen/src/generator.rs @@ -144,8 +144,6 @@ impl<'a, 'b> GearWasmGenerator<'a, 'b> { module }; - let module = utils::inject_stack_limiter(module); - Ok(if config.remove_recursions { log::trace!("Removing recursions"); utils::remove_recursion(module) diff --git a/utils/wasm-gen/src/utils.rs b/utils/wasm-gen/src/utils.rs index 30a8aaefd36..c516904bf92 100644 --- a/utils/wasm-gen/src/utils.rs +++ b/utils/wasm-gen/src/utils.rs @@ -27,7 +27,6 @@ use gear_wasm_instrument::{ }, }, syscalls::SyscallName, - wasm_instrument::{self, InjectionConfig}, }; use std::{ collections::{BTreeMap, BTreeSet}, @@ -223,37 +222,6 @@ fn find_recursion_impl( path.pop(); } -pub fn inject_stack_limiter(module: Module) -> Module { - wasm_instrument::inject_stack_limiter_with_config( - module, - InjectionConfig { - stack_limit: 30_003, - injection_fn: |signature| { - let results = signature.results(); - let mut body = Vec::with_capacity(results.len() + 1); - - for result in results { - let instruction = match result { - ValueType::I32 => Instruction::I32Const(u32::MAX as i32), - ValueType::I64 => Instruction::I64Const(u64::MAX as i64), - ValueType::F32 | ValueType::F64 => { - unreachable!("f32/64 types are not supported") - } - }; - - body.push(instruction); - } - - body.push(Instruction::Return); - - body - }, - stack_height_export_name: None, - }, - ) - .expect("Failed to inject stack height limits") -} - /// Injects a critical gas limit to a given wasm module. /// /// Code before injection gas limiter: