From 834d6d59b468256fabbbc43eee9cd8b33bbe73f9 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Fri, 8 Dec 2023 19:14:48 -0800 Subject: [PATCH] fix(proxy-wasm) properly unlink trapped instances from root contexts An instance sweeped by a request but used in a root context could leave the root context with read-after-freed memory (e.g. next tick) in-between `instance_invalidate` and `instance_destroy`. --- src/common/proxy_wasm/ngx_proxy_wasm.c | 33 ++++++++++++++++---------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/common/proxy_wasm/ngx_proxy_wasm.c b/src/common/proxy_wasm/ngx_proxy_wasm.c index c027ed54f..a614e27a2 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm.c +++ b/src/common/proxy_wasm/ngx_proxy_wasm.c @@ -1113,8 +1113,11 @@ ngx_proxy_wasm_create_context(ngx_proxy_wasm_filter_t *filter, goto error; } + dd("link root ctx #%ld instance (rexec: %p)", rexec->id, rexec); + rexec->node.key = rexec->id; ngx_rbtree_insert(&ictx->root_ctxs, &rexec->node); + rexec->started = 1; } @@ -1709,13 +1712,11 @@ ngx_proxy_wasm_instance_invalidate(ngx_proxy_wasm_instance_t *ictx) while (*root != *sentinel) { n = ngx_rbtree_min(*root, *sentinel); -#if (DDEBUG) pwexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); - dd("invalidate root ctx #%ld instance (pwexec: %p)", - pwexec->id, pwexec); -#endif + dd("unlink instance of root ctx #%ld (rexec: %p)", pwexec->id, pwexec); + pwexec->ictx = NULL; ngx_rbtree_delete(&ictx->root_ctxs, n); } @@ -1729,10 +1730,9 @@ ngx_proxy_wasm_instance_invalidate(ngx_proxy_wasm_instance_t *ictx) n = ngx_rbtree_min(*root, *sentinel); pwexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); - dd("invalidate ctx #%ld instance (pwexec: %p)", pwexec->id, pwexec); + dd("destroy ctx #%ld (pwexec: %p)", pwexec->id, pwexec); ngx_rbtree_delete(&ictx->tree_ctxs, n); - destroy_pwexec(pwexec); } @@ -1750,9 +1750,11 @@ static void ngx_proxy_wasm_instance_destroy(ngx_proxy_wasm_instance_t *ictx) { ngx_rbtree_node_t **root, **sentinel, *s, *n; - ngx_proxy_wasm_exec_t *rexec; + ngx_proxy_wasm_exec_t *pwexec; - dd("enter"); + dd("enter (ictx: %p)", ictx); + + /* root context */ root = &ictx->root_ctxs.root; s = &ictx->sentinel_root_ctxs; @@ -1760,26 +1762,31 @@ ngx_proxy_wasm_instance_destroy(ngx_proxy_wasm_instance_t *ictx) while (*root != *sentinel) { n = ngx_rbtree_min(*root, *sentinel); - rexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); + pwexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); - rexec->ictx = NULL; + dd("unlink+destroy instance of root ctx #%ld (rexec: %p)", + pwexec->id, pwexec); + pwexec->ictx = NULL; ngx_rbtree_delete(&ictx->root_ctxs, n); } + /* stream context */ + root = &ictx->tree_ctxs.root; s = &ictx->sentinel_ctxs; sentinel = &s; while (*root != *sentinel) { n = ngx_rbtree_min(*root, *sentinel); - rexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); + pwexec = ngx_rbtree_data(n, ngx_proxy_wasm_exec_t, node); - ngx_wasm_assert(rexec->ev == NULL); + ngx_wasm_assert(pwexec->ev == NULL); - rexec->ictx = NULL; + dd("destroy ctx #%ld (pwexec: %p)", pwexec->id, pwexec); ngx_rbtree_delete(&ictx->tree_ctxs, n); + destroy_pwexec(pwexec); } ngx_wavm_instance_destroy(ictx->instance);