From 2eb60060d8eb6556ebbe411b22ee7b15ba4f7ec1 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Fri, 2 Feb 2024 12:03:58 +0800 Subject: [PATCH] Fix read and validation of misc/simd/atomic sub opcodes (#3115) The format of sub opcodes after misc, simd and atomic prefix is leb u32. The issue was found in #2921. --- RELEASE_NOTES.md | 1 + core/iwasm/compilation/aot_compiler.c | 21 +++++++++++---- core/iwasm/fast-jit/jit_frontend.c | 13 ++++++--- core/iwasm/interpreter/wasm_interp_classic.c | 8 +++++- core/iwasm/interpreter/wasm_loader.c | 28 +++++++++++++------- core/iwasm/interpreter/wasm_mini_loader.c | 26 +++++++++++++----- wamr-compiler/main.c | 6 +++-- 7 files changed, 75 insertions(+), 28 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 7570b8a7a..3fcfaf4bc 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -24,6 +24,7 @@ - fast-interp: Fix frame_offset pop order (#3101) - Fix AOT compilation on MacOS (#3102) - fast-interp: Fix block with parameter in polymorphic stack issue (#3112) +- Fix read and validation of misc/simd/atomic sub opcodes (#3115) ### Enhancements - Clear compilation warning and dead code (#3002) diff --git a/core/iwasm/compilation/aot_compiler.c b/core/iwasm/compilation/aot_compiler.c index 464ca61f3..fada0abc8 100644 --- a/core/iwasm/compilation/aot_compiler.c +++ b/core/iwasm/compilation/aot_compiler.c @@ -1050,7 +1050,9 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index) uint32 opcode1; read_leb_uint32(frame_ip, frame_ip_end, opcode1); - opcode = (uint32)opcode1; + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; #if WASM_ENABLE_BULK_MEMORY != 0 if (WASM_OP_MEMORY_INIT <= opcode @@ -1211,10 +1213,13 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index) case WASM_OP_ATOMIC_PREFIX: { uint8 bin_op, op_type; + uint32 opcode1; + + read_leb_uint32(frame_ip, frame_ip_end, opcode1); + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; - if (frame_ip < frame_ip_end) { - opcode = *frame_ip++; - } if (opcode != WASM_OP_ATOMIC_FENCE) { read_leb_uint32(frame_ip, frame_ip_end, align); read_leb_uint32(frame_ip, frame_ip_end, offset); @@ -1364,11 +1369,17 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index) #if WASM_ENABLE_SIMD != 0 case WASM_OP_SIMD_PREFIX: { + uint32 opcode1; + if (!comp_ctx->enable_simd) { goto unsupport_simd; } - opcode = *frame_ip++; + read_leb_uint32(frame_ip, frame_ip_end, opcode1); + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; + /* follow the order of enum WASMSimdEXTOpcode in wasm_opcode.h */ switch (opcode) { diff --git a/core/iwasm/fast-jit/jit_frontend.c b/core/iwasm/fast-jit/jit_frontend.c index e0b204ce9..e9d7a3ff3 100644 --- a/core/iwasm/fast-jit/jit_frontend.c +++ b/core/iwasm/fast-jit/jit_frontend.c @@ -2257,7 +2257,9 @@ jit_compile_func(JitCompContext *cc) uint32 opcode1; read_leb_uint32(frame_ip, frame_ip_end, opcode1); - opcode = (uint32)opcode1; + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; switch (opcode) { case WASM_OP_I32_TRUNC_SAT_S_F32: @@ -2396,10 +2398,13 @@ jit_compile_func(JitCompContext *cc) case WASM_OP_ATOMIC_PREFIX: { uint8 bin_op, op_type; + uint32 opcode1; + + read_leb_uint32(frame_ip, frame_ip_end, opcode1); + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; - if (frame_ip < frame_ip_end) { - opcode = *frame_ip++; - } if (opcode != WASM_OP_ATOMIC_FENCE) { read_leb_uint32(frame_ip, frame_ip_end, align); read_leb_uint32(frame_ip, frame_ip_end, offset); diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index ff167b6e9..e4da90b9c 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -3511,6 +3511,8 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, uint32 opcode1; read_leb_uint32(frame_ip, frame_ip_end, opcode1); + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ opcode = (uint8)opcode1; switch (opcode) { @@ -3843,8 +3845,12 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, HANDLE_OP(WASM_OP_ATOMIC_PREFIX) { uint32 offset = 0, align, addr; + uint32 opcode1; - opcode = *frame_ip++; + read_leb_uint32(frame_ip, frame_ip_end, opcode1); + /* opcode1 was checked in loader and is no larger than + UINT8_MAX */ + opcode = (uint8)opcode1; if (opcode != WASM_OP_ATOMIC_FENCE) { read_leb_uint32(frame_ip, frame_ip_end, align); diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index 3811f5a13..8ca3796bc 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -5092,9 +5092,13 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache, #if (WASM_ENABLE_WAMR_COMPILER != 0) || (WASM_ENABLE_JIT != 0) case WASM_OP_SIMD_PREFIX: { - /* TODO: shall we ceate a table to be friendly to branch - * prediction */ - opcode = read_uint8(p); + uint32 opcode1; + + read_leb_uint32(p, p_end, opcode1); + /* opcode1 was checked in wasm_loader_prepare_bytecode and + is no larger than UINT8_MAX */ + opcode = (uint8)opcode1; + /* follow the order of enum WASMSimdEXTOpcode in wasm_opcode.h */ switch (opcode) { @@ -5184,8 +5188,14 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache, #if WASM_ENABLE_SHARED_MEMORY != 0 case WASM_OP_ATOMIC_PREFIX: { - /* atomic_op (1 u8) + memarg (2 u32_leb) */ - opcode = read_uint8(p); + uint32 opcode1; + + /* atomic_op (u32_leb) + memarg (2 u32_leb) */ + read_leb_uint32(p, p_end, opcode1); + /* opcode1 was checked in wasm_loader_prepare_bytecode and + is no larger than UINT8_MAX */ + opcode = (uint8)opcode1; + if (opcode != WASM_OP_ATOMIC_FENCE) { skip_leb_uint32(p, p_end); /* align */ skip_leb_uint32(p, p_end); /* offset */ @@ -9836,8 +9846,8 @@ re_scan: { uint32 opcode1; - CHECK_BUF(p, p_end, 1); - opcode1 = read_uint8(p); + read_leb_uint32(p, p_end, opcode1); + /* follow the order of enum WASMSimdEXTOpcode in wasm_opcode.h */ switch (opcode1) { @@ -10498,8 +10508,8 @@ re_scan: { uint32 opcode1; - CHECK_BUF(p, p_end, 1); - opcode1 = read_uint8(p); + read_leb_uint32(p, p_end, opcode1); + #if WASM_ENABLE_FAST_INTERP != 0 emit_byte(loader_ctx, opcode1); #endif diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index e283b10c9..ba2c37fc7 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -3492,8 +3492,11 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache, uint32 opcode1; read_leb_uint32(p, p_end, opcode1); + /* opcode1 was checked in wasm_loader_prepare_bytecode and + is no larger than UINT8_MAX */ + opcode = (uint8)opcode1; - switch (opcode1) { + switch (opcode) { case WASM_OP_I32_TRUNC_SAT_S_F32: case WASM_OP_I32_TRUNC_SAT_U_F32: case WASM_OP_I32_TRUNC_SAT_S_F64: @@ -3549,8 +3552,14 @@ wasm_loader_find_block_addr(WASMExecEnv *exec_env, BlockAddr *block_addr_cache, #if WASM_ENABLE_SHARED_MEMORY != 0 case WASM_OP_ATOMIC_PREFIX: { - /* atomic_op (1 u8) + memarg (2 u32_leb) */ - opcode = read_uint8(p); + uint32 opcode1; + + /* atomic_op (u32_leb) + memarg (2 u32_leb) */ + read_leb_uint32(p, p_end, opcode1); + /* opcode1 was checked in wasm_loader_prepare_bytecode and + is no larger than UINT8_MAX */ + opcode = (uint8)opcode1; + if (opcode != WASM_OP_ATOMIC_FENCE) { skip_leb_uint32(p, p_end); /* align */ skip_leb_uint32(p, p_end); /* offset */ @@ -7464,11 +7473,14 @@ re_scan: #if WASM_ENABLE_SHARED_MEMORY != 0 case WASM_OP_ATOMIC_PREFIX: { - opcode = read_uint8(p); + uint32 opcode1; + + read_leb_uint32(p, p_end, opcode1); + #if WASM_ENABLE_FAST_INTERP != 0 - emit_byte(loader_ctx, opcode); + emit_byte(loader_ctx, opcode1); #endif - if (opcode != WASM_OP_ATOMIC_FENCE) { + if (opcode1 != WASM_OP_ATOMIC_FENCE) { CHECK_MEMORY(); read_leb_uint32(p, p_end, align); /* align */ read_leb_uint32(p, p_end, mem_offset); /* offset */ @@ -7479,7 +7491,7 @@ re_scan: #if WASM_ENABLE_JIT != 0 || WASM_ENABLE_WAMR_COMPILER != 0 func->has_memory_operations = true; #endif - switch (opcode) { + switch (opcode1) { case WASM_OP_ATOMIC_NOTIFY: POP2_AND_PUSH(VALUE_TYPE_I32, VALUE_TYPE_I32); break; diff --git a/wamr-compiler/main.c b/wamr-compiler/main.c index f2e5efafd..17c19143c 100644 --- a/wamr-compiler/main.c +++ b/wamr-compiler/main.c @@ -627,8 +627,10 @@ main(int argc, char *argv[]) goto fail1; } - if (get_package_type(wasm_file, wasm_file_size) != Wasm_Module_Bytecode) { - printf("Invalid file type: expected wasm file but got other\n"); + if (wasm_file_size >= 4 /* length of MAGIC NUMBER */ + && get_package_type(wasm_file, wasm_file_size) + != Wasm_Module_Bytecode) { + printf("Invalid wasm file: magic header not detected\n"); goto fail2; }