From 0a80cc4e94de6b4c11242a8e49e31a8167550460 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Mon, 3 Jun 2024 19:48:11 +0800 Subject: [PATCH] Fix wasm loader check data segment count (#3492) When datacount section exists, loader will check whether the data count read from data segment section is same with the data count read from datacount section, but the value of latter can be 0, loader should not skip the check when the latter is 0. This fixes #3491. And fix handle_name_section return value not checked issue and early return true issue after handle_name_section. And also add the failed case in #3491 to ba-issues. --- core/iwasm/interpreter/wasm_loader.c | 25 +++++++++++++----- core/iwasm/interpreter/wasm_mini_loader.c | 23 ++++++++++++---- .../nop_0LM_592_17171016522810388.wasm | Bin 0 -> 514 bytes .../regression/ba-issues/running_config.json | 18 ++++++++++++- 4 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 tests/regression/ba-issues/issues/issue-3491/nop_0LM_592_17171016522810388.wasm diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index b5185b928..d61907e43 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -4716,8 +4716,12 @@ fail: static bool load_data_segment_section(const uint8 *buf, const uint8 *buf_end, - WASMModule *module, bool clone_data_seg, - char *error_buf, uint32 error_buf_size) + WASMModule *module, +#if WASM_ENABLE_BULK_MEMORY != 0 + bool has_datacount_section, +#endif + bool clone_data_seg, char *error_buf, + uint32 error_buf_size) { const uint8 *p = buf, *p_end = buf_end; uint32 data_seg_count, i, mem_index, data_seg_len; @@ -4733,8 +4737,7 @@ load_data_segment_section(const uint8 *buf, const uint8 *buf_end, read_leb_uint32(p, p_end, data_seg_count); #if WASM_ENABLE_BULK_MEMORY != 0 - if ((module->data_seg_count1 != 0) - && (data_seg_count != module->data_seg_count1)) { + if (has_datacount_section && data_seg_count != module->data_seg_count1) { set_error_buf(error_buf, error_buf_size, "data count and data section have inconsistent lengths"); return false; @@ -5242,10 +5245,11 @@ load_user_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, module->name_section_buf = buf; module->name_section_buf_end = buf_end; p += name_len; - handle_name_section(p, p_end, module, is_load_from_file_buf, error_buf, - error_buf_size); + if (!handle_name_section(p, p_end, module, is_load_from_file_buf, + error_buf, error_buf_size)) { + return false; + } LOG_VERBOSE("Load custom name section success."); - return true; } #endif @@ -5789,6 +5793,9 @@ load_from_sections(WASMModule *module, WASMSection *sections, uint8 malloc_free_io_type = VALUE_TYPE_I32; bool reuse_const_strings = is_load_from_file_buf && !wasm_binary_freeable; bool clone_data_seg = is_load_from_file_buf && wasm_binary_freeable; +#if WASM_ENABLE_BULK_MEMORY != 0 + bool has_datacount_section = false; +#endif /* Find code and function sections if have */ while (section) { @@ -5881,6 +5888,9 @@ load_from_sections(WASMModule *module, WASMSection *sections, break; case SECTION_TYPE_DATA: if (!load_data_segment_section(buf, buf_end, module, +#if WASM_ENABLE_BULK_MEMORY != 0 + has_datacount_section, +#endif clone_data_seg, error_buf, error_buf_size)) return false; @@ -5890,6 +5900,7 @@ load_from_sections(WASMModule *module, WASMSection *sections, if (!load_datacount_section(buf, buf_end, module, error_buf, error_buf_size)) return false; + has_datacount_section = true; break; #endif #if WASM_ENABLE_STRINGREF != 0 diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index 15e559250..96d7b5ed0 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -1740,8 +1740,12 @@ load_table_segment_section(const uint8 *buf, const uint8 *buf_end, static bool load_data_segment_section(const uint8 *buf, const uint8 *buf_end, - WASMModule *module, bool clone_data_seg, - char *error_buf, uint32 error_buf_size) + WASMModule *module, +#if WASM_ENABLE_BULK_MEMORY != 0 + bool has_datacount_section, +#endif + bool clone_data_seg, char *error_buf, + uint32 error_buf_size) { const uint8 *p = buf, *p_end = buf_end; uint32 data_seg_count, i, mem_index, data_seg_len; @@ -1757,7 +1761,7 @@ load_data_segment_section(const uint8 *buf, const uint8 *buf_end, read_leb_uint32(p, p_end, data_seg_count); #if WASM_ENABLE_BULK_MEMORY != 0 - bh_assert(module->data_seg_count1 == 0 + bh_assert(!has_datacount_section || data_seg_count == module->data_seg_count1); #endif @@ -2029,8 +2033,10 @@ load_user_section(const uint8 *buf, const uint8 *buf_end, WASMModule *module, #if WASM_ENABLE_CUSTOM_NAME_SECTION != 0 if (name_len == 4 && memcmp(p, "name", 4) == 0) { p += name_len; - handle_name_section(p, p_end, module, is_load_from_file_buf, error_buf, - error_buf_size); + if (!handle_name_section(p, p_end, module, is_load_from_file_buf, + error_buf, error_buf_size)) { + return false; + } } #endif LOG_VERBOSE("Load custom section success.\n"); @@ -2579,6 +2585,9 @@ load_from_sections(WASMModule *module, WASMSection *sections, uint8 malloc_free_io_type = VALUE_TYPE_I32; bool reuse_const_strings = is_load_from_file_buf && !wasm_binary_freeable; bool clone_data_seg = is_load_from_file_buf && wasm_binary_freeable; +#if WASM_ENABLE_BULK_MEMORY != 0 + bool has_datacount_section = false; +#endif /* Find code and function sections if have */ while (section) { @@ -2660,6 +2669,9 @@ load_from_sections(WASMModule *module, WASMSection *sections, break; case SECTION_TYPE_DATA: if (!load_data_segment_section(buf, buf_end, module, +#if WASM_ENABLE_BULK_MEMORY != 0 + has_datacount_section, +#endif clone_data_seg, error_buf, error_buf_size)) return false; @@ -2669,6 +2681,7 @@ load_from_sections(WASMModule *module, WASMSection *sections, if (!load_datacount_section(buf, buf_end, module, error_buf, error_buf_size)) return false; + has_datacount_section = true; break; #endif default: diff --git a/tests/regression/ba-issues/issues/issue-3491/nop_0LM_592_17171016522810388.wasm b/tests/regression/ba-issues/issues/issue-3491/nop_0LM_592_17171016522810388.wasm new file mode 100644 index 0000000000000000000000000000000000000000..5de7a3ce55eed41ae8f41d832d70aa3be93781aa GIT binary patch literal 514 zcmWN@yRMU9007|sFFifUsTvm(V;oMR4^W^`IvGl750sW(xD|&}3jF2L0xgtVtga5u zCMFIpzJjhkgKyyA=0kXfFZqC#9v}b!KEXBsY~Tf6HlXq0J=TDYW)mi$b%jBIwI0KF zaQIaH`6Ipk_ojXQve~*%GwYNAaGk~XnVUWg8V@c2Jc8in1$++R@C*6p@9($Ietmrf zzP$$QXKe8Os@?vCLHH5Fg<{z2vPL;Y`l% zW-(>SP8aogLSUpxhg7DJ?w+Taowjvr zX^clRt;gHLu@judea6cEfijX+!gQxcs^aGgw@971e{$7wu^p&&N7zmBQ;*`ep5%6D zt}NE19GZm^3c4P|KAU*^*obgO%Z_L{QxswxpCZ1($&uhWca*nABW|qCmyunCxrJnW zXJ{Ei)a5CX{Zc@MAy!=pdCXK=@5n)Yc5PZi_IbHj1jNE}H8!?zx3VXU!kI)&tyqKW zpUVVa*#+TlBU?wK%+%&+lCo1rk7&{AQ-_GB20rfjoOc|VW+5bX&ihEA#Gu<*>4`d@ NlzKW~5HBrr^*^^qps4@= literal 0 HcmV?d00001 diff --git a/tests/regression/ba-issues/running_config.json b/tests/regression/ba-issues/running_config.json index b7e35cad0..e50b108bd 100644 --- a/tests/regression/ba-issues/running_config.json +++ b/tests/regression/ba-issues/running_config.json @@ -912,7 +912,7 @@ "options": "", "argument": "", "expected return": { - "ret code": 57, + "ret code": 8, "stdout content": "", "description": "sock_shutdown on a non-socket file descriptor should fail with 57 notsock" } @@ -1706,6 +1706,22 @@ "stdout content": "WASM module load failed: unknown type", "description": "no '0x0:i64'" } + }, + { + "deprecated": false, + "ids": [ + 3491 + ], + "runtime": "iwasm-default-wasi-disabled", + "file": "nop_0LM_592_17171016522810388.wasm", + "mode": "fast-interp", + "options": "", + "argument": "", + "expected return": { + "ret code": 255, + "stdout content": "WASM module load failed: data count and data section have inconsistent lengths", + "description": "Check data segment count" + } } ] }