Welcome! Log In Create A New Profile

Advanced

[njs] Fixed copying of closures for declared functions.

Alexander Borisov
October 11, 2021 10:48AM
details: https://hg.nginx.org/njs/rev/66bd2cc7fd87
branches:
changeset: 1719:66bd2cc7fd87
user: Alexander Borisov <alexander.borisov@nginx.com>
date: Mon Oct 11 17:46:24 2021 +0300
description:
Fixed copying of closures for declared functions.

After 0a2a0b5a74f4 (0.6.0), the referencing of a closure value inside of
a nested function may result in heap-use-after-free. For this to happen
the closure value have to be referenced in a function invoked asynchronously.
For example if a closure value is referenced in r.subrequest() or setTimeout()
handler.

The problem was that closure values of nested function were assigned during
the function call and the memory shared between all the cloned VMs was used
to store temporary assignments until the moment the declared function was
referenced. When two VMs executed concurrently, the first VM might see
the changed made by the second VM if the first one was suspended.

The fix is to copy all declared functions at the time of the call.

This closes #421 issue on GitHub.

diffstat:

src/njs_function.c | 11 ++++++++++-
src/njs_function.h | 2 +-
src/njs_parser.h | 6 ++++++
src/njs_variable.c | 14 ++++++++------
src/test/njs_unit_test.c | 2 --
5 files changed, 25 insertions(+), 10 deletions(-)

diffs (127 lines):

diff -r 839307cc293a -r 66bd2cc7fd87 src/njs_function.c
--- a/src/njs_function.c Fri Oct 08 13:50:50 2021 +0000
+++ b/src/njs_function.c Mon Oct 11 17:46:24 2021 +0300
@@ -615,6 +615,7 @@ njs_function_lambda_call(njs_vm_t *vm)
njs_value_t *args, **local, *value;
njs_value_t **cur_local, **cur_closures, **cur_temp;
njs_function_t *function;
+ njs_declaration_t *declr;
njs_function_lambda_t *lambda;

frame = (njs_frame_t *) vm->top_frame;
@@ -680,7 +681,15 @@ njs_function_lambda_call(njs_vm_t *vm)
while (n != 0) {
n--;

- function = njs_function(lambda->declarations[n]);
+ declr = &lambda->declarations[n];
+ value = njs_scope_value(vm, declr->index);
+
+ *value = *declr->value;
+
+ function = njs_function_value_copy(vm, value);
+ if (njs_slow_path(function == NULL)) {
+ return NJS_ERROR;
+ }

ret = njs_function_capture_closure(vm, function, function->u.lambda);
if (njs_slow_path(ret != NJS_OK)) {
diff -r 839307cc293a -r 66bd2cc7fd87 src/njs_function.h
--- a/src/njs_function.h Fri Oct 08 13:50:50 2021 +0000
+++ b/src/njs_function.h Mon Oct 11 17:46:24 2021 +0300
@@ -14,7 +14,7 @@ struct njs_function_lambda_s {
uint32_t nlocal;
uint32_t temp;

- njs_value_t **declarations;
+ njs_declaration_t *declarations;
uint32_t ndeclarations;

njs_index_t self;
diff -r 839307cc293a -r 66bd2cc7fd87 src/njs_parser.h
--- a/src/njs_parser.h Fri Oct 08 13:50:50 2021 +0000
+++ b/src/njs_parser.h Mon Oct 11 17:46:24 2021 +0300
@@ -104,6 +104,12 @@ typedef struct {
} njs_parser_rbtree_node_t;


+typedef struct {
+ njs_value_t *value;
+ njs_index_t index;
+} njs_declaration_t;
+
+
typedef njs_int_t (*njs_parser_traverse_cb_t)(njs_vm_t *vm,
njs_parser_node_t *node, void *ctx);

diff -r 839307cc293a -r 66bd2cc7fd87 src/njs_variable.c
--- a/src/njs_variable.c Fri Oct 08 13:50:50 2021 +0000
+++ b/src/njs_variable.c Mon Oct 11 17:46:24 2021 +0300
@@ -9,7 +9,7 @@
#include <njs_main.h>


-static njs_value_t **njs_variable_scope_function_add(njs_parser_t *parser,
+static njs_declaration_t *njs_variable_scope_function_add(njs_parser_t *parser,
njs_parser_scope_t *scope);
static njs_parser_scope_t *njs_variable_scope_find(njs_parser_t *parser,
njs_parser_scope_t *scope, uintptr_t unique_id, njs_variable_type_t type);
@@ -39,8 +39,8 @@ njs_variable_function_add(njs_parser_t *
uintptr_t unique_id, njs_variable_type_t type)
{
njs_bool_t ctor;
- njs_value_t **declr;
njs_variable_t *var;
+ njs_declaration_t *declr;
njs_parser_scope_t *root;
njs_function_lambda_t *lambda;

@@ -76,10 +76,12 @@ njs_variable_function_add(njs_parser_t *
return NULL;
}

- *declr = &var->value;
-
var->index = njs_scope_index(root->type, root->items, NJS_LEVEL_LOCAL,
type);
+
+ declr->value = &var->value;
+ declr->index = var->index;
+
root->items++;
}

@@ -90,12 +92,12 @@ njs_variable_function_add(njs_parser_t *
}


-static njs_value_t **
+static njs_declaration_t *
njs_variable_scope_function_add(njs_parser_t *parser, njs_parser_scope_t *scope)
{
if (scope->declarations == NULL) {
scope->declarations = njs_arr_create(parser->vm->mem_pool, 1,
- sizeof(njs_value_t *));
+ sizeof(njs_declaration_t));
if (njs_slow_path(scope->declarations == NULL)) {
return NULL;
}
diff -r 839307cc293a -r 66bd2cc7fd87 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Fri Oct 08 13:50:50 2021 +0000
+++ b/src/test/njs_unit_test.c Mon Oct 11 17:46:24 2021 +0300
@@ -20964,7 +20964,6 @@ static njs_unit_test_t njs_async_handle
),
njs_str("1") },

-#if 0 /* FIXME */
{ njs_str("globalThis.main = (function() {"
" let obj = { a: 1, b: 2};"
" function cb(r) { r.retval(obj.a); }"
@@ -20975,7 +20974,6 @@ static njs_unit_test_t njs_async_handle
"})();"
),
njs_str("1") },
-#endif
};


_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[njs] Fixed copying of closures for declared functions.

Alexander Borisov 124 October 11, 2021 10:48AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 85
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready