Просмотр исходного кода

Fix potential recursive lock in pthread_create_wrapper (#2980)

Potential recursive lock occurs in:
```
pthread_create_wrapper   (acquire exec_env->wait_lock)
  => wasm_cluster_create_thread
    => allocate_aux_stack
      => wasm_runtime_module_malloc_internal
        => wasm_call_function
          => wasm_exec_env_set_thread_info (acquire exec_env->wait_lock again)
```
Allocate aux stack before calling wasm_cluster_create_thread to resolve it.

Reported in https://github.com/bytecodealliance/wasm-micro-runtime/pull/2977.
Wenyong Huang 2 лет назад
Родитель
Сommit
c39214e8a5

+ 18 - 3
core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c

@@ -558,6 +558,7 @@ pthread_create_wrapper(wasm_exec_env_t exec_env,
     ThreadRoutineArgs *routine_args = NULL;
     uint32 thread_handle;
     uint32 stack_size = 8192;
+    uint32 aux_stack_start = 0, aux_stack_size;
     int32 ret = -1;
 
     bh_assert(module);
@@ -609,10 +610,22 @@ pthread_create_wrapper(wasm_exec_env_t exec_env,
     routine_args->info_node = info_node;
     routine_args->module_inst = new_module_inst;
 
+    /* Allocate aux stack previously since exec_env->wait_lock is acquired
+       below, and if the stack is allocated in wasm_cluster_create_thread,
+       runtime may call the exported malloc function to allocate the stack,
+       which acquires exec_env->wait again in wasm_exec_env_set_thread_info,
+       and recursive lock (or hang) occurs */
+    if (!wasm_cluster_allocate_aux_stack(exec_env, &aux_stack_start,
+                                         &aux_stack_size)) {
+        LOG_ERROR("thread manager error: "
+                  "failed to allocate aux stack space for new thread");
+        goto fail;
+    }
+
     os_mutex_lock(&exec_env->wait_lock);
-    ret =
-        wasm_cluster_create_thread(exec_env, new_module_inst, true,
-                                   pthread_start_routine, (void *)routine_args);
+    ret = wasm_cluster_create_thread(
+        exec_env, new_module_inst, true, aux_stack_start, aux_stack_size,
+        pthread_start_routine, (void *)routine_args);
     if (ret != 0) {
         os_mutex_unlock(&exec_env->wait_lock);
         goto fail;
@@ -636,6 +649,8 @@ fail:
         wasm_runtime_free(info_node);
     if (routine_args)
         wasm_runtime_free(routine_args);
+    if (aux_stack_start)
+        wasm_cluster_free_aux_stack(exec_env, aux_stack_start);
     return ret;
 }
 

+ 1 - 1
core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c

@@ -119,7 +119,7 @@ thread_spawn_wrapper(wasm_exec_env_t exec_env, uint32 start_arg)
     thread_start_arg->arg = start_arg;
     thread_start_arg->start_func = start_func;
 
-    ret = wasm_cluster_create_thread(exec_env, new_module_inst, false,
+    ret = wasm_cluster_create_thread(exec_env, new_module_inst, false, 0, 0,
                                      thread_start, thread_start_arg);
     if (ret != 0) {
         LOG_ERROR("Failed to spawn a new thread");

+ 35 - 17
core/iwasm/libraries/thread-mgr/thread_manager.c

@@ -208,6 +208,33 @@ free_aux_stack(WASMExecEnv *exec_env, uint32 start)
 #endif
 }
 
+bool
+wasm_cluster_allocate_aux_stack(WASMExecEnv *exec_env, uint32 *p_start,
+                                uint32 *p_size)
+{
+    WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env);
+    bool ret;
+
+    os_mutex_lock(&cluster->lock);
+    ret = allocate_aux_stack(exec_env, p_start, p_size);
+    os_mutex_unlock(&cluster->lock);
+
+    return ret;
+}
+
+bool
+wasm_cluster_free_aux_stack(WASMExecEnv *exec_env, uint32 start)
+{
+    WASMCluster *cluster = wasm_exec_env_get_cluster(exec_env);
+    bool ret;
+
+    os_mutex_lock(&cluster->lock);
+    ret = free_aux_stack(exec_env, start);
+    os_mutex_unlock(&cluster->lock);
+
+    return ret;
+}
+
 WASMCluster *
 wasm_cluster_create(WASMExecEnv *exec_env)
 {
@@ -654,12 +681,13 @@ thread_manager_start_routine(void *arg)
 
 int32
 wasm_cluster_create_thread(WASMExecEnv *exec_env,
-                           wasm_module_inst_t module_inst, bool alloc_aux_stack,
+                           wasm_module_inst_t module_inst,
+                           bool is_aux_stack_allocated, uint32 aux_stack_start,
+                           uint32 aux_stack_size,
                            void *(*thread_routine)(void *), void *arg)
 {
     WASMCluster *cluster;
     WASMExecEnv *new_exec_env;
-    uint32 aux_stack_start = 0, aux_stack_size;
     korp_tid tid;
 
     cluster = wasm_exec_env_get_cluster(exec_env);
@@ -676,17 +704,11 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
     if (!new_exec_env)
         goto fail1;
 
-    if (alloc_aux_stack) {
-        if (!allocate_aux_stack(exec_env, &aux_stack_start, &aux_stack_size)) {
-            LOG_ERROR("thread manager error: "
-                      "failed to allocate aux stack space for new thread");
-            goto fail2;
-        }
-
+    if (is_aux_stack_allocated) {
         /* Set aux stack for current thread */
         if (!wasm_exec_env_set_aux_stack(new_exec_env, aux_stack_start,
                                          aux_stack_size)) {
-            goto fail3;
+            goto fail2;
         }
     }
     else {
@@ -699,7 +721,7 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
     new_exec_env->suspend_flags.flags = exec_env->suspend_flags.flags;
 
     if (!wasm_cluster_add_exec_env(cluster, new_exec_env))
-        goto fail3;
+        goto fail2;
 
     new_exec_env->thread_start_routine = thread_routine;
     new_exec_env->thread_arg = arg;
@@ -711,7 +733,7 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
                             (void *)new_exec_env,
                             APP_THREAD_STACK_SIZE_DEFAULT)) {
         os_mutex_unlock(&new_exec_env->wait_lock);
-        goto fail4;
+        goto fail3;
     }
 
     /* Wait until the new_exec_env->handle is set to avoid it is
@@ -723,12 +745,8 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
 
     return 0;
 
-fail4:
-    wasm_cluster_del_exec_env_internal(cluster, new_exec_env, false);
 fail3:
-    /* free the allocated aux stack space */
-    if (alloc_aux_stack)
-        free_aux_stack(exec_env, aux_stack_start);
+    wasm_cluster_del_exec_env_internal(cluster, new_exec_env, false);
 fail2:
     wasm_exec_env_destroy_internal(new_exec_env);
 fail1:

+ 10 - 1
core/iwasm/libraries/thread-mgr/thread_manager.h

@@ -81,7 +81,9 @@ wasm_cluster_dup_c_api_imports(WASMModuleInstanceCommon *module_inst_dst,
 
 int32
 wasm_cluster_create_thread(WASMExecEnv *exec_env,
-                           wasm_module_inst_t module_inst, bool alloc_aux_stack,
+                           wasm_module_inst_t module_inst,
+                           bool is_aux_stack_allocated, uint32 aux_stack_start,
+                           uint32 aux_stack_size,
                            void *(*thread_routine)(void *), void *arg);
 
 int32
@@ -221,6 +223,13 @@ wasm_cluster_traverse_lock(WASMExecEnv *exec_env);
 void
 wasm_cluster_traverse_unlock(WASMExecEnv *exec_env);
 
+bool
+wasm_cluster_allocate_aux_stack(WASMExecEnv *exec_env, uint32 *p_start,
+                                uint32 *p_size);
+
+bool
+wasm_cluster_free_aux_stack(WASMExecEnv *exec_env, uint32 start);
+
 #ifdef __cplusplus
 }
 #endif