From 91222e1e44c7d6852f183be09565c1171cbe8df9 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 21 Apr 2022 13:21:37 +0900 Subject: [PATCH] interpreter: Fix an UBSan complaint in word_copy (#1106) Fix an UBSan complaint introduced by recent change by adding more checks to word_copy: ``` wasm_interp_fast.c:792:9: runtime error: applying zero offset to null pointer ``` --- core/iwasm/interpreter/wasm_interp_classic.c | 39 ++++++++++++-------- core/iwasm/interpreter/wasm_interp_fast.c | 13 +++++-- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index ae6e4996..5305c775 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -256,19 +256,21 @@ read_leb(const uint8 *buf, uint32 *p_offset, uint32 maxbits, bool sign) --frame_csp; \ } while (0) -#define POP_CSP_N(n) \ - do { \ - uint32 *frame_sp_old = frame_sp; \ - uint32 cell_num_to_copy; \ - POP_CSP_CHECK_OVERFLOW(n + 1); \ - frame_csp -= n; \ - frame_ip = (frame_csp - 1)->target_addr; \ - /* copy arity values of block */ \ - frame_sp = (frame_csp - 1)->frame_sp; \ - cell_num_to_copy = (frame_csp - 1)->cell_num; \ - word_copy(frame_sp, frame_sp_old - cell_num_to_copy, \ - cell_num_to_copy); \ - frame_sp += cell_num_to_copy; \ +#define POP_CSP_N(n) \ + do { \ + uint32 *frame_sp_old = frame_sp; \ + uint32 cell_num_to_copy; \ + POP_CSP_CHECK_OVERFLOW(n + 1); \ + frame_csp -= n; \ + frame_ip = (frame_csp - 1)->target_addr; \ + /* copy arity values of block */ \ + frame_sp = (frame_csp - 1)->frame_sp; \ + cell_num_to_copy = (frame_csp - 1)->cell_num; \ + if (cell_num_to_copy > 0) { \ + word_copy(frame_sp, frame_sp_old - cell_num_to_copy, \ + cell_num_to_copy); \ + } \ + frame_sp += cell_num_to_copy; \ } while (0) /* Pop the given number of elements from the given frame's stack. */ @@ -717,6 +719,9 @@ sign_ext_32_64(int32 val) static inline void word_copy(uint32 *dest, uint32 *src, unsigned num) { + bh_assert(dest != NULL); + bh_assert(src != NULL); + bh_assert(num > 0); if (dest != src) { /* No overlap buffer */ bh_assert(!((src < dest) && (dest < src + num))); @@ -3581,7 +3586,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, call_func_from_return_call: { POP(cur_func->param_cell_num); - word_copy(frame->lp, frame_sp, cur_func->param_cell_num); + if (cur_func->param_cell_num > 0) { + word_copy(frame->lp, frame_sp, cur_func->param_cell_num); + } FREE_FRAME(exec_env, frame); wasm_exec_env_set_cur_frame(exec_env, (WASMRuntimeFrame *)prev_frame); goto call_func_from_entry; @@ -3593,7 +3600,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, WASMInterpFrame *outs_area = wasm_exec_env_wasm_stack_top(exec_env); POP(cur_func->param_cell_num); SYNC_ALL_TO_FRAME(); - word_copy(outs_area->lp, frame_sp, cur_func->param_cell_num); + if (cur_func->param_cell_num > 0) { + word_copy(outs_area->lp, frame_sp, cur_func->param_cell_num); + } prev_frame = frame; } diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index 25f96dbc..1c817b96 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -787,6 +787,9 @@ sign_ext_32_64(int32 val) static inline void word_copy(uint32 *dest, uint32 *src, unsigned num) { + bh_assert(dest != NULL); + bh_assert(src != NULL); + bh_assert(num > 0); if (dest != src) { /* No overlap buffer */ bh_assert(!((src < dest) && (dest < src + num))); @@ -3575,7 +3578,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, } } frame->lp = frame->operand + cur_func->const_cell_num; - word_copy(frame->lp, lp_base, lp - lp_base); + if (lp - lp_base > 0) { + word_copy(frame->lp, lp_base, lp - lp_base); + } wasm_runtime_free(lp_base); FREE_FRAME(exec_env, frame); frame_ip += cur_func->param_count * sizeof(int16); @@ -3695,8 +3700,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, frame->operand + cur_wasm_func->const_cell_num; /* Initialize the consts */ - word_copy(frame->operand, (uint32 *)cur_wasm_func->consts, - cur_wasm_func->const_cell_num); + if (cur_wasm_func->const_cell_num > 0) { + word_copy(frame->operand, (uint32 *)cur_wasm_func->consts, + cur_wasm_func->const_cell_num); + } /* Initialize the local variables */ memset(frame_lp + cur_func->param_cell_num, 0,