details: https://hg.nginx.org/njs/rev/d776b59196c5
branches:
changeset: 1814:d776b59196c5
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Fri Jan 21 14:31:30 2022 +0000
description:
Fixed recursive async function calls.

Previously, PromiseCapability record was stored (function->context)
directly in function object during a function invocation. This is
not correct, because PromiseCapability record should be linked to
current execution context. As a result, function->context is
overwritten with consecutive recursive calls which results in
use-after-free.

This closes #451 issue on Github.

diffstat:

src/njs_async.c | 15 ++-------------
src/njs_function.c | 8 +++++---
src/njs_function.h | 3 ++-
src/njs_value.h | 1 -
src/njs_vm.c | 2 +-
src/njs_vmcode.c | 18 ++++++++----------
src/njs_vmcode.h | 3 ++-
test/js/async_recursive_last.t.js | 26 ++++++++++++++++++++++++++
test/js/async_recursive_mid.t.js | 26 ++++++++++++++++++++++++++
9 files changed, 72 insertions(+), 30 deletions(-)

diffs (264 lines):

diff -r 620418b1a641 -r d776b59196c5 src/njs_async.c
--- a/src/njs_async.c Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_async.c Fri Jan 21 14:31:30 2022 +0000
@@ -29,9 +29,7 @@ njs_async_function_frame_invoke(njs_vm_t
return NJS_ERROR;
}

- frame->function->context = capability;
-
- ret = njs_function_lambda_call(vm);
+ ret = njs_function_lambda_call(vm, capability, NULL);

if (ret == NJS_OK) {
ret = njs_function_call(vm, njs_function(&capability->resolve),
@@ -63,7 +61,6 @@ njs_await_fulfilled(njs_vm_t *vm, njs_va
njs_int_t ret;
njs_value_t **cur_local, **cur_closures, **cur_temp, *value;
njs_frame_t *frame, *async_frame;
- njs_function_t *function;
njs_async_ctx_t *ctx;
njs_native_frame_t *top, *async;

@@ -78,8 +75,6 @@ njs_await_fulfilled(njs_vm_t *vm, njs_va
async = &async_frame->native;
async->previous = vm->top_frame;

- function = async->function;
-
cur_local = vm->levels[NJS_LEVEL_LOCAL];
cur_closures = vm->levels[NJS_LEVEL_CLOSURE];
cur_temp = vm->levels[NJS_LEVEL_TEMP];
@@ -98,13 +93,7 @@ njs_await_fulfilled(njs_vm_t *vm, njs_va

vm->top_frame->retval = &vm->retval;

- function->context = ctx->capability;
- function->await = ctx;
-
- ret = njs_vmcode_interpreter(vm, ctx->pc);
-
- function->context = NULL;
- function->await = NULL;
+ ret = njs_vmcode_interpreter(vm, ctx->pc, ctx->capability, ctx);

vm->levels[NJS_LEVEL_LOCAL] = cur_local;
vm->levels[NJS_LEVEL_CLOSURE] = cur_closures;
diff -r 620418b1a641 -r d776b59196c5 src/njs_function.c
--- a/src/njs_function.c Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_function.c Fri Jan 21 14:31:30 2022 +0000
@@ -608,7 +608,7 @@ njs_function_call2(njs_vm_t *vm, njs_fun


njs_int_t
-njs_function_lambda_call(njs_vm_t *vm)
+njs_function_lambda_call(njs_vm_t *vm, void *promise_cap, void *async_ctx)
{
uint32_t n;
njs_int_t ret;
@@ -622,6 +622,8 @@ njs_function_lambda_call(njs_vm_t *vm)
frame = (njs_frame_t *) vm->top_frame;
function = frame->native.function;

+ njs_assert(function->context == NULL);
+
if (function->global && !function->closure_copied) {
ret = njs_function_capture_global_closures(vm, function);
if (njs_slow_path(ret != NJS_OK)) {
@@ -698,7 +700,7 @@ njs_function_lambda_call(njs_vm_t *vm)
}
}

- ret = njs_vmcode_interpreter(vm, lambda->start);
+ ret = njs_vmcode_interpreter(vm, lambda->start, promise_cap, async_ctx);

/* Restore current level. */
vm->levels[NJS_LEVEL_LOCAL] = cur_local;
@@ -775,7 +777,7 @@ njs_function_frame_invoke(njs_vm_t *vm,
return njs_function_native_call(vm);

} else {
- return njs_function_lambda_call(vm);
+ return njs_function_lambda_call(vm, NULL, NULL);
}
}

diff -r 620418b1a641 -r d776b59196c5 src/njs_function.h
--- a/src/njs_function.h Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_function.h Fri Jan 21 14:31:30 2022 +0000
@@ -112,7 +112,8 @@ njs_int_t njs_function_lambda_frame(njs_
njs_int_t njs_function_call2(njs_vm_t *vm, njs_function_t *function,
const njs_value_t *this, const njs_value_t *args,
njs_uint_t nargs, njs_value_t *retval, njs_bool_t ctor);
-njs_int_t njs_function_lambda_call(njs_vm_t *vm);
+njs_int_t njs_function_lambda_call(njs_vm_t *vm, void *promise_cap,
+ void *async_ctx);
njs_int_t njs_function_native_call(njs_vm_t *vm);
njs_native_frame_t *njs_function_frame_alloc(njs_vm_t *vm, size_t size);
void njs_function_frame_free(njs_vm_t *vm, njs_native_frame_t *frame);
diff -r 620418b1a641 -r d776b59196c5 src/njs_value.h
--- a/src/njs_value.h Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_value.h Fri Jan 21 14:31:30 2022 +0000
@@ -270,7 +270,6 @@ struct njs_function_s {
} u;

void *context;
- void *await;

njs_value_t *bound;
};
diff -r 620418b1a641 -r d776b59196c5 src/njs_vm.c
--- a/src/njs_vm.c Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_vm.c Fri Jan 21 14:31:30 2022 +0000
@@ -490,7 +490,7 @@ njs_vm_start(njs_vm_t *vm)
return ret;
}

- ret = njs_vmcode_interpreter(vm, vm->start);
+ ret = njs_vmcode_interpreter(vm, vm->start, NULL, NULL);

return (ret == NJS_ERROR) ? NJS_ERROR : NJS_OK;
}
diff -r 620418b1a641 -r d776b59196c5 src/njs_vmcode.c
--- a/src/njs_vmcode.c Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_vmcode.c Fri Jan 21 14:31:30 2022 +0000
@@ -42,7 +42,8 @@ static njs_jump_off_t njs_vmcode_debugge
static njs_jump_off_t njs_vmcode_return(njs_vm_t *vm, njs_value_t *invld,
njs_value_t *retval);

-static njs_jump_off_t njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await);
+static njs_jump_off_t njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await,
+ njs_promise_capability_t *pcap, njs_async_ctx_t *actx);

static njs_jump_off_t njs_vmcode_try_start(njs_vm_t *vm, njs_value_t *value,
njs_value_t *offset, u_char *pc);
@@ -77,7 +78,8 @@ static njs_jump_off_t njs_function_frame


njs_int_t
-njs_vmcode_interpreter(njs_vm_t *vm, u_char *pc)
+njs_vmcode_interpreter(njs_vm_t *vm, u_char *pc, void *promise_cap,
+ void *async_ctx)
{
u_char *catch;
double num, exponent;
@@ -826,7 +828,7 @@ next:

case NJS_VMCODE_AWAIT:
await = (njs_vmcode_await_t *) pc;
- return njs_vmcode_await(vm, await);
+ return njs_vmcode_await(vm, await, promise_cap, async_ctx);

case NJS_VMCODE_TRY_START:
ret = njs_vmcode_try_start(vm, value1, value2, pc);
@@ -1812,7 +1814,8 @@ njs_vmcode_return(njs_vm_t *vm, njs_valu


static njs_jump_off_t
-njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await)
+njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await,
+ njs_promise_capability_t *pcap, njs_async_ctx_t *ctx)
{
size_t size;
njs_int_t ret;
@@ -1820,7 +1823,6 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcod
njs_value_t ctor, val, on_fulfilled, on_rejected, *value;
njs_promise_t *promise;
njs_function_t *fulfilled, *rejected;
- njs_async_ctx_t *ctx;
njs_native_frame_t *active;

active = &vm->active_frame->native;
@@ -1837,8 +1839,6 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcod
return NJS_ERROR;
}

- ctx = active->function->await;
-
if (ctx == NULL) {
ctx = njs_mp_alloc(vm->mem_pool, sizeof(njs_async_ctx_t));
if (njs_slow_path(ctx == NULL)) {
@@ -1854,9 +1854,7 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcod
}

ctx->await = fulfilled->context;
- ctx->capability = active->function->context;
-
- active->function->context = NULL;
+ ctx->capability = pcap;

ret = njs_function_frame_save(vm, ctx->await, NULL);
if (njs_slow_path(ret != NJS_OK)) {
diff -r 620418b1a641 -r d776b59196c5 src/njs_vmcode.h
--- a/src/njs_vmcode.h Wed Jan 19 14:03:49 2022 +0000
+++ b/src/njs_vmcode.h Fri Jan 21 14:31:30 2022 +0000
@@ -437,7 +437,8 @@ typedef struct {
} njs_vmcode_await_t;


-njs_int_t njs_vmcode_interpreter(njs_vm_t *vm, u_char *pc);
+njs_int_t njs_vmcode_interpreter(njs_vm_t *vm, u_char *pc,
+ void *promise_cap, void *async_ctx);

njs_object_t *njs_function_new_object(njs_vm_t *vm, njs_value_t *constructor);

diff -r 620418b1a641 -r d776b59196c5 test/js/async_recursive_last.t.js
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/async_recursive_last.t.js Fri Jan 21 14:31:30 2022 +0000
@@ -0,0 +1,26 @@
+/*---
+includes: [compareArray.js]
+flags: [async]
+---*/
+
+let stages = [];
+
+async function f(v) {
+ if (v == 3) {
+ return;
+ }
+
+ stages.push(`f>${v}`);
+
+ f(v + 1);
+
+ stages.push(`f<${v}`);
+
+ await "X";
+}
+
+f(0)
+.then(v => {
+ assert.compareArray(stages, ['f>0', 'f>1', 'f>2', 'f<2', 'f<1', 'f<0']);
+})
+.then($DONE, $DONE);
diff -r 620418b1a641 -r d776b59196c5 test/js/async_recursive_mid.t.js
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/async_recursive_mid.t.js Fri Jan 21 14:31:30 2022 +0000
@@ -0,0 +1,26 @@
+/*---
+includes: [compareArray.js]
+flags: [async]
+---*/
+
+let stages = [];
+
+async function f(v) {
+ if (v == 3) {
+ return;
+ }
+
+ stages.push(`f>${v}`);
+
+ await "X";
+
+ f(v + 1);
+
+ stages.push(`f<${v}`);
+}
+
+f(0)
+.then(v => {
+ assert.compareArray(stages, ['f>0','f>1','f<0','f>2','f<1']);
+})
+.then($DONE, $DONE);
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org