From dd728a68e821f29cb46320d008498823a59b904d Mon Sep 17 00:00:00 2001 From: James Ring Date: Fri, 18 Oct 2024 13:14:02 -0700 Subject: [PATCH] Fix quadratic runtime for duplicate export name detection Previously, the loader would check the name of a new export against all existing exports, leading to a quadratic running time. This change makes the loader parse the entire export section. The exports are then sorted by name, then adjacent exports are checked for uniqueness. --- core/iwasm/aot/aot_loader.c | 61 ++++++++++++++++++++++----- core/iwasm/interpreter/wasm_loader.c | 62 ++++++++++++++++++++++------ 2 files changed, 99 insertions(+), 24 deletions(-) diff --git a/core/iwasm/aot/aot_loader.c b/core/iwasm/aot/aot_loader.c index df06e87069..01e246aa32 100644 --- a/core/iwasm/aot/aot_loader.c +++ b/core/iwasm/aot/aot_loader.c @@ -4,9 +4,8 @@ */ #include "aot_runtime.h" -#include "bh_common.h" -#include "bh_log.h" #include "aot_reloc.h" +#include "bh_platform.h" #include "../common/wasm_runtime_common.h" #include "../common/wasm_native.h" #include "../common/wasm_loader_common.h" @@ -2753,6 +2752,48 @@ destroy_exports(AOTExport *exports) wasm_runtime_free(exports); } +static int +cmp_export_name(const void *a, const void *b) +{ + return strcmp(*(char **)a, *(char **)b); +} + +static bool +check_duplicate_exports(AOTModule *module, char *error_buf, + uint32 error_buf_size) +{ + uint32 i; + bool result = false; + char *names_buf[32], **names = names_buf; + if (module->export_count > 32) { + names = loader_malloc(module->export_count * sizeof(char *), error_buf, + error_buf_size); + if (!names) { + return result; + } + } + + for (i = 0; i < module->export_count; i++) { + names[i] = module->exports[i].name; + } + + qsort(names, module->export_count, sizeof(char *), cmp_export_name); + + for (i = 1; i < module->export_count; i++) { + if (!strcmp(names[i], names[i - 1])) { + set_error_buf(error_buf, error_buf_size, "duplicate export name"); + goto cleanup; + } + } + + result = true; +cleanup: + if (module->export_count > 32) { + wasm_runtime_free(names); + } + return result; +} + static bool load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module, bool is_load_from_file_buf, char *error_buf, uint32 error_buf_size) @@ -2760,7 +2801,7 @@ load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module, const uint8 *buf = *p_buf; AOTExport *exports; uint64 size; - uint32 i, j; + uint32 i; /* Allocate memory */ size = sizeof(AOTExport) * (uint64)module->export_count; @@ -2775,14 +2816,6 @@ load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module, read_uint8(buf, buf_end, exports[i].kind); read_string(buf, buf_end, exports[i].name); - for (j = 0; j < i; j++) { - if (!strcmp(exports[i].name, exports[j].name)) { - set_error_buf(error_buf, error_buf_size, - "duplicate export name"); - return false; - } - } - /* Check export kind and index */ switch (exports[i].kind) { case EXPORT_KIND_FUNC: @@ -2830,6 +2863,12 @@ load_exports(const uint8 **p_buf, const uint8 *buf_end, AOTModule *module, } } + if (module->export_count > 0) { + if (!check_duplicate_exports(module, error_buf, error_buf_size)) { + return false; + } + } + *p_buf = buf; return true; fail: diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index 306f108a01..ae823d7be0 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -4,8 +4,7 @@ */ #include "wasm_loader.h" -#include "bh_common.h" -#include "bh_log.h" +#include "bh_platform.h" #include "wasm.h" #include "wasm_opcode.h" #include "wasm_runtime.h" @@ -3184,6 +3183,12 @@ load_memory(const uint8 **p_buf, const uint8 *buf_end, WASMMemory *memory, return false; } +static int +cmp_export_name(const void *a, const void *b) +{ + return strcmp(*(char **)a, *(char **)b); +} + static bool load_import_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, bool is_load_from_file_buf, bool no_resolve, @@ -4054,17 +4059,53 @@ load_global_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, return false; } +static bool +check_duplicate_exports(WASMModule *module, char *error_buf, + uint32 error_buf_size) +{ + uint32 i; + bool result = false; + char *names_buf[32], **names = names_buf; + + if (module->export_count > 32) { + names = loader_malloc(module->export_count * sizeof(char *), error_buf, + error_buf_size); + if (!names) { + return result; + } + } + + for (i = 0; i < module->export_count; i++) { + names[i] = module->exports[i].name; + } + + qsort(names, module->export_count, sizeof(char *), cmp_export_name); + + for (i = 1; i < module->export_count; i++) { + if (!strcmp(names[i], names[i - 1])) { + set_error_buf(error_buf, error_buf_size, "duplicate export name"); + goto cleanup; + } + } + + result = true; +cleanup: + if (module->export_count > 32) { + wasm_runtime_free(names); + } + return result; +} + static bool load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, bool is_load_from_file_buf, char *error_buf, uint32 error_buf_size) { const uint8 *p = buf, *p_end = buf_end; - uint32 export_count, i, j, index; + uint32 export_count, i, index; uint64 total_size; uint32 str_len; WASMExport *export; - const char *name; read_leb_uint32(p, p_end, export_count); @@ -4090,15 +4131,6 @@ load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, read_leb_uint32(p, p_end, str_len); CHECK_BUF(p, p_end, str_len); - for (j = 0; j < i; j++) { - name = module->exports[j].name; - if (strlen(name) == str_len && memcmp(name, p, str_len) == 0) { - set_error_buf(error_buf, error_buf_size, - "duplicate export name"); - return false; - } - } - if (!(export->name = wasm_const_str_list_insert( p, str_len, module, is_load_from_file_buf, error_buf, error_buf_size))) { @@ -4171,6 +4203,10 @@ load_export_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, return false; } } + + if (!check_duplicate_exports(module, error_buf, error_buf_size)) { + return false; + } } if (p != p_end) {