Skip to content

Commit 0bef627

Browse files
authored
Fix data races in atomic wait/notify and interp goto table (bytecodealliance#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.
1 parent b5189db commit 0bef627

File tree

2 files changed

+37
-19
lines changed

2 files changed

+37
-19
lines changed

core/iwasm/common/wasm_shared_memory.c

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,13 @@ int32
157157
shared_memory_inc_reference(WASMModuleCommon *module)
158158
{
159159
WASMSharedMemNode *node = search_module(module);
160+
uint32 ref_count = -1;
160161
if (node) {
161162
os_mutex_lock(&node->lock);
162-
node->ref_count++;
163+
ref_count = ++node->ref_count;
163164
os_mutex_unlock(&node->lock);
164-
return node->ref_count;
165165
}
166-
return -1;
166+
return ref_count;
167167
}
168168

169169
int32
@@ -385,7 +385,8 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
385385
WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
386386
AtomicWaitInfo *wait_info;
387387
AtomicWaitNode *wait_node;
388-
bool check_ret, is_timeout;
388+
WASMSharedMemNode *node;
389+
bool check_ret, is_timeout, no_wait;
389390

390391
bh_assert(module->module_type == Wasm_Module_Bytecode
391392
|| module->module_type == Wasm_Module_AoT);
@@ -417,8 +418,13 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
417418

418419
os_mutex_lock(&wait_info->wait_list_lock);
419420

420-
if ((!wait64 && *(uint32 *)address != (uint32)expect)
421-
|| (wait64 && *(uint64 *)address != expect)) {
421+
node = search_module((WASMModuleCommon *)module_inst->module);
422+
os_mutex_lock(&node->shared_mem_lock);
423+
no_wait = (!wait64 && *(uint32 *)address != (uint32)expect)
424+
|| (wait64 && *(uint64 *)address != expect);
425+
os_mutex_unlock(&node->shared_mem_lock);
426+
427+
if (no_wait) {
422428
os_mutex_unlock(&wait_info->wait_list_lock);
423429
return 1;
424430
}
@@ -476,7 +482,9 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address,
476482
wasm_runtime_free(wait_node);
477483
os_mutex_unlock(&wait_info->wait_list_lock);
478484

485+
os_mutex_lock(&node->shared_mem_lock);
479486
release_wait_info(wait_map, wait_info, address);
487+
os_mutex_unlock(&node->shared_mem_lock);
480488

481489
(void)check_ret;
482490
return is_timeout ? 2 : 0;
@@ -489,17 +497,29 @@ wasm_runtime_atomic_notify(WASMModuleInstanceCommon *module, void *address,
489497
WASMModuleInstance *module_inst = (WASMModuleInstance *)module;
490498
uint32 notify_result;
491499
AtomicWaitInfo *wait_info;
500+
WASMSharedMemNode *node;
501+
bool out_of_bounds;
492502

493503
bh_assert(module->module_type == Wasm_Module_Bytecode
494504
|| module->module_type == Wasm_Module_AoT);
495505

496-
if ((uint8 *)address < module_inst->memories[0]->memory_data
497-
|| (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end) {
506+
node = search_module((WASMModuleCommon *)module_inst->module);
507+
if (node)
508+
os_mutex_lock(&node->shared_mem_lock);
509+
out_of_bounds =
510+
((uint8 *)address < module_inst->memories[0]->memory_data
511+
|| (uint8 *)address + 4 > module_inst->memories[0]->memory_data_end);
512+
513+
if (out_of_bounds) {
514+
if (node)
515+
os_mutex_unlock(&node->shared_mem_lock);
498516
wasm_runtime_set_exception(module, "out of bounds memory access");
499517
return -1;
500518
}
501519

502520
wait_info = acquire_wait_info(address, false);
521+
if (node)
522+
os_mutex_unlock(&node->shared_mem_lock);
503523

504524
/* Nobody wait on this address */
505525
if (!wait_info)

core/iwasm/interpreter/wasm_opcode.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -675,12 +675,14 @@ typedef enum WASMAtomicEXTOpcode {
675675
} WASMAtomicEXTOpcode;
676676

677677
#if WASM_ENABLE_DEBUG_INTERP != 0
678-
#define DEF_DEBUG_BREAK_HANDLE(_name) \
679-
_name[DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK); /* 0xd7 */
678+
#define DEF_DEBUG_BREAK_HANDLE() \
679+
[DEBUG_OP_BREAK] = HANDLE_OPCODE(DEBUG_OP_BREAK), /* 0xd7 */
680680
#else
681-
#define DEF_DEBUG_BREAK_HANDLE(_name)
681+
#define DEF_DEBUG_BREAK_HANDLE()
682682
#endif
683683

684+
#define SET_GOTO_TABLE_ELEM(opcode) [opcode] = HANDLE_OPCODE(opcode)
685+
684686
/*
685687
* Macro used to generate computed goto tables for the C interpreter.
686688
*/
@@ -903,14 +905,10 @@ typedef enum WASMAtomicEXTOpcode {
903905
HANDLE_OPCODE(EXT_OP_LOOP), /* 0xd4 */ \
904906
HANDLE_OPCODE(EXT_OP_IF), /* 0xd5 */ \
905907
HANDLE_OPCODE(EXT_OP_BR_TABLE_CACHE), /* 0xd6 */ \
906-
}; \
907-
do { \
908-
_name[WASM_OP_MISC_PREFIX] = \
909-
HANDLE_OPCODE(WASM_OP_MISC_PREFIX); /* 0xfc */ \
910-
_name[WASM_OP_ATOMIC_PREFIX] = \
911-
HANDLE_OPCODE(WASM_OP_ATOMIC_PREFIX); /* 0xfe */ \
912-
DEF_DEBUG_BREAK_HANDLE(_name) \
913-
} while (0)
908+
SET_GOTO_TABLE_ELEM(WASM_OP_MISC_PREFIX), /* 0xfc */ \
909+
SET_GOTO_TABLE_ELEM(WASM_OP_ATOMIC_PREFIX), /* 0xfe */ \
910+
DEF_DEBUG_BREAK_HANDLE() \
911+
};
914912

915913
#ifdef __cplusplus
916914
}

0 commit comments

Comments
 (0)