From d3e89895be21e976231efed50337ac2518f74041 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Fri, 14 Jun 2024 16:22:08 +0800 Subject: [PATCH] wasm loader: Fix pop invalid offset count when stack top is ANY (#3516) In wasm_loader_pop_frame_offset, when the stack is in polymorphic state and the stack top operand is VALUE_TYPE_ANY, if we popping I64/F64 operand, we should pop one offset but not two offsets. The issue was reported in #3513 and #3514. --- core/iwasm/interpreter/wasm_loader.c | 49 ++++++++++-------- core/iwasm/interpreter/wasm_mini_loader.c | 46 ++++++++-------- .../issues/issue-3513/iwasm_poc_04.wasm | Bin 0 -> 1523 bytes .../issues/issue-3514/iwasm_poc_05.wasm | Bin 0 -> 1968 bytes .../regression/ba-issues/running_config.json | 32 ++++++++++++ 5 files changed, 83 insertions(+), 44 deletions(-) create mode 100644 tests/regression/ba-issues/issues/issue-3513/iwasm_poc_04.wasm create mode 100644 tests/regression/ba-issues/issues/issue-3514/iwasm_poc_05.wasm diff --git a/core/iwasm/interpreter/wasm_loader.c b/core/iwasm/interpreter/wasm_loader.c index 129ca5221..5bac2dfa0 100644 --- a/core/iwasm/interpreter/wasm_loader.c +++ b/core/iwasm/interpreter/wasm_loader.c @@ -9370,42 +9370,41 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type, char *error_buf, uint32 error_buf_size) { /* if ctx->frame_csp equals ctx->frame_csp_bottom, - then current block is the function block */ + then current block is the function block */ uint32 depth = ctx->frame_csp > ctx->frame_csp_bottom ? 1 : 0; BranchBlock *cur_block = ctx->frame_csp - depth; int32 available_stack_cell = (int32)(ctx->stack_cell_num - cur_block->stack_cell_num); + uint32 cell_num_to_pop; /* Directly return success if current block is in stack - * polymorphic state while stack is empty. */ + polymorphic state while stack is empty. */ if (available_stack_cell <= 0 && cur_block->is_stack_polymorphic) return true; if (type == VALUE_TYPE_VOID) return true; - if (is_32bit_type(type)) { - /* Check the offset stack bottom to ensure the frame offset - stack will not go underflow. But we don't thrown error - and return true here, because the error msg should be - given in wasm_loader_pop_frame_ref */ - if (!check_offset_pop(ctx, 1)) - return true; + /* Change type to ANY when the stack top is ANY, so as to avoid + popping unneeded offsets, e.g. if type is I64/F64, we may pop + two offsets */ + if (available_stack_cell > 0 && *(ctx->frame_ref - 1) == VALUE_TYPE_ANY) + type = VALUE_TYPE_ANY; - ctx->frame_offset -= 1; - if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) - && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) - ctx->dynamic_offset -= 1; - } - else { - if (!check_offset_pop(ctx, 2)) - return true; + cell_num_to_pop = wasm_value_type_cell_num(type); + + /* Check the offset stack bottom to ensure the frame offset + stack will not go underflow. But we don't thrown error + and return true here, because the error msg should be + given in wasm_loader_pop_frame_ref */ + if (!check_offset_pop(ctx, cell_num_to_pop)) + return true; + + ctx->frame_offset -= cell_num_to_pop; + if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) + && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) + ctx->dynamic_offset -= cell_num_to_pop; - ctx->frame_offset -= 2; - if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) - && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) - ctx->dynamic_offset -= 2; - } emit_operand(ctx, *(ctx->frame_offset)); (void)error_buf; @@ -10893,6 +10892,12 @@ wasm_loader_get_custom_section(WASMModule *module, const char *name, } #endif +#if 0 +#define HANDLE_OPCODE(opcode) #opcode +DEFINE_GOTO_TABLE(const char *, op_mnemonics); +#undef HANDLE_OPCODE +#endif + static bool wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func, uint32 cur_func_idx, char *error_buf, diff --git a/core/iwasm/interpreter/wasm_mini_loader.c b/core/iwasm/interpreter/wasm_mini_loader.c index 41d137c25..dba20b300 100644 --- a/core/iwasm/interpreter/wasm_mini_loader.c +++ b/core/iwasm/interpreter/wasm_mini_loader.c @@ -4924,43 +4924,45 @@ wasm_loader_pop_frame_offset(WASMLoaderContext *ctx, uint8 type, char *error_buf, uint32 error_buf_size) { /* if ctx->frame_csp equals ctx->frame_csp_bottom, - then current block is the function block */ + then current block is the function block */ uint32 depth = ctx->frame_csp > ctx->frame_csp_bottom ? 1 : 0; BranchBlock *cur_block = ctx->frame_csp - depth; int32 available_stack_cell = (int32)(ctx->stack_cell_num - cur_block->stack_cell_num); + uint32 cell_num_to_pop; /* Directly return success if current block is in stack - * polymorphic state while stack is empty. */ + polymorphic state while stack is empty. */ if (available_stack_cell <= 0 && cur_block->is_stack_polymorphic) return true; if (type == VALUE_TYPE_VOID) return true; - if (is_32bit_type(type)) { - /* Check the offset stack bottom to ensure the frame offset - stack will not go underflow. But we don't thrown error - and return true here, because the error msg should be - given in wasm_loader_pop_frame_ref */ - if (!check_offset_pop(ctx, 1)) - return true; + /* Change type to ANY when the stack top is ANY, so as to avoid + popping unneeded offsets, e.g. if type is I64/F64, we may pop + two offsets */ + if (available_stack_cell > 0 && *(ctx->frame_ref - 1) == VALUE_TYPE_ANY) + type = VALUE_TYPE_ANY; - ctx->frame_offset -= 1; - if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) - && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) - ctx->dynamic_offset -= 1; - } - else { - if (!check_offset_pop(ctx, 2)) - return true; + cell_num_to_pop = wasm_value_type_cell_num(type); + + /* Check the offset stack bottom to ensure the frame offset + stack will not go underflow. But we don't thrown error + and return true here, because the error msg should be + given in wasm_loader_pop_frame_ref */ + if (!check_offset_pop(ctx, cell_num_to_pop)) + return true; + + ctx->frame_offset -= cell_num_to_pop; + if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) + && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) + ctx->dynamic_offset -= cell_num_to_pop; - ctx->frame_offset -= 2; - if ((*(ctx->frame_offset) > ctx->start_dynamic_offset) - && (*(ctx->frame_offset) < ctx->max_dynamic_offset)) - ctx->dynamic_offset -= 2; - } emit_operand(ctx, *(ctx->frame_offset)); + + (void)error_buf; + (void)error_buf_size; return true; } diff --git a/tests/regression/ba-issues/issues/issue-3513/iwasm_poc_04.wasm b/tests/regression/ba-issues/issues/issue-3513/iwasm_poc_04.wasm new file mode 100644 index 0000000000000000000000000000000000000000..0e58443bb37e41de44d7588a930a21974b48e0af GIT binary patch literal 1523 zcmZuwT}&KR6h8Of+1Z6VuoLw`wni5Q9~(`4@X`5MqYuUq6QjllY4o8y(A|V+02cj7v~Y}i{THpMNeDc+>PLDz9YV$ zVRWWqUHi5_GqD5}Mk$t~KsG~@j76dG?ZLC}>$WeiEtR7!4=l-&#!K1ewRVXp5G(zw z=GooPwCR0}8Rx%>R zQsAvTSS3JB++;EK-OItg-DPD5C85t^+%Yf(7|X>K$R#8qQH=A)sl-6q63$?f^!$t^ zHXw{pK`YS4D`=EZT25q9ANzId=;}c4)|iRh*JmddGD#0<9~=h_A5b3gAoVQX?x}n} zJk$!Hc9=ogvAIvpzplq@`zhD%8(lJeP75gHe3dbhbQU%B@vc}RLi%`XyuyO?m zb-sE_iyE^Z{itX9#YYCBL~r}~_sq>(vy+O%s&=tv1t!9sdC;%S{&aKd2!CPak!G?UhBxdG}nlXKk?mK#aN|%VHjk5av#)#D)ZwGOgqC@=n9k z83~CH%v_wB+UV86HJBh)z@<)KNgq6#zw*P}$L8E{jl(dLxM8A?j!xrryZ%t;kW~B$ zV>H2%$fe4&dvggNzw@>wE>-Ovk&2zbVOflLP`OCQu5Vv|?8#$+oR4BI;mpN<2FAUw zZRXMojXrPMSBIu=)CX{Y?Rdd;&hN2&Zx9OEi1;i!R(}u?`xYzg4P$}T2xE1H!)RPs zmY=xWOm2$&2t@{`k2}37;TW8Ark^ovVN|=_PF3t?)nx(A;|>5wh$I+sKDsrE$4S=Y z;3bP%vQ)%oU;z<1P$2WUu|B@}Kn2meNB-;`Sot!w4Cl0O+b(5=5*%qKXGaoDbv?== zRXK9LLRAGyiyTKnKnxXu&jIWxPT;H*Dli7RWl;(d8fQuE(6y`k85ya-`78&A-)31H eWK0|Xx@(qYs2U}wu_X1bJ4ELHmanA1z2|Rj9=uQh literal 0 HcmV?d00001 diff --git a/tests/regression/ba-issues/issues/issue-3514/iwasm_poc_05.wasm b/tests/regression/ba-issues/issues/issue-3514/iwasm_poc_05.wasm new file mode 100644 index 0000000000000000000000000000000000000000..f4ac1037fc61f4c7c15c732baac057dda897b082 GIT binary patch literal 1968 zcmY*aYfKbZ6h8Of*8hn z!mh2@21UU5NI|3}jVx6WtjiWbK%hk{>S6=ZP~TFmRz;*SmBH|hR;E%=V2-*oy4I{v7=W9G*tD3%FtK~h!27C#zM;_s+ zE{1SxSGzt2tw6YEPU>|xsD^k*F>&MV5kLUDH*!fuZ4#$P-*qxG0;9~z+kF*}x8ENA zvwgV@T`r3(!D>=fkoGkMAV!Qz8 zF6b6GIhGk|8l3cNUSvchlv+Tq140?6(K00rj|q}USLG}VNGyTi_QpIDq`|&`xctpH z?Pf|utp&_*?%+ad#o&n}<>EJS=-dn=59S_DAh{;8E|Ks&uZ|{Knn-b?4985YG?5wj z$k<`x)+KV0i4(fj6eFwXU<+d^$fA~rob!_}zR&(h_l-s(GA98>2_pk3BA1g4w=Nr- zFqIMu7^%I|n;I;zojN@+T9gHHxwgpGHa#I1DYS?jr4kI{rWwXc@Qr()T5V-J9}Slz z!}*O;MU@H6WZE}dj-~0Os^1hxI?UwmC1V!3yFaN*rnYSr?e#?~I-438b?)1wz2Dyb z%9un1B2Y$y1p*U+QfKa1{@;^_b81D5lv$aTS_RJT>lmy3^54$Wt7x&n9EQwCsl2Qa zQ3Fok!h}H!vl3p8B?5QExk9t}-4i}GOQsnR2$fxm?>j1Le{cnY?gSXjfclH`);mI> zhanZmD1=pmig^Tol?@eZ-oQsgKSNZQHbc}YLzGMY7F7~)Gppkv@?=DHE`ss;WF!Mz z8PN9KY)_8M0#5VtcTRRK{O}i<3gAj&*B(mC;LO=)I~y}$;DlsBWK7@+2(U+l=E58- z5-Y;YqfJ1K9e7?Ki%H+K$=|+LW&&A4TZa_|HgxAH|MOWlrH+sVpuDmotUMiJR_RNJ zrZx5p)%}4$u=?K0UWs%d{9!G{1JnxIItwrl>PImtvD>`fZU`12Av)*1YTp`N>y-pq zTa=RqJJ0j11QiWn1)1v&x(9!Z!j`vpY~S>A)8YEA9i9kEDr8}ytI*oKk_u;(En-j# zAW|AL{DhB(Z7NGaH*c_obiAJuQmjNst-J`xu?J8RzF;niy3fVZP?pThQo|#BYV%Gun1RTSsCMJ^2vj&3c}FQ(^?#){gq zQUS5Fz!Iq0ZS*@o$UU{CaOoT6Ermz6H5y?nvznCnLpp)_4lE@%M_;mFyxs94>c?d! z<)hE#r;yH`Ih2?{&naz)@O2TDswk-<4CC|k!5dU04Ys~h<5N0Y@2-wp9AD3{$%$H# zJ8mYMy)R_K%^Im;5nTJrY@2MkU0Re5BW-gt;SS0SyuFk-C?+p}ZO_P^(-Fml;z6b) z9j=VUSP*l_e{01OIPVKLMnyJ?i0*-S%$bxH4hK7L1HDeSiu8J7LZxMAyDPJYzN%M%>^*^d2jYPFQC2L0Q a;SYh1k`2TIJVMVS;kFxk9!nlIN8mp$9A7^G literal 0 HcmV?d00001 diff --git a/tests/regression/ba-issues/running_config.json b/tests/regression/ba-issues/running_config.json index e50b108bd..2532decfe 100644 --- a/tests/regression/ba-issues/running_config.json +++ b/tests/regression/ba-issues/running_config.json @@ -1722,6 +1722,38 @@ "stdout content": "WASM module load failed: data count and data section have inconsistent lengths", "description": "Check data segment count" } + }, + { + "deprecated": false, + "ids": [ + 3513 + ], + "runtime": "iwasm-default-wasi-disabled", + "file": "iwasm_poc_04.wasm", + "mode": "fast-interp", + "options": "-f main", + "argument": "", + "expected return": { + "ret code": 0, + "stdout content": "", + "description": "no sanitizer 'heap-buffer-overflow'" + } + }, + { + "deprecated": false, + "ids": [ + 3514 + ], + "runtime": "iwasm-default-wasi-disabled", + "file": "iwasm_poc_05.wasm", + "mode": "fast-interp", + "options": "-f main", + "argument": "", + "expected return": { + "ret code": 0, + "stdout content": "", + "description": "no sanitizer 'heap-buffer-overflow'" + } } ] }