From: Dmitry Volyntsev Date: Mon, 15 Jun 2020 15:26:40 +0000 (+0000) Subject: Fixed potential memory corruption in njs_function_frame_invoke(). X-Git-Tag: 0.4.2~28 X-Git-Url: http://www.kaiwu.me/postgresql/commit/static/gitweb.js?a=commitdiff_plain;h=5f6d38546a44889161a29cae58239454e0af0ece;p=njs.git Fixed potential memory corruption in njs_function_frame_invoke(). It is a bug to cast vm->top_frame which is (njs_native_frame_t *) type, to (njs_frame_t *) in general context. The cast is allowed only for the lambda frames. The bug did not manifest itself previously because native frames were allocated with NJS_NATIVE_FRAME_SIZE, which is sizeof(njs_value_t) aligned size of njs_native_frame_t (on 64bit platform are 80 and 72 bytes respectively). njs_frame_t contains njs_native_frame_t as a first field. The frame->retval assignment for native frames resulted in 8 padding bytes were used as a storage space for the retval field. The bug becomes visible when the size of njs_native_frame_t changes. The issue was introduced in 540f03725df2. --- diff --git a/src/njs_function.c b/src/njs_function.c index a50cde43..66a70fad 100644 --- a/src/njs_function.c +++ b/src/njs_function.c @@ -677,13 +677,11 @@ njs_function_native_call(njs_vm_t *vm) { njs_int_t ret; njs_value_t *value; - njs_frame_t *frame; njs_function_t *function, *target; njs_native_frame_t *native, *previous; njs_function_native_t call; native = vm->top_frame; - frame = (njs_frame_t *) native; function = native->function; if (njs_fast_path(function->bound == NULL)) { @@ -711,10 +709,10 @@ njs_function_native_call(njs_vm_t *vm) previous = njs_function_previous_frame(native); - njs_vm_scopes_restore(vm, frame, previous); + njs_vm_scopes_restore(vm, native, previous); if (!native->skip) { - value = njs_vmcode_operand(vm, frame->retval); + value = njs_vmcode_operand(vm, native->retval); /* * GC: value external/internal++ depending * on vm->retval and retval type @@ -1035,10 +1033,10 @@ static njs_int_t njs_function_prototype_call(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t unused) { - njs_int_t ret; - njs_frame_t *frame; - njs_function_t *function; - const njs_value_t *this; + njs_int_t ret; + njs_function_t *function; + const njs_value_t *this; + njs_native_frame_t *frame; if (!njs_is_function(&args[0])) { njs_type_error(vm, "\"this\" argument is not a function"); @@ -1054,12 +1052,12 @@ njs_function_prototype_call(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, nargs = 0; } - frame = (njs_frame_t *) vm->top_frame; - - function = njs_function(&args[0]); + frame = vm->top_frame; /* Skip the "call" method frame. */ - vm->top_frame->skip = 1; + frame->skip = 1; + + function = njs_function(&args[0]); ret = njs_function_frame(vm, function, this, &args[2], nargs, 0); if (njs_slow_path(ret != NJS_OK)) { @@ -1144,7 +1142,7 @@ activate: return ret; } - ret = njs_function_frame_invoke(vm, frame->retval); + ret = njs_function_frame_invoke(vm, frame->native.retval); if (njs_slow_path(ret != NJS_OK)) { return ret; } diff --git a/src/njs_function.h b/src/njs_function.h index 7724277f..159f47ce 100644 --- a/src/njs_function.h +++ b/src/njs_function.h @@ -39,10 +39,6 @@ struct njs_function_lambda_s { njs_align_size(sizeof(njs_frame_t) + closures * sizeof(njs_closure_t *), \ sizeof(njs_value_t)) -/* The retval field is not used in the global frame. */ -#define NJS_GLOBAL_FRAME_SIZE \ - njs_align_size(offsetof(njs_frame_t, retval), sizeof(njs_value_t)) - #define NJS_FRAME_SPARE_SIZE 512 @@ -68,6 +64,7 @@ struct njs_native_frame_s { njs_object_t *arguments_object; njs_exception_t exception; + njs_index_t retval; uint32_t size; uint32_t free_size; @@ -84,9 +81,6 @@ struct njs_native_frame_s { struct njs_frame_s { njs_native_frame_t native; - njs_index_t retval; - - u_char *return_address; njs_frame_t *previous_active_frame; njs_value_t *local; @@ -170,13 +164,12 @@ njs_function_previous_frame(njs_native_frame_t *frame) njs_inline njs_int_t njs_function_frame_invoke(njs_vm_t *vm, njs_index_t retval) { - njs_frame_t *frame; - - frame = (njs_frame_t *) vm->top_frame; + njs_native_frame_t *frame; + frame = vm->top_frame; frame->retval = retval; - if (frame->native.function->native) { + if (frame->function->native) { return njs_function_native_call(vm); } else { diff --git a/src/njs_promise.c b/src/njs_promise.c index cf0a879e..c1db7bcf 100644 --- a/src/njs_promise.c +++ b/src/njs_promise.c @@ -559,16 +559,16 @@ njs_promise_resolve_function(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t unused) { njs_int_t ret; - njs_frame_t *active_frame; njs_value_t *resolution, error, then, arguments[3]; njs_promise_t *promise; njs_function_t *function; + njs_native_frame_t *active_frame; njs_promise_context_t *context; static const njs_value_t string_then = njs_string("then"); - active_frame = (njs_frame_t *) vm->top_frame; - context = active_frame->native.function->context; + active_frame = vm->top_frame; + context = active_frame->function->context; promise = njs_promise(&context->promise); if (*context->resolved_ref) { @@ -715,12 +715,12 @@ static njs_int_t njs_promise_reject_function(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t unused) { - njs_frame_t *active_frame; njs_value_t *value; + njs_native_frame_t *active_frame; njs_promise_context_t *context; - active_frame = (njs_frame_t *) vm->top_frame; - context = active_frame->native.function->context; + active_frame = vm->top_frame; + context = active_frame->function->context; if (*context->resolved_ref) { njs_vm_retval_set(vm, &njs_value_undefined); @@ -995,12 +995,12 @@ njs_promise_then_finally_function(njs_vm_t *vm, njs_value_t *args, { njs_int_t ret; njs_value_t value, retval; - njs_frame_t *frame; njs_promise_t *promise; + njs_native_frame_t *frame; njs_promise_context_t *context; - frame = (njs_frame_t *) vm->top_frame; - context = frame->native.function->context; + frame = vm->top_frame; + context = frame->function->context; ret = njs_function_call(vm, njs_function(&context->finally), &njs_value_undefined, args, 0, &retval); diff --git a/src/njs_vm.c b/src/njs_vm.c index 15acd5d7..2dffb7d9 100644 --- a/src/njs_vm.c +++ b/src/njs_vm.c @@ -286,7 +286,7 @@ njs_vm_init(njs_vm_t *vm) scope_size = vm->scope_size + NJS_INDEX_GLOBAL_OFFSET; - size = NJS_GLOBAL_FRAME_SIZE + scope_size + NJS_FRAME_SPARE_SIZE; + size = njs_frame_size(0) + scope_size + NJS_FRAME_SPARE_SIZE; size = njs_align_size(size, NJS_FRAME_SPARE_SIZE); frame = njs_mp_align(vm->mem_pool, sizeof(njs_value_t), size); @@ -294,15 +294,15 @@ njs_vm_init(njs_vm_t *vm) return NJS_ERROR; } - njs_memzero(frame, NJS_GLOBAL_FRAME_SIZE); + njs_memzero(frame, njs_frame_size(0)); vm->top_frame = &frame->native; vm->active_frame = frame; frame->native.size = size; - frame->native.free_size = size - (NJS_GLOBAL_FRAME_SIZE + scope_size); + frame->native.free_size = size - (njs_frame_size(0) + scope_size); - values = (u_char *) frame + NJS_GLOBAL_FRAME_SIZE; + values = (u_char *) frame + njs_frame_size(0); frame->native.free = values + scope_size; @@ -357,11 +357,12 @@ njs_vm_invoke(njs_vm_t *vm, njs_function_t *function, const njs_value_t *args, void -njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame, +njs_vm_scopes_restore(njs_vm_t *vm, njs_native_frame_t *native, njs_native_frame_t *previous) { njs_uint_t n, nesting; njs_value_t *args; + njs_frame_t *frame; njs_closure_t **closures; njs_function_t *function; @@ -376,7 +377,7 @@ njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame, vm->scopes[NJS_SCOPE_CALLEE_ARGUMENTS] = args; - function = frame->native.function; + function = native->function; if (function->native) { return; @@ -386,6 +387,7 @@ njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame, /* GC: release function closures. */ } + frame = (njs_frame_t *) native; frame = frame->previous_active_frame; vm->active_frame = frame; diff --git a/src/njs_vm.h b/src/njs_vm.h index 48c8fea8..fabe159b 100644 --- a/src/njs_vm.h +++ b/src/njs_vm.h @@ -286,7 +286,7 @@ struct njs_vm_shared_s { }; -void njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame, +void njs_vm_scopes_restore(njs_vm_t *vm, njs_native_frame_t *frame, njs_native_frame_t *previous); njs_int_t njs_vm_add_backtrace_entry(njs_vm_t *vm, njs_arr_t *stack, njs_native_frame_t *native_frame); diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c index 6b74c4aa..67078543 100644 --- a/src/njs_vmcode.c +++ b/src/njs_vmcode.c @@ -705,13 +705,13 @@ next: previous = njs_function_previous_frame(&frame->native); - njs_vm_scopes_restore(vm, frame, previous); + njs_vm_scopes_restore(vm, &frame->native, previous); /* * If a retval is in a callee arguments scope it * must be in the previous callee arguments scope. */ - retval = njs_vmcode_operand(vm, frame->retval); + retval = njs_vmcode_operand(vm, frame->native.retval); /* * GC: value external/internal++ depending on @@ -923,7 +923,7 @@ error: lambda_call = (frame == vm->active_frame); - njs_vm_scopes_restore(vm, frame, previous); + njs_vm_scopes_restore(vm, &frame->native, previous); if (frame->native.size != 0) { vm->stack_size -= frame->native.size; @@ -1687,13 +1687,13 @@ njs_vmcode_return(njs_vm_t *vm, njs_value_t *invld, njs_value_t *retval) previous = njs_function_previous_frame(&frame->native); - njs_vm_scopes_restore(vm, frame, previous); + njs_vm_scopes_restore(vm, &frame->native, previous); /* * If a retval is in a callee arguments scope it * must be in the previous callee arguments scope. */ - retval = njs_vmcode_operand(vm, frame->retval); + retval = njs_vmcode_operand(vm, frame->native.retval); /* GC: value external/internal++ depending on value and retval type */ *retval = *value;