details: https://hg.nginx.org/njs/rev/d776b59196c5 branches: changeset: 1814:d776b59196c5 user: Dmitry Volyntsev <xei...@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-le...@nginx.org