From bc60064de55e9df9dfac8112eab903f55272d84f Mon Sep 17 00:00:00 2001 From: Enrico Loparco Date: Wed, 22 Feb 2023 04:43:49 +0100 Subject: [PATCH] Fix data races in atomic wait/notify and interp goto table (#1971) Add shared memory lock when accessing the address to atomic wait/notify inside linear memory to resolve its data race issue. And statically initialize the goto table of interpreter labels to resolve the data race issue of accessing the table. --- core/iwasm/common/wasm_shared_memory.c | 36 ++++++++++++++++++++------ core/iwasm/interpreter/wasm_opcode.h | 20 +++++++------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/core/iwasm/common/wasm_shared_memory.c b/core/iwasm/common/wasm_shared_memory.c index 3a9c1f59..6295de01 100644 --- a/core/iwasm/common/wasm_shared_memory.c +++ b/core/iwasm/common/wasm_shared_memory.c @@ -157,13 +157,13 @@ int32 shared_memory_inc_reference(WASMModuleCommon *module) { WASMSharedMemNode *node = search_module(module); + uint32 ref_count = -1; if (node) { os_mutex_lock(&node->lock); - node->ref_count++; + ref_count = ++node->ref_count; os_mutex_unlock(&node->lock); - return node->ref_count; } - return -1; + return ref_count; } int32 @@ -385,7 +385,8 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address, WASMModuleInstance *module_inst = (WASMModuleInstance *)module; AtomicWaitInfo *wait_info; AtomicWaitNode *wait_node; - bool check_ret, is_timeout; + WASMSharedMemNode *node; + bool check_ret, is_timeout, no_wait; bh_assert(module->module_type == Wasm_Module_Bytecode || module->module_type == Wasm_Module_AoT); @@ -417,8 +418,13 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address, os_mutex_lock(&wait_info->wait_list_lock); - if ((!wait64 && *(uint32 *)address != (uint32)expect) - || (wait64 && *(uint64 *)address != expect)) { + node = search_module((WASMModuleCommon *)module_inst->module); + os_mutex_lock(&node->shared_mem_lock); + no_wait = (!wait64 && *(uint32 *)address != (uint32)expect) + || (wait64 && *(uint64 *)address != expect); + os_mutex_unlock(&node->shared_mem_lock); + + if (no_wait) { os_mutex_unlock(&wait_info->wait_list_lock); return 1; } @@ -476,7 +482,9 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address, wasm_runtime_free(wait_node); os_mutex_unlock(&wait_info->wait_list_lock); + os_mutex_lock(&node->shared_mem_lock); release_wait_info(wait_map, wait_info, address); + os_mutex_unlock(&node->shared_mem_lock); (void)check_ret; return is_timeout ? 2 : 0; @@ -489,17 +497,29 @@ wasm_runtime_atomic_notify(WASMModuleInstanceCommon *module, void *address, WASMModuleInstance *module_inst = (WASMModuleInstance *)module; uint32 notify_result; AtomicWaitInfo *wait_info; + WASMSharedMemNode *node; + bool out_of_bounds; bh_assert(module->module_type == Wasm_Module_Bytecode || module->module_type == Wasm_Module_AoT); - if ((uint8 *)address < module_inst->memories[0]->memory_data - || (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end) { + node = search_module((WASMModuleCommon *)module_inst->module); + if (node) + os_mutex_lock(&node->shared_mem_lock); + out_of_bounds = + ((uint8 *)address < module_inst->memories[0]->memory_data + || (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end); + + if (out_of_bounds) { + if (node) + os_mutex_unlock(&node->shared_mem_lock); wasm_runtime_set_exception(module, "out of bounds memory access"); return -1; } wait_info = acquire_wait_info(address, false); + if (node) + os_mutex_unlock(&node->shared_mem_lock); /* Nobody wait on this address */ if (!wait_info) diff --git a/core/iwasm/interpreter/wasm_opcode.h b/core/iwasm/interpreter/wasm_opcode.h index cd7478a6..ce5e358a 100644 --- a/core/iwasm/interpreter/wasm_opcode.h +++ b/core/iwasm/interpreter/wasm_opcode.h @@ -675,12 +675,14 @@ typedef enum WASMAtomicEXTOpcode { } WASMAtomicEXTOpcode; #if WASM_ENABLE_DEBUG_INTERP != 0 -#define DEF_DEBUG_BREAK_HANDLE(_name) \ - _name[DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK); /* 0xd7 */ +#define DEF_DEBUG_BREAK_HANDLE() \ + [DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK), /* 0xd7 */ #else -#define DEF_DEBUG_BREAK_HANDLE(_name) +#define DEF_DEBUG_BREAK_HANDLE() #endif +#define SET_GOTO_TABLE_ELEM(opcode) [opcode] = HANDLE_OPCODE(opcode) + /* * Macro used to generate computed goto tables for the C interpreter. */ @@ -903,14 +905,10 @@ typedef enum WASMAtomicEXTOpcode { HANDLE_OPCODE(EXT_OP_LOOP), /* 0xd4 */ \ HANDLE_OPCODE(EXT_OP_IF), /* 0xd5 */ \ HANDLE_OPCODE(EXT_OP_BR_TABLE_CACHE), /* 0xd6 */ \ - }; \ - do { \ - _name[WASM_OP_MISC_PREFIX] = \ - HANDLE_OPCODE(WASM_OP_MISC_PREFIX); /* 0xfc */ \ - _name[WASM_OP_ATOMIC_PREFIX] = \ - HANDLE_OPCODE(WASM_OP_ATOMIC_PREFIX); /* 0xfe */ \ - DEF_DEBUG_BREAK_HANDLE(_name) \ - } while (0) + SET_GOTO_TABLE_ELEM(WASM_OP_MISC_PREFIX), /* 0xfc */ \ + SET_GOTO_TABLE_ELEM(WASM_OP_ATOMIC_PREFIX), /* 0xfe */ \ + DEF_DEBUG_BREAK_HANDLE() \ + }; #ifdef __cplusplus }