From 6382162711a9cfaedc63de9065b0bc6eda8123f3 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Tue, 26 Sep 2023 10:17:54 +0800 Subject: [PATCH] Fix loader push_pop_frame_ref_offset (#2590) `wasm_loader_push_pop_frame_offset` may pop n operands by using `loader_ctx->stack_cell_num` to check whether the operand can be popped or not. While `loader_ctx->stack_cell_num` is updated in the later `wasm_loader_push_pop_frame_ref`, the check may fail if the stack is in polymorphic state and lead to `ctx->frame_offset` underflow. Fix issue #2577 and #2586. --- core/iwasm/interpreter/wasm_loader.c | 45 ++++++++++------------- core/iwasm/interpreter/wasm_mini_loader.c | 43 ++++++++++------------ 2 files changed, 38 insertions(+), 50 deletions(-) diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index c775ee6c0d..130ad4392d 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -5476,6 +5476,7 @@ wasm_loader_pop_frame_ref(WASMLoaderContext *ctx, uint8 type, char *error_buf, return true; } +#if WASM_ENABLE_FAST_INTERP == 0 static bool wasm_loader_push_pop_frame_ref(WASMLoaderContext *ctx, uint8 pop_cnt, uint8 type_push, uint8 type_pop, char *error_buf, @@ -5490,6 +5491,7 @@ wasm_loader_push_pop_frame_ref(WASMLoaderContext *ctx, uint8 pop_cnt, return false; return true; } +#endif static bool wasm_loader_push_frame_csp(WASMLoaderContext *ctx, uint8 label_type, @@ -6166,27 +6168,6 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type, return true; } -static bool -wasm_loader_push_pop_frame_offset(WASMLoaderContext *ctx, uint8 pop_cnt, - uint8 type_push, uint8 type_pop, - bool disable_emit, int16 operand_offset, - char *error_buf, uint32 error_buf_size) -{ - uint8 i; - - for (i = 0; i < pop_cnt; i++) { - if (!wasm_loader_pop_frame_offset(ctx, type_pop, error_buf, - error_buf_size)) - return false; - } - if (!wasm_loader_push_frame_offset(ctx, type_push, disable_emit, - operand_offset, error_buf, - error_buf_size)) - return false; - - return true; -} - static bool wasm_loader_push_frame_ref_offset(WASMLoaderContext *ctx, uint8 type, bool disable_emit, int16 operand_offset, @@ -6220,12 +6201,24 @@ wasm_loader_push_pop_frame_ref_offset(WASMLoaderContext *ctx, uint8 pop_cnt, bool disable_emit, int16 operand_offset, char *error_buf, uint32 error_buf_size) { - if (!wasm_loader_push_pop_frame_offset(ctx, pop_cnt, type_push, type_pop, - disable_emit, operand_offset, - error_buf, error_buf_size)) + uint8 i; + + for (i = 0; i < pop_cnt; i++) { + if (!wasm_loader_pop_frame_offset(ctx, type_pop, error_buf, + error_buf_size)) + return false; + + if (!wasm_loader_pop_frame_ref(ctx, type_pop, error_buf, + error_buf_size)) + return false; + } + + if (!wasm_loader_push_frame_offset(ctx, type_push, disable_emit, + operand_offset, error_buf, + error_buf_size)) return false; - if (!wasm_loader_push_pop_frame_ref(ctx, pop_cnt, type_push, type_pop, - error_buf, error_buf_size)) + + if (!wasm_loader_push_frame_ref(ctx, type_push, error_buf, error_buf_size)) return false; return true; diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index 9f568e132c..d16bc1a240 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -3937,6 +3937,7 @@ wasm_loader_pop_frame_ref(WASMLoaderContext *ctx, uint8 type, char *error_buf, return true; } +#if WASM_ENABLE_FAST_INTERP == 0 static bool wasm_loader_push_pop_frame_ref(WASMLoaderContext *ctx, uint8 pop_cnt, uint8 type_push, uint8 type_pop, char *error_buf, @@ -3951,6 +3952,7 @@ wasm_loader_push_pop_frame_ref(WASMLoaderContext *ctx, uint8 pop_cnt, return false; return true; } +#endif static bool wasm_loader_push_frame_csp(WASMLoaderContext *ctx, uint8 label_type, @@ -4608,25 +4610,6 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type, return true; } -static bool -wasm_loader_push_pop_frame_offset(WASMLoaderContext *ctx, uint8 pop_cnt, - uint8 type_push, uint8 type_pop, - bool disable_emit, int16 operand_offset, - char *error_buf, uint32 error_buf_size) -{ - for (int i = 0; i < pop_cnt; i++) { - if (!wasm_loader_pop_frame_offset(ctx, type_pop, error_buf, - error_buf_size)) - return false; - } - if (!wasm_loader_push_frame_offset(ctx, type_push, disable_emit, - operand_offset, error_buf, - error_buf_size)) - return false; - - return true; -} - static bool wasm_loader_push_frame_ref_offset(WASMLoaderContext *ctx, uint8 type, bool disable_emit, int16 operand_offset, @@ -4660,12 +4643,24 @@ wasm_loader_push_pop_frame_ref_offset(WASMLoaderContext *ctx, uint8 pop_cnt, bool disable_emit, int16 operand_offset, char *error_buf, uint32 error_buf_size) { - if (!wasm_loader_push_pop_frame_offset(ctx, pop_cnt, type_push, type_pop, - disable_emit, operand_offset, - error_buf, error_buf_size)) + uint8 i; + + for (i = 0; i < pop_cnt; i++) { + if (!wasm_loader_pop_frame_offset(ctx, type_pop, error_buf, + error_buf_size)) + return false; + + if (!wasm_loader_pop_frame_ref(ctx, type_pop, error_buf, + error_buf_size)) + return false; + } + + if (!wasm_loader_push_frame_offset(ctx, type_push, disable_emit, + operand_offset, error_buf, + error_buf_size)) return false; - if (!wasm_loader_push_pop_frame_ref(ctx, pop_cnt, type_push, type_pop, - error_buf, error_buf_size)) + + if (!wasm_loader_push_frame_ref(ctx, type_push, error_buf, error_buf_size)) return false; return true;