From 62d173f7af34eb3c6323e8c8aac15e20498c1785 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Tue, 23 Nov 2021 15:36:24 +0300 Subject: Fixed possible access to an uninitialized field. The "recv_msg.incoming_buf" is checked after jumping to the "done" label if nxt_socket_msg_oob_get_fds() returns an error. Also moved initialization of "port_msg" near to its first usage. Found by Coverity (CID 373899). --- src/nxt_unit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 06ad1636..135c06ed 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -937,9 +937,9 @@ nxt_unit_process_msg(nxt_unit_ctx_t *ctx, nxt_unit_read_buf_t *rbuf, lib = nxt_container_of(ctx->unit, nxt_unit_impl_t, unit); + recv_msg.incoming_buf = NULL; recv_msg.fd[0] = -1; recv_msg.fd[1] = -1; - port_msg = (nxt_port_msg_t *) rbuf->buf; rc = nxt_socket_msg_oob_get_fds(&rbuf->oob, recv_msg.fd); if (nxt_slow_path(rc != NXT_OK)) { @@ -948,8 +948,6 @@ nxt_unit_process_msg(nxt_unit_ctx_t *ctx, nxt_unit_read_buf_t *rbuf, goto done; } - recv_msg.incoming_buf = NULL; - if (nxt_slow_path(rbuf->size < (ssize_t) sizeof(nxt_port_msg_t))) { if (nxt_slow_path(rbuf->size == 0)) { nxt_unit_debug(ctx, "read port closed"); @@ -965,6 +963,8 @@ nxt_unit_process_msg(nxt_unit_ctx_t *ctx, nxt_unit_read_buf_t *rbuf, goto done; } + port_msg = (nxt_port_msg_t *) rbuf->buf; + nxt_unit_debug(ctx, "#%"PRIu32": process message %d fd[0] %d fd[1] %d", port_msg->stream, (int) port_msg->type, recv_msg.fd[0], recv_msg.fd[1]); -- cgit From c33c2925d91510e1c059a1581aad6dd959817191 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Wed, 24 Nov 2021 13:11:48 +0300 Subject: Fixing alerts on router restart. Splitting the process type connectivity matrix to 'keep ports' and 'send ports'; the 'keep ports' matrix is used to clean up unnecessary ports after forking a new process, and the 'send ports' matrix determines which process types expect to get created process ports. Unfortunately, the original single connectivity matrix no longer works because of an application stop delay caused by prototypes. Existing applications should not get the new router port at the moment. --- src/nxt_port.c | 2 +- src/nxt_process.c | 15 ++++++++++++--- src/nxt_process.h | 6 +++--- 3 files changed, 16 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/nxt_port.c b/src/nxt_port.c index 1e8fa28a..a5b64695 100644 --- a/src/nxt_port.c +++ b/src/nxt_port.c @@ -216,7 +216,7 @@ nxt_port_send_new_port(nxt_task_t *task, nxt_runtime_t *rt, port = nxt_process_port_first(process); - if (nxt_proc_conn_matrix[port->type][new_port->type]) { + if (nxt_proc_send_matrix[port->type][new_port->type]) { (void) nxt_port_send_port(task, port, new_port, stream); } diff --git a/src/nxt_process.c b/src/nxt_process.c index fca197eb..82e66a99 100644 --- a/src/nxt_process.c +++ b/src/nxt_process.c @@ -58,7 +58,7 @@ nxt_uid_t nxt_euid; /* A cached process effective gid */ nxt_gid_t nxt_egid; -nxt_bool_t nxt_proc_conn_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { +uint8_t nxt_proc_keep_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { { 1, 1, 1, 1, 1, 1 }, { 1, 0, 0, 0, 0, 0 }, { 1, 0, 0, 1, 0, 0 }, @@ -67,7 +67,16 @@ nxt_bool_t nxt_proc_conn_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { { 1, 0, 0, 1, 0, 0 }, }; -nxt_bool_t nxt_proc_remove_notify_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { +uint8_t nxt_proc_send_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { + { 1, 1, 1, 1, 1, 1 }, + { 1, 0, 0, 0, 0, 0 }, + { 1, 0, 0, 1, 0, 0 }, + { 1, 0, 1, 1, 1, 1 }, + { 1, 0, 0, 0, 0, 0 }, + { 1, 0, 0, 0, 0, 0 }, +}; + +uint8_t nxt_proc_remove_notify_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = { { 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 1, 0, 0 }, @@ -265,7 +274,7 @@ nxt_process_child_fixup(nxt_task_t *task, nxt_process_t *process) /* Remove not ready processes. */ nxt_runtime_process_each(rt, p) { - if (nxt_proc_conn_matrix[ptype][nxt_process_type(p)] == 0 + if (nxt_proc_keep_matrix[ptype][nxt_process_type(p)] == 0 && p->pid != nxt_ppid) /* Always keep parent's port. */ { nxt_debug(task, "remove not required process %PI", p->pid); diff --git a/src/nxt_process.h b/src/nxt_process.h index c92eebd8..694f457e 100644 --- a/src/nxt_process.h +++ b/src/nxt_process.h @@ -151,9 +151,9 @@ typedef struct { } nxt_process_init_t; -extern nxt_bool_t nxt_proc_conn_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX]; -extern nxt_bool_t - nxt_proc_remove_notify_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX]; +extern uint8_t nxt_proc_keep_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX]; +extern uint8_t nxt_proc_send_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX]; +extern uint8_t nxt_proc_remove_notify_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX]; NXT_EXPORT nxt_pid_t nxt_process_execute(nxt_task_t *task, char *name, char **argv, char **envp); -- cgit From aaa34e0a644912e49d7c5619749b67f585da9c85 Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Wed, 24 Nov 2021 13:11:50 +0300 Subject: Fixing zombie process appearance and hang up on shutdown. After the c8790d2a89bb commit, the SIGCHLD handler may return before processing all awaiting PIDs. To avoid zombie processes and ensure successful main process termination, waitpid() must be called until an error is returned. This closes #600 issue on GitHub. --- src/nxt_main_process.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/nxt_main_process.c b/src/nxt_main_process.c index a5a20d3d..61521854 100644 --- a/src/nxt_main_process.c +++ b/src/nxt_main_process.c @@ -950,9 +950,11 @@ nxt_main_process_sigchld_handler(nxt_task_t *task, void *obj, void *data) if (rt->nprocesses <= 1) { nxt_runtime_quit(task, 0); + + return; } - return; + continue; } nxt_port_remove_notify_others(task, process); -- cgit From 1c0436d644c3a313096cd7d43c4148151c2193ee Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Thu, 25 Nov 2021 16:58:43 +0300 Subject: Fixing access_log structure reference counting. The reference to the access_log structure is stored in the current nxt_router_conf_t and the global nxt_router_t. When the reference is copied, the reference counter should be adjusted accordingly. This closes #593 issue on GitHub. --- src/nxt_router.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/nxt_router.c b/src/nxt_router.c index 7623ccbb..85556a72 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -210,6 +210,8 @@ static void nxt_router_access_log_ready(nxt_task_t *task, nxt_port_recv_msg_t *msg, void *data); static void nxt_router_access_log_error(nxt_task_t *task, nxt_port_recv_msg_t *msg, void *data); +static void nxt_router_access_log_use(nxt_thread_spinlock_t *lock, + nxt_router_access_log_t *access_log); static void nxt_router_access_log_release(nxt_task_t *task, nxt_thread_spinlock_t *lock, nxt_router_access_log_t *access_log); static void nxt_router_access_log_reopen_completion(nxt_task_t *task, void *obj, @@ -1149,7 +1151,13 @@ nxt_router_conf_apply(nxt_task_t *task, void *obj, void *data) nxt_queue_add(&router->sockets, &updating_sockets); nxt_queue_add(&router->sockets, &creating_sockets); - router->access_log = rtcf->access_log; + if (router->access_log != rtcf->access_log) { + nxt_router_access_log_use(&router->lock, rtcf->access_log); + + nxt_router_access_log_release(task, &router->lock, router->access_log); + + router->access_log = rtcf->access_log; + } nxt_router_conf_ready(task, tmcf); @@ -1971,9 +1979,7 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, access_log = router->access_log; if (access_log != NULL && nxt_strstr_eq(&path, &access_log->path)) { - nxt_thread_spin_lock(&router->lock); - access_log->count++; - nxt_thread_spin_unlock(&router->lock); + nxt_router_access_log_use(&router->lock, access_log); } else { access_log = nxt_malloc(sizeof(nxt_router_access_log_t) @@ -3922,6 +3928,22 @@ nxt_router_access_log_error(nxt_task_t *task, nxt_port_recv_msg_t *msg, } +static void +nxt_router_access_log_use(nxt_thread_spinlock_t *lock, + nxt_router_access_log_t *access_log) +{ + if (access_log == NULL) { + return; + } + + nxt_thread_spin_lock(lock); + + access_log->count++; + + nxt_thread_spin_unlock(lock); +} + + static void nxt_router_access_log_release(nxt_task_t *task, nxt_thread_spinlock_t *lock, nxt_router_access_log_t *access_log) -- cgit From d4b13c7cd5520b2b8bd8833765a3ba4246a93df7 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Thu, 25 Nov 2021 19:58:54 +0300 Subject: PHP: fixed crash when calling module functions in OPcache preload. In PHP, custom fastcgi_finish_request() and overloaded chdir() functions can be invoked by an OPcache preloading script (it runs when php_module_startup() is called in the app process setup handler). In this case, there was no runtime context set so trying to access it caused a segmentation fault. This closes #602 issue on GitHub. --- src/nxt_php_sapi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c index ea5f5581..68ef07eb 100644 --- a/src/nxt_php_sapi.c +++ b/src/nxt_php_sapi.c @@ -204,7 +204,10 @@ ZEND_NAMED_FUNCTION(nxt_php_chdir) nxt_php_run_ctx_t *ctx; ctx = SG(server_context); - ctx->chdir = 1; + + if (nxt_fast_path(ctx != NULL)) { + ctx->chdir = 1; + } nxt_php_chdir_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -225,7 +228,7 @@ PHP_FUNCTION(fastcgi_finish_request) ctx = SG(server_context); - if (nxt_slow_path(ctx->req == NULL)) { + if (nxt_slow_path(ctx == NULL || ctx->req == NULL)) { RETURN_FALSE; } -- cgit From b8ea9d34fdff699006ecfdbd948f113686d2dc0c Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Wed, 1 Dec 2021 17:09:02 +0300 Subject: Logging of the daemon version on startup. --- src/nxt_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/nxt_main.c b/src/nxt_main.c index 03403991..26bee873 100644 --- a/src/nxt_main.c +++ b/src/nxt_main.c @@ -30,7 +30,7 @@ main(int argc, char **argv) return 1; } - nxt_log(&nxt_main_task, NXT_LOG_INFO, "unit started"); + nxt_log(&nxt_main_task, NXT_LOG_INFO, "unit " NXT_VERSION " started"); nxt_event_engine_start(nxt_main_task.thread->engine); -- cgit From 97e61aad736da5fe1ad136a1d3055e16698f7d2b Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Wed, 1 Dec 2021 18:05:50 +0300 Subject: Fixing prototype process crash. A prototype stores linked application processes structures. When an application process terminates, it's removed from the list. To avoid double removal, the pointer to the next element should be set to NULL. The issue was introduced in c8790d2a89bb. --- src/nxt_application.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/nxt_application.c b/src/nxt_application.c index 589821fb..82385ec4 100644 --- a/src/nxt_application.c +++ b/src/nxt_application.c @@ -1147,6 +1147,8 @@ nxt_proto_process_remove(nxt_task_t *task, nxt_pid_t pid) process = lhq.value; nxt_queue_remove(&process->link); + process->link.next = NULL; + break; default: -- cgit