commit 2c3e53dac6faed4d9502bd3310b4837f0d3112cf from: Omar Polo date: Wed Mar 03 17:22:01 2021 UTC give each server process its own socket for the executor this fixes a bug introduced with the prefork mechanics: every server process shared the same socket, and this would cause a race condition when multiple server processes asked for a script cgi being executed. This gives each server process its own socket to talk to the executor, so the race cannot happen. commit - fda7b99fc7f19b04eced114983268cfe3eb46c99 commit + 2c3e53dac6faed4d9502bd3310b4837f0d3112cf blob - f1b4fa5df52ee194cc7c703fe826a2dfa6855599 blob + 5ff75159db1183a7bc7f6fcdf58020329571049d --- ex.c +++ ex.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -397,14 +398,49 @@ childerr: _exit(1); } -int -executor_main() +static void +handle_fork_req(int fd, short ev, void *data) { char *spath, *relpath, *addr, *ruser, *cissuer, *chash; - struct vhost *vhost; + struct vhost *vhost; struct iri iri; time_t notbefore, notafter; int d; + + if (!recv_iri(fd, &iri) + || !recv_string(fd, &spath) + || !recv_string(fd, &relpath) + || !recv_string(fd, &addr) + || !recv_string(fd, &ruser) + || !recv_string(fd, &cissuer) + || !recv_string(fd, &chash) + || !recv_time(fd, ¬before) + || !recv_time(fd, ¬after) + || !recv_vhost(fd, &vhost)) + fatal("failure in handling fork request"); + + d = launch_cgi(&iri, spath, relpath, addr, ruser, cissuer, chash, + notbefore, notafter, vhost); + if (!send_fd(fd, d)) + fatal("failure in sending the fd to the server: %s", + strerror(errno)); + close(d); + + free_recvd_iri(&iri); + free(spath); + free(relpath); + free(addr); + free(ruser); + free(cissuer); + free(chash); +} + +int +executor_main(void) +{ + struct vhost *vhost; + struct event evs[PROC_MAX]; + int i; #ifdef __OpenBSD__ for (vhost = hosts; vhost->domain != NULL; ++vhost) { @@ -419,39 +455,15 @@ executor_main() err(1, "pledge"); #endif - while (!hupped) { - if (!recv_iri(exfd, &iri) - || !recv_string(exfd, &spath) - || !recv_string(exfd, &relpath) - || !recv_string(exfd, &addr) - || !recv_string(exfd, &ruser) - || !recv_string(exfd, &cissuer) - || !recv_string(exfd, &chash) - || !recv_time(exfd, ¬before) - || !recv_time(exfd, ¬after) - || !recv_vhost(exfd, &vhost)) - break; + event_init(); - d = launch_cgi(&iri, spath, relpath, addr, ruser, cissuer, chash, - notbefore, notafter, vhost); - if (!send_fd(exfd, d)) - break; - close(d); - - free_recvd_iri(&iri); - free(spath); - free(relpath); - free(addr); - free(ruser); - free(cissuer); - free(chash); + for (i = 0; i < conf.prefork; ++i) { + event_set(&evs[i], servpipes[i], EV_READ | EV_PERSIST, + handle_fork_req, NULL); + event_add(&evs[i], NULL); } - if (hupped) - _exit(0); + event_dispatch(); - /* kill all process in my group. This means the listener and - * every pending CGI script. */ - kill(0, SIGINT); return 1; } blob - daa8a30114c2cb7b84a4bb5d3a1a862b4898cf8e blob + 7c76bb56f5fe6ba219d9b91817c7df8b323b0584 --- gmid.1 +++ gmid.1 @@ -159,6 +159,7 @@ This increases the performance and prevents delays whe a server. .Nm runs 3 server processes by default, when not in config-less mode. +The maximum number allowed is 16. .It Ic protocols Ar string Specify the TLS protocols to enable. Refer to blob - d14891ab6dfa1a4a68e79ee7c51da7fe78924c50 blob + 3671763da3eb448807659c929d683f2f7e4cce2f --- gmid.c +++ gmid.c @@ -29,7 +29,7 @@ volatile sig_atomic_t hupped; struct vhost hosts[HOSTSLEN]; -int exfd, logfd, sock4, sock6; +int exfd, logfd, sock4, sock6, servpipes[PROC_MAX]; struct imsgbuf logpibuf, logcibuf; @@ -297,31 +297,6 @@ drop_priv(void) if (getuid() == 0) log_warn(NULL, "not a good idea to run a network daemon as root"); -} - -static int -spawn_listeners(int *p) -{ - int i; - - close(p[0]); - exfd = p[1]; - - for (i = 0; i < conf.prefork -1; ++i) { - switch (fork()) { - case -1: - fatal("fork: %s", strerror(errno)); - case 0: /* child */ - setproctitle("server(%d)", i+1); - return listener_main(); - } - } - - if (conf.prefork == 0) - setproctitle("server"); - else - setproctitle("server(%d)", conf.prefork); - return listener_main(); } static void @@ -359,13 +334,13 @@ logger_init(void) } } - static int -serve(int argc, char **argv, int *p) +serve(int argc, char **argv) { - char path[PATH_MAX]; + char path[PATH_MAX]; + int i, p[2]; - if (config_path == NULL) { + if (config_path == NULL) { if (hostname == NULL) hostname = "localhost"; if (certs_dir == NULL) @@ -395,28 +370,35 @@ serve(int argc, char **argv, int *p) * to put private certs inside the chroot. */ setup_tls(); - switch (fork()) { - case -1: - fatal("fork: %s", strerror(errno)); + for (i = 0; i < conf.prefork; ++i) { + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, + PF_UNSPEC, p) == -1) + fatal("socketpair: %s", strerror(errno)); - case 0: /* child */ - return spawn_listeners(p); - - default: /* parent */ - setproctitle("executor"); - close(p[1]); - exfd = p[0]; - drop_priv(); - unblock_signals(); - return executor_main(); + switch (fork()) { + case -1: + fatal("fork: %s", strerror(errno)); + case 0: /* child */ + close(p[0]); + exfd = p[1]; + setproctitle("server"); + _exit(listener_main()); + default: + servpipes[i] = p[0]; + close(p[1]); + } } + + setproctitle("executor"); + drop_priv(); + unblock_signals(); + _exit(executor_main()); } int main(int argc, char **argv) { - int ch, p[2]; - int conftest = 0, configless = 0; + int ch, conftest = 0, configless = 0; int old_ipv6, old_port; init_config(); @@ -482,7 +464,7 @@ main(int argc, char **argv) if (config_path == NULL) { configless = 1; conf.foreground = 1; - conf.prefork = 0; + conf.prefork = 1; conf.verbose++; } @@ -509,10 +491,6 @@ main(int argc, char **argv) err(1, "daemon"); } - if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, - PF_UNSPEC, p) == -1) - err(1, "socketpair"); - if (config_path != NULL) parse_conf(config_path); @@ -524,7 +502,7 @@ main(int argc, char **argv) sock6 = make_socket(conf.port, AF_INET6); if (configless) - return serve(argc, argv, p); + return serve(argc, argv); /* wait a sighup and reload the daemon */ for (;;) { @@ -535,12 +513,9 @@ main(int argc, char **argv) case -1: fatal("fork: %s", strerror(errno)); case 0: - return serve(argc, argv, p); + return serve(argc, argv); } - close(p[0]); - close(p[1]); - wait_sighup(); unblock_signals(); log_info(NULL, "reloading configuration %s", config_path); @@ -568,9 +543,5 @@ main(int argc, char **argv) sock4 = make_socket(conf.port, AF_INET); if (sock6 == -1 && conf.ipv6) sock6 = make_socket(conf.port, AF_INET6); - - if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, - PF_UNSPEC, p) == -1) - fatal("socketpair: %s", strerror(errno)); } } blob - e926f9c5b69fa2e933874dbb5c5a69e6f6ea0a5d blob + b858ebc97cbf83139224a9c5d91c53ade5b33194 --- gmid.h +++ gmid.h @@ -55,6 +55,8 @@ /* maximum hostname and label length, +1 for the NUL-terminator */ #define DOMAIN_NAME_LEN (253+1) #define LABEL_LEN (63+1) + +#define PROC_MAX 16 struct location { const char *match; @@ -120,6 +122,8 @@ extern struct imsgbuf logpibuf, logcibuf; extern volatile sig_atomic_t hupped; +extern int servpipes[PROC_MAX]; + struct iri { char *schema; char *host; blob - 06fdf250bcc88b65db050fde6b1d7e2160329cbb blob + 717bcdcd6f01bf609b59e1d4cc6bef832e73b4fa --- parse.y +++ parse.y @@ -299,7 +299,7 @@ check_strip_no(int n) int check_prefork_num(int n) { - if (n <= 0) + if (n <= 0 || n >= PROC_MAX) yyerror("invalid prefork number %d", n); return n; }