From 1652f22a7761b55bdc7187398fae1a8f159073d2 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Thu, 1 Dec 2022 19:24:13 +0800 Subject: [PATCH] Fix issues reported by Coverity (#1775) Fix some issues reported by Coverity and fix windows exception check with guard page issue --- core/iwasm/common/wasm_runtime_common.c | 61 +++++++++++-------- core/iwasm/compilation/aot_emit_control.c | 1 + core/iwasm/interpreter/wasm_mini_loader.c | 3 +- .../sandboxed-system-primitives/src/posix.c | 9 +-- samples/file/src/main.c | 6 +- 5 files changed, 45 insertions(+), 35 deletions(-) diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 17cf9da3..8ddf4af4 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -143,9 +143,9 @@ runtime_signal_handler(void *sig_addr) WASMJmpBuf *jmpbuf_node; uint8 *mapped_mem_start_addr = NULL; uint8 *mapped_mem_end_addr = NULL; + uint32 page_size = os_getpagesize(); #if WASM_DISABLE_STACK_HW_BOUND_CHECK == 0 uint8 *stack_min_addr; - uint32 page_size; uint32 guard_page_count = STACK_OVERFLOW_CHECK_GUARD_PAGE_COUNT; #endif @@ -163,7 +163,6 @@ runtime_signal_handler(void *sig_addr) #if WASM_DISABLE_STACK_HW_BOUND_CHECK == 0 /* Get stack info of current thread */ - page_size = os_getpagesize(); stack_min_addr = os_thread_get_stack_boundary(); #endif @@ -216,29 +215,41 @@ runtime_exception_handler(EXCEPTION_POINTERS *exce_info) mapped_mem_start_addr = memory_inst->memory_data; mapped_mem_end_addr = memory_inst->memory_data + 8 * (uint64)BH_GB; - if (mapped_mem_start_addr <= (uint8 *)sig_addr - && (uint8 *)sig_addr < mapped_mem_end_addr) { - /* The address which causes segmentation fault is inside - the memory instance's guard regions. - Set exception and let the wasm func continue to run, when - the wasm func returns, the caller will check whether the - exception is thrown and return to runtime. */ - wasm_set_exception(module_inst, - "out of bounds memory access"); - if (module_inst->module_type == Wasm_Module_Bytecode) { - /* Continue to search next exception handler for - interpreter mode as it can be caught by - `__try { .. } __except { .. }` sentences in - wasm_runtime.c */ - return EXCEPTION_CONTINUE_SEARCH; - } - else { - /* Skip current instruction and continue to run for - AOT mode. TODO: implement unwind support for AOT - code in Windows platform */ - exce_info->ContextRecord->Rip++; - return EXCEPTION_CONTINUE_EXECUTION; - } + } + + if (memory_inst && mapped_mem_start_addr <= (uint8 *)sig_addr + && (uint8 *)sig_addr < mapped_mem_end_addr) { + /* The address which causes segmentation fault is inside + the memory instance's guard regions. + Set exception and let the wasm func continue to run, when + the wasm func returns, the caller will check whether the + exception is thrown and return to runtime. */ + wasm_set_exception(module_inst, "out of bounds memory access"); + if (module_inst->module_type == Wasm_Module_Bytecode) { + /* Continue to search next exception handler for + interpreter mode as it can be caught by + `__try { .. } __except { .. }` sentences in + wasm_runtime.c */ + return EXCEPTION_CONTINUE_SEARCH; + } + else { + /* Skip current instruction and continue to run for + AOT mode. TODO: implement unwind support for AOT + code in Windows platform */ + exce_info->ContextRecord->Rip++; + return EXCEPTION_CONTINUE_EXECUTION; + } + } + else if (exec_env_tls->exce_check_guard_page <= (uint8 *)sig_addr + && (uint8 *)sig_addr + < exec_env_tls->exce_check_guard_page + page_size) { + bh_assert(wasm_get_exception(module_inst)); + if (module_inst->module_type == Wasm_Module_Bytecode) { + return EXCEPTION_CONTINUE_SEARCH; + } + else { + exce_info->ContextRecord->Rip++; + return EXCEPTION_CONTINUE_EXECUTION; } } } diff --git a/core/iwasm/compilation/aot_emit_control.c b/core/iwasm/compilation/aot_emit_control.c index 6d452b04..68c286f4 100644 --- a/core/iwasm/compilation/aot_emit_control.c +++ b/core/iwasm/compilation/aot_emit_control.c @@ -462,6 +462,7 @@ aot_compile_op_block(AOTCompContext *comp_ctx, AOTFuncContext *func_ctx, false, NULL, NULL))) { goto fail; } + aot_block_destroy(block); return aot_handle_next_reachable_block(comp_ctx, func_ctx, p_frame_ip); } diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index f009ca87..af116a82 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -5125,10 +5125,11 @@ copy_params_to_dynamic_space(WASMLoaderContext *loader_ctx, bool is_if_block, /* Free the emit data */ wasm_runtime_free(emit_data); - return true; fail: + /* Free the emit data */ + wasm_runtime_free(emit_data); return false; } #endif diff --git a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c index a1e3e373..0d44ae7d 100644 --- a/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c +++ b/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c @@ -3081,14 +3081,15 @@ wasi_ssp_sock_addr_resolve( size_t _max_info_size; size_t actual_info_size; - if (!ns_lookup_list_search(ns_lookup_list, host)) { - return __WASI_EACCES; - } - if (!wamr_addr_info) { return __WASI_ENOMEM; } + if (!ns_lookup_list_search(ns_lookup_list, host)) { + wasm_runtime_free(wamr_addr_info); + return __WASI_EACCES; + } + int ret = os_socket_addr_resolve( host, service, hints->hints_enabled ? &hints_is_tcp : NULL, hints->hints_enabled ? &hints_is_ipv4 : NULL, wamr_addr_info, diff --git a/samples/file/src/main.c b/samples/file/src/main.c index de5eb291..2fb99e25 100644 --- a/samples/file/src/main.c +++ b/samples/file/src/main.c @@ -26,7 +26,6 @@ main(int argc, char *argv_main[]) wasm_module_inst_t module_inst = NULL; wasm_exec_env_t exec_env = NULL; uint32 buf_size, stack_size = 8092, heap_size = 8092; - uint32_t wasm_buffer = 0; RuntimeInitArgs init_args; memset(&init_args, 0, sizeof(RuntimeInitArgs)); @@ -103,11 +102,8 @@ main(int argc, char *argv_main[]) fail: if (exec_env) wasm_runtime_destroy_exec_env(exec_env); - if (module_inst) { - if (wasm_buffer) - wasm_runtime_module_free(module_inst, wasm_buffer); + if (module_inst) wasm_runtime_deinstantiate(module_inst); - } if (module) wasm_runtime_unload(module); if (buffer)