Fix wasi ctx memory free issue when app heap is corrupted (#455)

core/iwasm/common/wasm_runtime_common.c

@@ -1654,26 +1654,20 @@ wasm_runtime_init_wasi(WASMModuleInstanceCommon *module_inst,
                        char *error_buf, uint32 error_buf_size)
 {
     WASIContext *wasi_ctx;
-    size_t *argv_offsets = NULL;
     char *argv_buf = NULL;
-    size_t *env_offsets = NULL;
+    char **argv_list = NULL;
     char *env_buf = NULL;
-    uint64 argv_buf_len = 0, env_buf_len = 0;
+    char **env_list = NULL;
+    uint64 argv_buf_size = 0, env_buf_size = 0, total_size;
     uint32 argv_buf_offset = 0, env_buf_offset = 0;
-    struct fd_table *curfds;
-    struct fd_prestats *prestats;
-    struct argv_environ_values *argv_environ;
+    struct fd_table *curfds = NULL;
+    struct fd_prestats *prestats = NULL;
+    struct argv_environ_values *argv_environ = NULL;
     bool fd_table_inited = false, fd_prestats_inited = false;
     bool argv_environ_inited = false;
-    uint32 offset_argv_offsets = 0, offset_env_offsets = 0;
-    uint32 offset_argv_buf = 0, offset_env_buf = 0;
-    uint32 offset_curfds = 0;
-    uint32 offset_prestats = 0;
-    uint32 offset_argv_environ = 0;
     __wasi_fd_t wasm_fd = 3;
     int32 raw_fd;
     char *path, resolved_path[PATH_MAX];
-    uint64 total_size;
     uint32 i;
 
     if (!(wasi_ctx = runtime_malloc(sizeof(WASIContext), NULL,
@@ -1697,60 +1691,49 @@ wasm_runtime_init_wasi(WASMModuleInstanceCommon *module_inst,
 
     /* process argv[0], trip the path and suffix, only keep the program name */
     for (i = 0; i < argc; i++)
-        argv_buf_len += strlen(argv[i]) + 1;
+        argv_buf_size += strlen(argv[i]) + 1;
 
-    total_size = sizeof(size_t) * (uint64)argc;
+    total_size = sizeof(char *) * (uint64)argc;
     if (total_size >= UINT32_MAX
-        || !(offset_argv_offsets = wasm_runtime_module_malloc
-                                    (module_inst, (uint32)total_size,
-                                     (void**)&argv_offsets))
-        || argv_buf_len >= UINT32_MAX
-        || !(offset_argv_buf = wasm_runtime_module_malloc
-                                    (module_inst, (uint32)argv_buf_len,
-                                     (void**)&argv_buf))) {
+        || !(argv_list = wasm_runtime_malloc((uint32)total_size))
+        || argv_buf_size >= UINT32_MAX
+        || !(argv_buf = wasm_runtime_malloc((uint32)argv_buf_size))) {
         set_error_buf(error_buf, error_buf_size,
                       "Init wasi environment failed: allocate memory failed");
         goto fail;
     }
 
     for (i = 0; i < argc; i++) {
-        argv_offsets[i] = argv_buf_offset;
+        argv_list[i] = argv_buf + argv_buf_offset;
         bh_strcpy_s(argv_buf + argv_buf_offset,
-                    (uint32)argv_buf_len - argv_buf_offset, argv[i]);
+                    (uint32)argv_buf_size - argv_buf_offset, argv[i]);
         argv_buf_offset += (uint32)(strlen(argv[i]) + 1);
     }
 
     for (i = 0; i < env_count; i++)
-        env_buf_len += strlen(env[i]) + 1;
+        env_buf_size += strlen(env[i]) + 1;
 
-    total_size = sizeof(size_t) * (uint64)argc;
+    total_size = sizeof(char *) * (uint64)argc;
     if (total_size >= UINT32_MAX
-        || !(offset_env_offsets = wasm_runtime_module_malloc
-                                    (module_inst, (uint32)total_size,
-                                     (void**)&env_offsets))
-        || env_buf_len >= UINT32_MAX
-        || !(offset_env_buf = wasm_runtime_module_malloc
-                                    (module_inst, (uint32)env_buf_len,
-                                     (void**)&env_buf))) {
+        || !(env_list = wasm_runtime_malloc((uint32)total_size))
+        || env_buf_size >= UINT32_MAX
+        || !(env_buf = wasm_runtime_malloc((uint32)env_buf_size))) {
         set_error_buf(error_buf, error_buf_size,
                       "Init wasi environment failed: allocate memory failed");
         goto fail;
     }
 
     for (i = 0; i < env_count; i++) {
-        env_offsets[i] = env_buf_offset;
+        env_list[i] = env_buf + env_buf_offset;
         bh_strcpy_s(env_buf + env_buf_offset,
-                    (uint32)env_buf_len - env_buf_offset, env[i]);
+                    (uint32)env_buf_size - env_buf_offset, env[i]);
         env_buf_offset += (uint32)(strlen(env[i]) + 1);
     }
 
-    if (!(offset_curfds = wasm_runtime_module_malloc
-                (module_inst, sizeof(struct fd_table), (void**)&curfds))
-        || !(offset_prestats = wasm_runtime_module_malloc
-                (module_inst, sizeof(struct fd_prestats), (void**)&prestats))
-        || !(offset_argv_environ = wasm_runtime_module_malloc
-                (module_inst, sizeof(struct argv_environ_values),
-                 (void**)&argv_environ))) {
+    if (!(curfds = wasm_runtime_malloc(sizeof(struct fd_table)))
+        || !(prestats = wasm_runtime_malloc(sizeof(struct fd_prestats)))
+        || !(argv_environ =
+                wasm_runtime_malloc(sizeof(struct argv_environ_values)))) {
         set_error_buf(error_buf, error_buf_size,
                       "Init wasi environment failed: allocate memory failed");
         goto fail;
@@ -1773,10 +1756,10 @@ wasm_runtime_init_wasi(WASMModuleInstanceCommon *module_inst,
     fd_prestats_inited = true;
 
     if (!argv_environ_init(argv_environ,
-                           argv_offsets, argc,
-                           argv_buf, argv_buf_len,
-                           env_offsets, env_count,
-                           env_buf, env_buf_len)) {
+                           argv_buf, argv_buf_size,
+                           argv_list, argc,
+                           env_buf, env_buf_size,
+                           env_list, env_count)) {
         set_error_buf(error_buf, error_buf_size,
                       "Init wasi environment failed: "
                       "init argument environment failed");
@@ -1817,13 +1800,13 @@ wasm_runtime_init_wasi(WASMModuleInstanceCommon *module_inst,
         fd_prestats_insert(prestats, dir_list[i], wasm_fd);
     }
 
-    wasi_ctx->curfds_offset = offset_curfds;
-    wasi_ctx->prestats_offset = offset_prestats;
-    wasi_ctx->argv_environ_offset = offset_argv_environ;
-    wasi_ctx->argv_buf_offset = offset_argv_buf;
-    wasi_ctx->argv_offsets_offset = offset_argv_offsets;
-    wasi_ctx->env_buf_offset = offset_env_buf;
-    wasi_ctx->env_offsets_offset = offset_env_offsets;
+    wasi_ctx->curfds = curfds;
+    wasi_ctx->prestats = prestats;
+    wasi_ctx->argv_environ = argv_environ;
+    wasi_ctx->argv_buf = argv_buf;
+    wasi_ctx->argv_list = argv_list;
+    wasi_ctx->env_buf = env_buf;
+    wasi_ctx->env_list = env_list;
 
     return true;
 
@@ -1834,20 +1817,20 @@ fail:
         fd_prestats_destroy(prestats);
     if (fd_table_inited)
         fd_table_destroy(curfds);
-    if (offset_curfds != 0)
-        wasm_runtime_module_free(module_inst, offset_curfds);
-    if (offset_prestats != 0)
-        wasm_runtime_module_free(module_inst, offset_prestats);
-    if (offset_argv_environ != 0)
-        wasm_runtime_module_free(module_inst, offset_argv_environ);
-    if (offset_argv_buf)
-        wasm_runtime_module_free(module_inst, offset_argv_buf);
-    if (offset_argv_offsets)
-        wasm_runtime_module_free(module_inst, offset_argv_offsets);
-    if (offset_env_buf)
-        wasm_runtime_module_free(module_inst, offset_env_buf);
-    if (offset_env_offsets)
-        wasm_runtime_module_free(module_inst, offset_env_offsets);
+    if (curfds)
+        wasm_runtime_free(curfds);
+    if (prestats)
+        wasm_runtime_free(prestats);
+    if (argv_environ)
+        wasm_runtime_free(argv_environ);
+    if (argv_buf)
+        wasm_runtime_free(argv_buf);
+    if (argv_list)
+        wasm_runtime_free(argv_list);
+    if (env_buf)
+        wasm_runtime_free(env_buf);
+    if (env_list)
+        wasm_runtime_free(env_list);
     return false;
 }
 
@@ -1921,40 +1904,28 @@ void
 wasm_runtime_destroy_wasi(WASMModuleInstanceCommon *module_inst)
 {
     WASIContext *wasi_ctx = wasm_runtime_get_wasi_ctx(module_inst);
-    struct argv_environ_values *argv_environ;
-    struct fd_table *curfds;
-    struct fd_prestats *prestats;
 
     if (wasi_ctx) {
-        if (wasi_ctx->argv_environ_offset) {
-            argv_environ = (struct argv_environ_values *)
-                wasm_runtime_addr_app_to_native(module_inst,
-                                                wasi_ctx->argv_environ_offset);
-            argv_environ_destroy(argv_environ);
-            wasm_runtime_module_free(module_inst, wasi_ctx->argv_environ_offset);
+        if (wasi_ctx->argv_environ) {
+            argv_environ_destroy(wasi_ctx->argv_environ);
+            wasm_runtime_free(wasi_ctx->argv_environ);
         }
-        if (wasi_ctx->curfds_offset) {
-            curfds = (struct fd_table *)
-                wasm_runtime_addr_app_to_native(module_inst,
-                                                wasi_ctx->curfds_offset);
-            fd_table_destroy(curfds);
-            wasm_runtime_module_free(module_inst, wasi_ctx->curfds_offset);
+        if (wasi_ctx->curfds) {
+            fd_table_destroy(wasi_ctx->curfds);
+            wasm_runtime_free(wasi_ctx->curfds);
         }
-        if (wasi_ctx->prestats_offset) {
-            prestats = (struct fd_prestats *)
-                wasm_runtime_addr_app_to_native(module_inst,
-                                                wasi_ctx->prestats_offset);
-            fd_prestats_destroy(prestats);
-            wasm_runtime_module_free(module_inst, wasi_ctx->prestats_offset);
+        if (wasi_ctx->prestats) {
+            fd_prestats_destroy(wasi_ctx->prestats);
+            wasm_runtime_free(wasi_ctx->prestats);
         }
-        if (wasi_ctx->argv_buf_offset)
-            wasm_runtime_module_free(module_inst, wasi_ctx->argv_buf_offset);
-        if (wasi_ctx->argv_offsets_offset)
-            wasm_runtime_module_free(module_inst, wasi_ctx->argv_offsets_offset);
-        if (wasi_ctx->env_buf_offset)
-            wasm_runtime_module_free(module_inst, wasi_ctx->env_buf_offset);
-        if (wasi_ctx->env_offsets_offset)
-            wasm_runtime_module_free(module_inst, wasi_ctx->env_offsets_offset);
+        if (wasi_ctx->argv_buf)
+            wasm_runtime_free(wasi_ctx->argv_buf);
+        if (wasi_ctx->argv_list)
+            wasm_runtime_free(wasi_ctx->argv_list);
+        if (wasi_ctx->env_buf)
+            wasm_runtime_free(wasi_ctx->env_buf);
+        if (wasi_ctx->env_list)
+            wasm_runtime_free(wasi_ctx->env_list);
         wasm_runtime_free(wasi_ctx);
     }
 }

core/iwasm/common/wasm_runtime_common.h

@@ -74,17 +74,13 @@ typedef struct WASMModuleInstMemConsumption {
 
 #if WASM_ENABLE_LIBC_WASI != 0
 typedef struct WASIContext {
-    /* Use offset but not native address, since these fields are
-       allocated from app's heap, and the heap space may be re-allocated
-       after memory.grow opcode is executed, the original native address
-       cannot be accessed again. */
-    uint32 curfds_offset;
-    uint32 prestats_offset;
-    uint32 argv_environ_offset;
-    uint32 argv_buf_offset;
-    uint32 argv_offsets_offset;
-    uint32 env_buf_offset;
-    uint32 env_offsets_offset;
+    struct fd_table *curfds;
+    struct fd_prestats *prestats;
+    struct argv_environ_values *argv_environ;
+    char *argv_buf;
+    char **argv_list;
+    char *env_buf;
+    char **env_list;
 } WASIContext;
 #endif
 

core/iwasm/libraries/libc-wasi/libc_wasi_wrapper.c

@@ -45,49 +45,43 @@ typedef struct iovec_app {
 } iovec_app_t;
 
 typedef struct WASIContext {
-    uint32 curfds_offset;
-    uint32 prestats_offset;
-    uint32 argv_environ_offset;
-    uint32 argv_buf_offset;
-    uint32 argv_offsets_offset;
-    uint32 env_buf_offset;
-    uint32 env_offsets_offset;
+    struct fd_table *curfds;
+    struct fd_prestats *prestats;
+    struct argv_environ_values *argv_environ;
+    char *argv_buf;
+    char **argv_list;
+    char *env_buf;
+    char **env_list;
 } *wasi_ctx_t;
 
 wasi_ctx_t
 wasm_runtime_get_wasi_ctx(wasm_module_inst_t module_inst);
 
-static struct fd_table *
+static inline struct fd_table *
 wasi_ctx_get_curfds(wasm_module_inst_t module_inst,
                     wasi_ctx_t wasi_ctx)
 {
     if (!wasi_ctx)
         return NULL;
-    return (struct fd_table *)
-        wasm_runtime_addr_app_to_native(module_inst,
-                                        wasi_ctx->curfds_offset);
+    return wasi_ctx->curfds;
 }
 
-static struct argv_environ_values *
+static inline struct argv_environ_values *
 wasi_ctx_get_argv_environ(wasm_module_inst_t module_inst,
                           wasi_ctx_t wasi_ctx)
 {
     if (!wasi_ctx)
         return NULL;
-    return (struct argv_environ_values *)
-        wasm_runtime_addr_app_to_native(module_inst,
-                                        wasi_ctx->argv_environ_offset);
+    return wasi_ctx->argv_environ;
 }
 
-static struct fd_prestats *
+static inline struct fd_prestats *
 wasi_ctx_get_prestats(wasm_module_inst_t module_inst,
                       wasi_ctx_t wasi_ctx)
 {
     if (!wasi_ctx)
         return NULL;
-    return (struct fd_prestats *)
-        wasm_runtime_addr_app_to_native(module_inst,
-                                        wasi_ctx->prestats_offset);
+    return wasi_ctx->prestats;
 }
 
 static wasi_errno_t
@@ -152,9 +146,7 @@ wasi_args_sizes_get(wasm_exec_env_t exec_env,
         || !validate_native_addr(argv_buf_size_app, sizeof(uint32)))
         return (wasi_errno_t)-1;
 
-    argv_environ = (struct argv_environ_values *)
-        wasm_runtime_addr_app_to_native(module_inst,
-                                        wasi_ctx->argv_environ_offset);
+    argv_environ = wasi_ctx->argv_environ;
 
     err = wasmtime_ssp_args_sizes_get(argv_environ,
                                       &argc, &argv_buf_size);

core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c

@@ -2822,7 +2822,7 @@ __wasi_errno_t wasmtime_ssp_args_get(
   char *argv_buf
 ) {
   for (size_t i = 0; i < argv_environ->argc; ++i) {
-    argv[i] = argv_buf + (argv_environ->argv[i] - argv_environ->argv_buf);
+    argv[i] = argv_buf + (argv_environ->argv_list[i] - argv_environ->argv_buf);
   }
   argv[argv_environ->argc] = NULL;
   bh_memcpy_s(argv_buf, (uint32)argv_environ->argv_buf_size,
@@ -2850,7 +2850,7 @@ __wasi_errno_t wasmtime_ssp_environ_get(
   char *environ_buf
 ) {
   for (size_t i = 0; i < argv_environ->environ_count; ++i) {
-    environ[i] = environ_buf + (argv_environ->environ[i] - argv_environ->environ_buf);
+    environ[i] = environ_buf + (argv_environ->environ_list[i] - argv_environ->environ_buf);
   }
   environ[argv_environ->environ_count] = NULL;
   bh_memcpy_s(environ_buf, (uint32)argv_environ->environ_buf_size,
@@ -2871,76 +2871,26 @@ __wasi_errno_t wasmtime_ssp_environ_sizes_get(
 }
 
 bool argv_environ_init(struct argv_environ_values *argv_environ,
-                       const size_t *argv_offsets, size_t argv_offsets_len,
-                       const char *argv_buf, size_t argv_buf_len,
-                       const size_t *environ_offsets, size_t environ_offsets_len,
-                       const char *environ_buf, size_t environ_buf_len)
+                       char *argv_buf, size_t argv_buf_size,
+                       char **argv_list, size_t argc,
+                       char *environ_buf, size_t environ_buf_size,
+                       char **environ_list, size_t environ_count)
 {
-    uint64 total_size;
-    size_t i;
-
     memset(argv_environ, 0, sizeof(struct argv_environ_values));
 
-    argv_environ->argc = argv_offsets_len;
-    argv_environ->argv_buf_size = argv_buf_len;
-
-    total_size = sizeof(char *) * (uint64)argv_offsets_len;
-    if (total_size >= UINT32_MAX
-        || !(argv_environ->argv = wasm_runtime_malloc((uint32)total_size)))
-        return false;
-
-
-    if (argv_buf_len >= UINT32_MAX
-        || !(argv_environ->argv_buf = wasm_runtime_malloc((uint32)argv_buf_len)))
-        goto fail1;
-
-    for (i = 0; i < argv_offsets_len; ++i) {
-        argv_environ->argv[i] = argv_environ->argv_buf + argv_offsets[i];
-    }
-    bh_memcpy_s(argv_environ->argv_buf, (uint32)argv_buf_len,
-                argv_buf, (uint32)argv_buf_len);
-
-    argv_environ->environ_count = environ_offsets_len;
-    argv_environ->environ_buf_size = environ_buf_len;
-
-    total_size = sizeof(char *) * (uint64)environ_offsets_len;
-    if (total_size >= UINT32_MAX
-        || !(argv_environ->environ = wasm_runtime_malloc((uint32)total_size)))
-        goto fail2;
-
-    if (environ_buf_len >= UINT32_MAX
-        || !(argv_environ->environ_buf = wasm_runtime_malloc((uint32)environ_buf_len)))
-        goto fail3;
-
-    for (i = 0; i < environ_offsets_len; ++i) {
-        argv_environ->environ[i] = argv_environ->environ_buf + environ_offsets[i];
-    }
-    bh_memcpy_s(argv_environ->environ_buf, (uint32)environ_buf_len,
-                environ_buf, (uint32)environ_buf_len);
-
+    argv_environ->argv_buf = argv_buf;
+    argv_environ->argv_buf_size = argv_buf_size;
+    argv_environ->argv_list = argv_list;
+    argv_environ->argc = argc;
+    argv_environ->environ_buf = environ_buf;
+    argv_environ->environ_buf_size = environ_buf_size;
+    argv_environ->environ_list = environ_list;
+    argv_environ->environ_count = environ_count;
     return true;
-
-fail3:
-    wasm_runtime_free(argv_environ->environ);
-fail2:
-    wasm_runtime_free(argv_environ->argv_buf);
-fail1:
-    wasm_runtime_free(argv_environ->argv);
-
-    memset(argv_environ, 0, sizeof(struct argv_environ_values));
-    return false;
 }
 
 void argv_environ_destroy(struct argv_environ_values *argv_environ)
 {
-    if (argv_environ->argv_buf)
-        wasm_runtime_free(argv_environ->argv_buf);
-    if (argv_environ->argv)
-        wasm_runtime_free(argv_environ->argv);
-    if (argv_environ->environ_buf)
-        wasm_runtime_free(argv_environ->environ_buf);
-    if (argv_environ->environ)
-        wasm_runtime_free(argv_environ->environ);
 }
 
 void fd_table_destroy(struct fd_table *ft)

core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.h

@@ -34,25 +34,25 @@ struct fd_prestats {
 };
 
 struct argv_environ_values {
-  size_t argc;
+  const char *argv_buf;
   size_t argv_buf_size;
-  char **argv;
-  char *argv_buf;
-  size_t environ_count;
-  size_t environ_buf_size;
-  char **environ;
+  char **argv_list;
+  size_t argc;
   char *environ_buf;
+  size_t environ_buf_size;
+  char **environ_list;
+  size_t environ_count;
 };
 
 bool fd_table_init(struct fd_table *);
 bool fd_table_insert_existing(struct fd_table *, __wasi_fd_t, int);
 bool fd_prestats_init(struct fd_prestats *);
 bool fd_prestats_insert(struct fd_prestats *, const char *, __wasi_fd_t);
-bool argv_environ_init(struct argv_environ_values *,
-                       const size_t *argv_offsets, size_t argv_offsets_len,
-                       const char *argv_buf, size_t argv_buf_len,
-                       const size_t *environ_offsets, size_t environ_offsets_len,
-                       const char *environ_buf, size_t environ_buf_len);
+bool argv_environ_init(struct argv_environ_values *argv_environ,
+                       char *argv_buf, size_t argv_buf_size,
+                       char **argv_list, size_t argc,
+                       char *environ_buf, size_t environ_buf_size,
+                       char **environ_list, size_t environ_count);
 void argv_environ_destroy(struct argv_environ_values *argv_environ);
 void fd_table_destroy(struct fd_table *ft);
 void fd_prestats_destroy(struct fd_prestats *pt);