From 473a670583b11cab6c5c82c37eefe1d7a5313d29 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Sun, 14 Apr 2024 20:04:40 -0700 Subject: [PATCH] refactor(wasmx) store ngx_wa_conf_t as our core module context This ensures `ngx_wasm_core_cycle_get_conf` can return `NULL` when one of the `wasm{}` or `ipc{}` subsystems are not initialized. Prior, the memory read was uninitialized in some cases (i.e. 001-ipc_block.t TEST 1). --- src/ngx_wasmx.c | 52 +++++++++++++++++---------------- src/ngx_wasmx.h | 10 +++++++ src/wasm/ngx_wasm.h | 8 ++--- src/wasm/ngx_wasm_core_module.c | 44 ++++++++++++++-------------- 4 files changed, 63 insertions(+), 51 deletions(-) diff --git a/src/ngx_wasmx.c b/src/ngx_wasmx.c index e80b48dd9..40787eabc 100644 --- a/src/ngx_wasmx.c +++ b/src/ngx_wasmx.c @@ -23,9 +23,6 @@ ngx_uint_t ngx_wasm_max_module = 0; ngx_uint_t ngx_ipc_max_module = 0; -static ngx_wa_conf_t *wacf = NULL; - - static ngx_command_t ngx_wasmx_cmds[] = { { ngx_string("wasm"), @@ -75,9 +72,10 @@ static char * ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, ngx_uint_t type, ngx_uint_t conf_type) { - void ***ctx = NULL; + void **confs; char *rv; ngx_module_t *m; + ngx_wa_conf_t *wacf = NULL; ngx_uint_t i; ngx_conf_t pcf; ngx_wa_create_conf_pt create_conf; @@ -86,43 +84,41 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, if (*(void **) conf) { /* ngx_wasmx_module conf found, wasm{} or ipc{} */ - ngx_wa_assert(wacf); + wacf = *(ngx_wa_conf_t **) conf; if (wacf->initialized_types & conf_type) { return NGX_WA_CONF_ERR_DUPLICATE; } - - ctx = *(void **) conf; } - if (ctx == NULL) { + if (wacf == NULL) { ngx_wasm_max_module = ngx_count_modules(cf->cycle, NGX_WASM_MODULE); #ifdef NGX_WA_IPC ngx_ipc_max_module = ngx_count_modules(cf->cycle, NGX_IPC_MODULE); #endif - ctx = ngx_pcalloc(cf->pool, sizeof(void *)); - if (ctx == NULL) { + wacf = ngx_pcalloc(cf->pool, sizeof(ngx_wa_conf_t)); + if (wacf == NULL) { return NGX_CONF_ERROR; } - *ctx = ngx_pcalloc(cf->pool, - ngx_wasm_max_module * sizeof(void *) - + ngx_ipc_max_module * sizeof(void *)); - if (*ctx == NULL) { + wacf->wasm_confs = ngx_pcalloc(cf->pool, + sizeof(void *) * ngx_wasm_max_module); + if (wacf->wasm_confs == NULL) { return NGX_CONF_ERROR; } - wacf = ngx_pcalloc(cf->pool, sizeof(ngx_wa_conf_t)); - if (wacf == NULL) { +#ifdef NGX_WA_IPC + wacf->ipc_confs = ngx_pcalloc(cf->pool, + sizeof(void *) * ngx_ipc_max_module); + if (wacf->ipc_confs == NULL) { return NGX_CONF_ERROR; } +#endif - *(void **) conf = ctx; + *(ngx_wa_conf_t **) conf = wacf; } - ngx_wa_assert(wacf); - wacf->initialized_types |= conf_type; /* create_conf */ @@ -141,12 +137,14 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, case NGX_WASM_MODULE: { ngx_wasm_module_t *wm = m->ctx; create_conf = wm->create_conf; + confs = wacf->wasm_confs; break; } #ifdef NGX_WA_IPC case NGX_IPC_MODULE: { ngx_ipc_module_t *im = m->ctx; create_conf = im->create_conf; + confs = wacf->ipc_confs; break; } #endif @@ -156,8 +154,8 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, } if (create_conf) { - (*ctx)[m->ctx_index] = create_conf(cf); - if ((*ctx)[m->ctx_index] == NULL) { + confs[m->ctx_index] = create_conf(cf); + if (confs[m->ctx_index] == NULL) { return NGX_CONF_ERROR; } } @@ -167,7 +165,7 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, pcf = *cf; - cf->ctx = ctx; + cf->ctx = wacf; cf->module_type = type; cf->cmd_type = conf_type; @@ -195,12 +193,14 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, case NGX_WASM_MODULE: { ngx_wasm_module_t *wm = m->ctx; init_conf = wm->init_conf; + confs = wacf->wasm_confs; break; } #ifdef NGX_WA_IPC case NGX_IPC_MODULE: { ngx_ipc_module_t *im = m->ctx; init_conf = im->init_conf; + confs = wacf->ipc_confs; break; } #endif @@ -210,7 +210,7 @@ ngx_wasmx_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf, } if (init_conf) { - rv = init_conf(cf, (*ctx)[m->ctx_index]); + rv = init_conf(cf, confs[m->ctx_index]); if (rv != NGX_CONF_OK) { return rv; } @@ -240,9 +240,11 @@ ngx_ipc_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) static ngx_int_t ngx_wasmx_init(ngx_cycle_t *cycle) { - size_t i; - ngx_int_t rc; + size_t i; + ngx_int_t rc; + ngx_wa_conf_t *wacf; + wacf = ngx_wa_cycle_get_conf(cycle); if (wacf == NULL) { return NGX_OK; } diff --git a/src/ngx_wasmx.h b/src/ngx_wasmx.h index 74d467151..e61001fd2 100644 --- a/src/ngx_wasmx.h +++ b/src/ngx_wasmx.h @@ -14,6 +14,12 @@ #define NGX_WA_BAD_FD (ngx_socket_t) -1 +#define NGX_WA_WASM_CONF_OFFSET offsetof(ngx_wa_conf_t, wasm_confs) +#define NGX_WA_IPC_CONF_OFFSET offsetof(ngx_wa_conf_t, ipc_confs) + +#define ngx_wa_cycle_get_conf(cycle) \ + (ngx_wa_conf_t *) ngx_get_conf(cycle->conf_ctx, ngx_wasmx_module) + typedef void *(*ngx_wa_create_conf_pt)(ngx_conf_t *cf); typedef char *(*ngx_wa_init_conf_pt)(ngx_conf_t *cf, void *conf); @@ -22,6 +28,10 @@ typedef ngx_int_t (*ngx_wa_init_pt)(ngx_cycle_t *cycle); typedef struct { ngx_uint_t initialized_types; + void **wasm_confs; +#ifdef NGX_WA_IPC + void **ipc_confs; +#endif } ngx_wa_conf_t; diff --git a/src/wasm/ngx_wasm.h b/src/wasm/ngx_wasm.h index d2433044a..e16b9d9d0 100644 --- a/src/wasm/ngx_wasm.h +++ b/src/wasm/ngx_wasm.h @@ -37,10 +37,10 @@ #define NGX_WASM_DEFAULT_RESP_BODY_BUF_SIZE 4096 #define ngx_wasm_core_cycle_get_conf(cycle) \ - (ngx_get_conf(cycle->conf_ctx, ngx_wasmx_module)) \ - ? (*(ngx_get_conf(cycle->conf_ctx, ngx_wasmx_module))) \ - [ngx_wasm_core_module.ctx_index] \ - : NULL + (cycle->conf_ctx[ngx_wasmx_module.index] \ + ? ((ngx_wa_conf_t *) cycle->conf_ctx[ngx_wasmx_module.index]) \ + ->wasm_confs[ngx_wasm_core_module.ctx_index] \ + : NULL) #define ngx_wasm_vec_set_i32(vec, i, v) \ (((wasm_val_vec_t *) (vec))->data[i].kind = WASM_I32); \ diff --git a/src/wasm/ngx_wasm_core_module.c b/src/wasm/ngx_wasm_core_module.c index 56c51e3bb..8275dad12 100644 --- a/src/wasm/ngx_wasm_core_module.c +++ b/src/wasm/ngx_wasm_core_module.c @@ -25,21 +25,21 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("wasmtime"), NGX_WASM_CONF|NGX_CONF_BLOCK|NGX_CONF_NOARGS, ngx_wasm_core_wasmtime_block, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("wasmer"), NGX_WASM_CONF|NGX_CONF_BLOCK|NGX_CONF_NOARGS, ngx_wasm_core_wasmer_block, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("v8"), NGX_WASM_CONF|NGX_CONF_BLOCK|NGX_CONF_NOARGS, ngx_wasm_core_v8_block, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, @@ -48,14 +48,14 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("flag"), NGX_WASMTIME_CONF|NGX_WASMER_CONF|NGX_V8_CONF|NGX_CONF_TAKE2, ngx_wasm_core_flag_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("compiler"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_str_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, vm_conf) + offsetof(ngx_wavm_conf_t, compiler), NULL }, @@ -63,7 +63,7 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("backtraces"), NGX_WASM_CONF|NGX_CONF_FLAG, ngx_conf_set_flag_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, vm_conf) + offsetof(ngx_wavm_conf_t, backtraces), NULL }, @@ -71,21 +71,21 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("module"), NGX_WASM_CONF|NGX_CONF_TAKE23, ngx_wasm_core_module_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("shm_kv"), NGX_WASM_CONF|NGX_CONF_TAKE23|NGX_CONF_TAKE4, ngx_wasm_core_shm_kv_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("shm_queue"), NGX_WASM_CONF|NGX_CONF_TAKE23|NGX_CONF_TAKE4, ngx_wasm_core_shm_queue_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, @@ -93,7 +93,7 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("tls_trusted_certificate"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_str_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, ssl_conf) + offsetof(ngx_wasm_ssl_conf_t, trusted_certificate), NULL }, @@ -101,7 +101,7 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("tls_verify_cert"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_flag_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, ssl_conf) + offsetof(ngx_wasm_ssl_conf_t, verify_cert), NULL }, @@ -109,7 +109,7 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("tls_verify_host"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_flag_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, ssl_conf) + offsetof(ngx_wasm_ssl_conf_t, verify_host), NULL }, @@ -117,7 +117,7 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("tls_no_verify_warn"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_flag_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, ssl_conf) + offsetof(ngx_wasm_ssl_conf_t, no_verify_warn), NULL }, @@ -126,63 +126,63 @@ static ngx_command_t ngx_wasm_core_commands[] = { { ngx_string("resolver"), NGX_WASM_CONF|NGX_CONF_1MORE, ngx_wasm_core_resolver_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, 0, NULL }, { ngx_string("resolver_timeout"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_msec_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, resolver_timeout), NULL }, { ngx_string("proxy_wasm_lua_resolver"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_wasm_core_pwm_lua_resolver_directive, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, pwm_lua_resolver), NULL }, { ngx_string("socket_connect_timeout"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_msec_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, connect_timeout), NULL }, { ngx_string("socket_send_timeout"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_msec_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, send_timeout), NULL }, { ngx_string("socket_read_timeout"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_msec_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, recv_timeout), NULL }, { ngx_string("socket_buffer_size"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_size_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, socket_buffer_size), NULL }, { ngx_string("socket_buffer_reuse"), NGX_WASM_CONF|NGX_CONF_TAKE1, ngx_conf_set_flag_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, socket_buffer_reuse), NULL }, { ngx_string("socket_large_buffers"), NGX_WASM_CONF|NGX_CONF_TAKE2, ngx_conf_set_bufs_slot, - 0, + NGX_WA_WASM_CONF_OFFSET, offsetof(ngx_wasm_core_conf_t, socket_large_buffers), NULL },