Skip to content

Commit

Permalink
Fix loader push_pop_frame_ref_offset (#2590)
Browse files Browse the repository at this point in the history 
`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.
  • Loading branch information
@wenyongh
wenyongh committed Sep 26, 2023
1 parent b622622 commit 6382162
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 50 deletions.
45 changes: 19 additions & 26 deletions core/iwasm/interpreter/wasm_loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
43 changes: 19 additions & 24 deletions core/iwasm/interpreter/wasm_mini_loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 6382162

Please # to comment.