From d7097fbce8363e18bc7e765ac60135274509be8d Mon Sep 17 00:00:00 2001 From: "liang.he" Date: Wed, 27 Apr 2022 13:28:27 +0800 Subject: [PATCH] Add more checks for wasm-c-api interfaces (#1121) Protect c-api from unlinked runtime objects Fix a potential memory leak Remove unused `imports` in `wasm_instance_t` --- core/iwasm/common/wasm_c_api.c | 136 +++++++++++++++--------- core/iwasm/common/wasm_c_api_internal.h | 1 - core/iwasm/common/wasm_runtime_common.c | 2 +- 3 files changed, 84 insertions(+), 55 deletions(-) diff --git a/core/iwasm/common/wasm_c_api.c b/core/iwasm/common/wasm_c_api.c index 94f45e8c..e5f1af30 100644 --- a/core/iwasm/common/wasm_c_api.c +++ b/core/iwasm/common/wasm_c_api.c @@ -570,14 +570,6 @@ wasm_functype_new(own wasm_valtype_vec_t *params, { wasm_functype_t *type = NULL; - if (!params) { - return NULL; - } - - if (!results) { - return NULL; - } - if (!(type = malloc_internal(sizeof(wasm_functype_t)))) { goto failed; } @@ -588,14 +580,18 @@ wasm_functype_new(own wasm_valtype_vec_t *params, if (!(type->params = malloc_internal(sizeof(wasm_valtype_vec_t)))) { goto failed; } - bh_memcpy_s(type->params, sizeof(wasm_valtype_vec_t), params, - sizeof(wasm_valtype_vec_t)); + if (params) { + bh_memcpy_s(type->params, sizeof(wasm_valtype_vec_t), params, + sizeof(wasm_valtype_vec_t)); + } if (!(type->results = malloc_internal(sizeof(wasm_valtype_vec_t)))) { goto failed; } - bh_memcpy_s(type->results, sizeof(wasm_valtype_vec_t), results, - sizeof(wasm_valtype_vec_t)); + if (results) { + bh_memcpy_s(type->results, sizeof(wasm_valtype_vec_t), results, + sizeof(wasm_valtype_vec_t)); + } return type; @@ -1395,7 +1391,7 @@ wasm_ref_copy(const wasm_ref_t *src) void wasm_ref_delete(own wasm_ref_t *ref) { - if (!ref) + if (!ref || !ref->store) return; DELETE_HOST_INFO(ref); @@ -1648,7 +1644,7 @@ wasm_trap_new(wasm_store_t *store, const wasm_message_t *message) { wasm_trap_t *trap; - if (!store || !message) { + if (!store) { return NULL; } @@ -1656,7 +1652,10 @@ wasm_trap_new(wasm_store_t *store, const wasm_message_t *message) return NULL; } - INIT_VEC(trap->message, wasm_byte_vec_new, message->size, message->data); + if (message) { + INIT_VEC(trap->message, wasm_byte_vec_new, message->size, + message->data); + } return trap; failed: @@ -2339,6 +2338,10 @@ wasm_func_new_basic(wasm_store_t *store, const wasm_functype_t *type, { wasm_func_t *func = NULL; + if (!type) { + goto failed; + } + if (!(func = malloc_internal(sizeof(wasm_func_t)))) { goto failed; } @@ -2363,6 +2366,10 @@ wasm_func_new_with_env_basic(wasm_store_t *store, const wasm_functype_t *type, { wasm_func_t *func = NULL; + if (!type) { + goto failed; + } + if (!(func = malloc_internal(sizeof(wasm_func_t)))) { goto failed; } @@ -2387,6 +2394,9 @@ wasm_func_new(wasm_store_t *store, const wasm_functype_t *type, wasm_func_callback_t callback) { bh_assert(singleton_engine); + if (!callback) { + return NULL; + } return wasm_func_new_basic(store, type, callback); } @@ -2396,6 +2406,9 @@ wasm_func_new_with_env(wasm_store_t *store, const wasm_functype_t *type, void (*finalizer)(void *)) { bh_assert(singleton_engine); + if (!callback) { + return NULL; + } return wasm_func_new_with_env_basic(store, type, callback, env, finalizer); } @@ -2601,7 +2614,7 @@ argv_to_results(const uint32 *argv, const wasm_valtype_vec_t *result_defs, return true; } - if (!results || !results->num_elems || !results->size || !results->data) { + if (!results || !results->size || !results->data) { return false; } @@ -2669,7 +2682,22 @@ wasm_func_call(const wasm_func_t *func, const wasm_val_vec_t *params, WASMExecEnv *exec_env = NULL; size_t param_count, result_count, alloc_count; - bh_assert(func && func->type && func->inst_comm_rt); + if (!func) { + return NULL; + } + + if (!func->inst_comm_rt) { + wasm_name_t message = { 0 }; + wasm_trap_t *trap; + + wasm_name_new_from_string(&message, "failed to call unlinked function"); + trap = wasm_trap_new(func->store, &message); + wasm_byte_vec_delete(&message); + + return trap; + } + + bh_assert(func->type); #if WASM_ENABLE_INTERP != 0 if (func->inst_comm_rt->module_type == Wasm_Module_Bytecode) { @@ -2794,6 +2822,10 @@ wasm_global_new(wasm_store_t *store, const wasm_globaltype_t *global_type, bh_assert(singleton_engine); + if (!global_type || !init) { + goto failed; + } + global = malloc_internal(sizeof(wasm_global_t)); if (!global) { goto failed; @@ -2981,7 +3013,7 @@ aot_global_get(const AOTModuleInstance *inst_aot, uint16 global_idx_rt, void wasm_global_set(wasm_global_t *global, const wasm_val_t *v) { - if (!global || !v) { + if (!global || !v || !global->inst_comm_rt) { return; } @@ -3015,6 +3047,10 @@ wasm_global_get(const wasm_global_t *global, wasm_val_t *out) return; } + if (!global->inst_comm_rt) { + return; + } + memset(out, 0, sizeof(wasm_val_t)); #if WASM_ENABLE_INTERP != 0 @@ -3305,7 +3341,7 @@ wasm_table_get(const wasm_table_t *table, wasm_table_size_t index) { uint32 ref_idx = NULL_REF; - if (!table) { + if (!table || !table->inst_comm_rt) { return NULL; } @@ -3365,7 +3401,7 @@ wasm_table_set(wasm_table_t *table, wasm_table_size_t index, uint32 *p_ref_idx = NULL; uint32 function_count = 0; - if (!table) { + if (!table || !table->inst_comm_rt) { return false; } @@ -3446,7 +3482,7 @@ wasm_table_set(wasm_table_t *table, wasm_table_size_t index, wasm_table_size_t wasm_table_size(const wasm_table_t *table) { - if (!table) { + if (!table || !table->inst_comm_rt) { return 0; } @@ -3502,6 +3538,10 @@ wasm_memory_new_basic(wasm_store_t *store, const wasm_memorytype_t *type) { wasm_memory_t *memory = NULL; + if (!type) { + goto failed; + } + if (!(memory = malloc_internal(sizeof(wasm_memory_t)))) { goto failed; } @@ -3635,8 +3675,13 @@ wasm_memory_type(const wasm_memory_t *memory) byte_t * wasm_memory_data(wasm_memory_t *memory) { - WASMModuleInstanceCommon *module_inst_comm = memory->inst_comm_rt; + WASMModuleInstanceCommon *module_inst_comm; + if (!memory || !memory->inst_comm_rt) { + return NULL; + } + + module_inst_comm = memory->inst_comm_rt; #if WASM_ENABLE_INTERP != 0 if (module_inst_comm->module_type == Wasm_Module_Bytecode) { WASMModuleInstance *module_inst = @@ -3667,8 +3712,13 @@ wasm_memory_data(wasm_memory_t *memory) size_t wasm_memory_data_size(const wasm_memory_t *memory) { - WASMModuleInstanceCommon *module_inst_comm = memory->inst_comm_rt; + WASMModuleInstanceCommon *module_inst_comm; + if (!memory || !memory->inst_comm_rt) { + return 0; + } + + module_inst_comm = memory->inst_comm_rt; #if WASM_ENABLE_INTERP != 0 if (module_inst_comm->module_type == Wasm_Module_Bytecode) { WASMModuleInstance *module_inst = @@ -3699,8 +3749,13 @@ wasm_memory_data_size(const wasm_memory_t *memory) wasm_memory_pages_t wasm_memory_size(const wasm_memory_t *memory) { - WASMModuleInstanceCommon *module_inst_comm = memory->inst_comm_rt; + WASMModuleInstanceCommon *module_inst_comm; + if (!memory || !memory->inst_comm_rt) { + return 0; + } + + module_inst_comm = memory->inst_comm_rt; #if WASM_ENABLE_INTERP != 0 if (module_inst_comm->module_type == Wasm_Module_Bytecode) { WASMModuleInstance *module_inst = @@ -3744,7 +3799,6 @@ interp_link_func(const wasm_instance_t *inst, const WASMModule *module_interp, uint16 func_idx_rt, wasm_func_t *import) { WASMImport *imported_func_interp = NULL; - wasm_func_t *cloned = NULL; bh_assert(inst && module_interp && import); bh_assert(func_idx_rt < module_interp->import_function_count); @@ -3753,15 +3807,6 @@ interp_link_func(const wasm_instance_t *inst, const WASMModule *module_interp, imported_func_interp = module_interp->import_functions + func_idx_rt; bh_assert(imported_func_interp); - if (!(cloned = wasm_func_copy(import))) { - return false; - } - - if (!bh_vector_append((Vector *)inst->imports, &cloned)) { - wasm_func_delete(cloned); - return false; - } - imported_func_interp->u.function.call_conv_wasm_c_api = true; imported_func_interp->u.function.wasm_c_api_with_env = import->with_env; if (import->with_env) { @@ -3965,30 +4010,22 @@ aot_link_func(const wasm_instance_t *inst, const AOTModule *module_aot, uint32 import_func_idx_rt, wasm_func_t *import) { AOTImportFunc *import_aot_func = NULL; - wasm_func_t *cloned = NULL; bh_assert(inst && module_aot && import); import_aot_func = module_aot->import_funcs + import_func_idx_rt; bh_assert(import_aot_func); - if (!(cloned = wasm_func_copy(import))) { - return false; - } - - if (!bh_vector_append((Vector *)inst->imports, &cloned)) { - wasm_func_delete(cloned); - return false; - } - import_aot_func->call_conv_wasm_c_api = true; import_aot_func->wasm_c_api_with_env = import->with_env; if (import->with_env) { import_aot_func->func_ptr_linked = import->u.cb_env.cb; import_aot_func->attachment = import->u.cb_env.env; } - else + else { import_aot_func->func_ptr_linked = import->u.cb; + import_aot_func->attachment = NULL; + } import->func_idx_rt = import_func_idx_rt; return true; @@ -4217,14 +4254,11 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module, } /* link module and imports */ - if (imports) { + if (imports && imports->num_elems) { #if WASM_ENABLE_INTERP != 0 if ((*module)->module_type == Wasm_Module_Bytecode) { import_count = MODULE_INTERP(module)->import_count; - INIT_VEC(instance->imports, wasm_extern_vec_new_uninitialized, - import_count); - if (import_count) { uint32 actual_link_import_count = interp_link(instance, MODULE_INTERP(module), @@ -4246,9 +4280,6 @@ wasm_instance_new_with_args(wasm_store_t *store, const wasm_module_t *module, + MODULE_AOT(module)->import_memory_count + MODULE_AOT(module)->import_table_count; - INIT_VEC(instance->imports, wasm_extern_vec_new_uninitialized, - import_count); - if (import_count) { import_count = aot_link(instance, MODULE_AOT(module), (wasm_extern_t **)imports->data); @@ -4373,7 +4404,6 @@ wasm_instance_delete_internal(wasm_instance_t *instance) return; } - DEINIT_VEC(instance->imports, wasm_extern_vec_delete); DEINIT_VEC(instance->exports, wasm_extern_vec_delete); if (instance->inst_comm_rt) { diff --git a/core/iwasm/common/wasm_c_api_internal.h b/core/iwasm/common/wasm_c_api_internal.h index 38954a68..95bc5fac 100644 --- a/core/iwasm/common/wasm_c_api_internal.h +++ b/core/iwasm/common/wasm_c_api_internal.h @@ -206,7 +206,6 @@ struct wasm_extern_t { struct wasm_instance_t { wasm_store_t *store; - wasm_extern_vec_t *imports; wasm_extern_vec_t *exports; struct wasm_host_info host_info; WASMModuleInstanceCommon *inst_comm_rt; diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c index 4bdae0b3..f239b51f 100644 --- a/core/iwasm/common/wasm_runtime_common.c +++ b/core/iwasm/common/wasm_runtime_common.c @@ -1309,7 +1309,7 @@ wasm_runtime_finalize_call_function(WASMExecEnv *exec_env, bh_assert((argv && ret_argv) || (argc == 0)); - if (argv == ret_argv || argc == 0) { + if (argv == ret_argv) { /* no need to transfrom externref results */ return true; }