details: https://hg.nginx.org/njs/rev/cf85ed422623
branches:
changeset: 1430:cf85ed422623
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Mon Jun 15 15:26:40 2020 +0000
description:
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.

diffstat:

src/njs_function.c | 24 +++++++++++-------------
src/njs_function.h | 15 ++++-----------
src/njs_promise.c | 18 +++++++++---------
src/njs_vm.c | 14 ++++++++------
src/njs_vm.h | 2 +-
src/njs_vmcode.c | 10 +++++-----
6 files changed, 38 insertions(+), 45 deletions(-)

diffs (294 lines):

diff -r b33402f10e82 -r cf85ed422623 src/njs_function.c
--- a/src/njs_function.c Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_function.c Mon Jun 15 15:26:40 2020 +0000
@@ -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,13 +1052,13 @@ njs_function_prototype_call(njs_vm_t *vm
nargs = 0;
}

- frame = (njs_frame_t *) vm->top_frame;
+ frame = vm->top_frame;
+
+ /* Skip the "call" method frame. */
+ frame->skip = 1;

function = njs_function(&args[0]);

- /* Skip the "call" method frame. */
- vm->top_frame->skip = 1;
-
ret = njs_function_frame(vm, function, this, &args[2], nargs, 0);
if (njs_slow_path(ret != NJS_OK)) {
return ret;
@@ -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 -r b33402f10e82 -r cf85ed422623 src/njs_function.h
--- a/src/njs_function.h Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_function.h Mon Jun 15 15:26:40 2020 +0000
@@ -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_f
njs_inline njs_int_t
njs_function_frame_invoke(njs_vm_t *vm, njs_index_t retval)
{
- njs_frame_t *frame;
+ njs_native_frame_t *frame;

- frame = (njs_frame_t *) vm->top_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 -r b33402f10e82 -r cf85ed422623 src/njs_promise.c
--- a/src/njs_promise.c Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_promise.c Mon Jun 15 15:26:40 2020 +0000
@@ -559,16 +559,16 @@ njs_promise_resolve_function(njs_vm_t *v
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
{
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 -r b33402f10e82 -r cf85ed422623 src/njs_vm.c
--- a/src/njs_vm.c Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_vm.c Mon Jun 15 15:26:40 2020 +0000
@@ -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


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_

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_
/* GC: release function closures. */
}

+ frame = (njs_frame_t *) native;
frame = frame->previous_active_frame;
vm->active_frame = frame;

diff -r b33402f10e82 -r cf85ed422623 src/njs_vm.h
--- a/src/njs_vm.h Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_vm.h Mon Jun 15 15:26:40 2020 +0000
@@ -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 -r b33402f10e82 -r cf85ed422623 src/njs_vmcode.c
--- a/src/njs_vmcode.c Sun May 31 08:45:41 2020 +0300
+++ b/src/njs_vmcode.c Mon Jun 15 15:26:40 2020 +0000
@@ -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_valu

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;
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel