Commit Diff
Diff:
fda7b99fc7f19b04eced114983268cfe3eb46c99
2c3e53dac6faed4d9502bd3310b4837f0d3112cf
Commit:
2c3e53dac6faed4d9502bd3310b4837f0d3112cf
Tree:
baaaccb963b920152e486c66672add78629b9d3e
Author:
Omar Polo <op@omarpolo.com>
Date:
Wed Mar 3 17:22:01 2021 UTC
Message:
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 <err.h>
#include <errno.h>
+#include <event.h>
#include <fcntl.h>
#include <libgen.h>
#include <limits.h>
@@ -397,14 +398,49 @@ int
_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, &notbefore)
+ || !recv_time(fd, &notafter)
+ || !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, &notbefore)
- || !recv_time(exfd, &notafter)
- || !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 @@ runs 3 server processes by default, when not in config
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 @@ int exfd, logfd, sock4, sock6;
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 @@ struct iri {
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_prefork_num(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;
}
Omar Polo