Rework how socket fds are passed around internally. This will allow

more flexibility in listening sockets (and fixes a bug related to inet6).
Everything is in arrays now instead of discrete variables.
ok anton

1 files changed, 200 insertions, 80 deletions

diff --git a/usr.sbin/rebound/rebound.c b/usr.sbin/rebound/rebound.c
index e3b97a16737..afad55bba69 100644
--- a/usr.sbin/rebound/rebound.c
+++ b/usr.sbin/rebound/rebound.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: rebound.c,v 1.103 2018/12/06 16:51:19 tedu Exp $ */
+/* $OpenBSD: rebound.c,v 1.104 2018/12/18 20:34:32 tedu Exp $ */
 /*
  * Copyright (c) 2015 Ted Unangst <tedu@openbsd.org>
  *
@@ -529,7 +529,8 @@ sendreply(struct request *req, uint8_t *buf, size_t r)
 	resp->id = req->clientid;
 	if (ntohs(resp->qdcount) == 1) {
 		/* some more checking */
-		size_t namelen = dnamelen(resp->qname, r - sizeof(struct dnspacket));
+		size_t namelen = dnamelen(resp->qname,
+		    r - sizeof(struct dnspacket));
 		if (namelen > r - sizeof(struct dnspacket))
 			return;
 		if (namelen > NAMELEN)
@@ -861,6 +862,8 @@ workerinit(void)
 	TAILQ_INIT(&reqfifo);
 	TAILQ_INIT(&cachefifo);
 	RB_INIT(&cachetree);
+	
+	https_init();
 
 	if (!(pwd = getpwnam("_rebound")))
 		logerr("getpwnam failed");
@@ -880,11 +883,50 @@ workerinit(void)
 		logerr("pledge failed");
 }
 
+static void
+readevent(int kq, struct kevent *ke, struct sockaddr *remoteaddr,
+    int *udpfds, int numudp, int *tcpfds, int numtcp)
+{
+	struct kevent ch[2];
+	struct request *req;
+	int i;
+
+	req = ke->udata;
+	if (req != NULL) {
+		if (req->tcp == 0)
+			handlereply(req);
+		freerequest(req);
+		return;
+	}
+	for (i = 0; i < numudp; i++) {
+		if (ke->ident == udpfds[i]) {
+			if ((req = newrequest(ke->ident, remoteaddr))) {
+				EV_SET(&ch[0], req->s, EVFILT_READ,
+				    EV_ADD, 0, 0, req);
+				kevent(kq, ch, 1, NULL, 0, NULL);
+			}
+			return;
+		}
+	}
+	for (i = 0; i < numtcp; i++) {
+		if (ke->ident == tcpfds[i]) {
+			if ((req = newtcprequest(ke->ident, remoteaddr))) {
+				EV_SET(&ch[0], req->s,
+				    req->tcp == 1 ? EVFILT_WRITE :
+				    EVFILT_READ, EV_ADD, 0, 0, req);
+				kevent(kq, ch, 1, NULL, 0, NULL);
+			}
+			return;
+		}
+	}
+	logerr("read event on unknown fd");
+}
+
 static int
-workerloop(int conffd, int ud, int ld, int ud6, int ld6)
+workerloop(int conffd, int udpfds[], int numudp, int tcpfds[], int numtcp)
 {
 	union sockun remoteaddr;
-	struct kevent ch[2], kev[4];
+	struct kevent ch[2];
 	struct timespec ts, *timeout = NULL;
 	struct request *req;
 	struct dnscache *ent;
@@ -895,8 +937,8 @@ workerloop(int conffd, int ud, int ld, int ud6, int ld6)
 	if (!debug) {
 		pid_t parent = getppid();
 		/* would need pledge(proc) to do this below */
-		EV_SET(&kev[0], parent, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
-		if (kevent(kq, kev, 1, NULL, 0, NULL) == -1)
+		EV_SET(&ch[0], parent, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
+		if (kevent(kq, ch, 1, NULL, 0, NULL) == -1)
 			logerr("kevent1: %d", errno);
 	}
 
@@ -906,18 +948,23 @@ workerloop(int conffd, int ud, int ld, int ud6, int ld6)
 	if (af == -1)
 		logerr("parse error in config file");
 
-	EV_SET(&kev[0], ud, EVFILT_READ, EV_ADD, 0, 0, NULL);
-	EV_SET(&kev[1], ld, EVFILT_READ, EV_ADD, 0, 0, NULL);
-	EV_SET(&kev[2], ud6, EVFILT_READ, EV_ADD, 0, 0, NULL);
-	EV_SET(&kev[3], ld6, EVFILT_READ, EV_ADD, 0, 0, NULL);
-	if (kevent(kq, kev, 4, NULL, 0, NULL) == -1)
-		logerr("kevent4: %d", errno);
-	EV_SET(&kev[0], SIGHUP, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
-	EV_SET(&kev[1], SIGUSR1, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
-	if (kevent(kq, kev, 2, NULL, 0, NULL) == -1)
-		logerr("kevent2: %d", errno);
+	for (i = 0; i < numudp; i++) {
+		EV_SET(&ch[0], udpfds[i], EVFILT_READ, EV_ADD, 0, 0, NULL);
+		if (kevent(kq, ch, 1, NULL, 0, NULL) == -1)
+			logerr("udp kevent: %d", errno);
+	}
+	for (i = 0; i < numtcp; i++) {
+		EV_SET(&ch[0], tcpfds[i], EVFILT_READ, EV_ADD, 0, 0, NULL);
+		if (kevent(kq, ch, 1, NULL, 0, NULL) == -1)
+			logerr("tcp kevent: %d", errno);
+	}
+	EV_SET(&ch[0], SIGHUP, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
+	EV_SET(&ch[1], SIGUSR1, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
+	if (kevent(kq, ch, 2, NULL, 0, NULL) == -1)
+		logerr("sig kevent: %d", errno);
 	logmsg(LOG_INFO, "worker process going to work");
 	while (1) {
+		struct kevent kev[4];
 		r = kevent(kq, NULL, 0, kev, 4, timeout);
 		if (r == -1)
 			logerr("kevent failed (%d)", errno);
@@ -925,8 +972,11 @@ workerloop(int conffd, int ud, int ld, int ud6, int ld6)
 		clock_gettime(CLOCK_MONOTONIC, &now);
 
 		if (stopaccepting) {
-			EV_SET(&ch[0], ld, EVFILT_READ, EV_ADD, 0, 0, NULL);
-			kevent(kq, ch, 1, NULL, 0, NULL);
+			for (i = 0; i < numtcp; i++) {
+				EV_SET(&ch[0], tcpfds[i], EVFILT_READ,
+				    EV_ADD, 0, 0, NULL);
+				kevent(kq, ch, 1, NULL, 0, NULL);
+			}
 			stopaccepting = 0;
 		}
 
@@ -962,25 +1012,8 @@ workerloop(int conffd, int ud, int ld, int ud6, int ld6)
 				}
 				break;
 			case EVFILT_READ:
-				if (ke->ident == ud || ke->ident == ud6) {
-					if ((req = newrequest(ke->ident, &remoteaddr.a))) {
-						EV_SET(&ch[0], req->s, EVFILT_READ,
-						    EV_ADD, 0, 0, req);
-						kevent(kq, ch, 1, NULL, 0, NULL);
-					}
-				} else if (ke->ident == ld || ke->ident == ld6) {
-					if ((req = newtcprequest(ke->ident, &remoteaddr.a))) {
-						EV_SET(&ch[0], req->s,
-						    req->tcp == 1 ? EVFILT_WRITE :
-						    EVFILT_READ, EV_ADD, 0, 0, req);
-						kevent(kq, ch, 1, NULL, 0, NULL);
-					}
-				} else {
-					req = ke->udata;
-					if (req->tcp == 0)
-						handlereply(req);
-					freerequest(req);
-				}
+				readevent(kq, ke, &remoteaddr.a, udpfds,
+				    numudp, tcpfds, numtcp);
 				break;
 			default:
 				logerr("don't know what happened");
@@ -991,8 +1024,11 @@ workerloop(int conffd, int ud, int ld, int ud6, int ld6)
 		timeout = NULL;
 
 		if (stopaccepting) {
-			EV_SET(&ch[0], ld, EVFILT_READ, EV_DELETE, 0, 0, NULL);
-			kevent(kq, ch, 1, NULL, 0, NULL);
+			for (i = 0; i < numtcp; i++) {
+				EV_SET(&ch[0], tcpfds[i], EVFILT_READ,
+				    EV_DELETE, 0, 0, NULL);
+				kevent(kq, ch, 1, NULL, 0, NULL);
+			}
 			memset(&ts, 0, sizeof(ts));
 			/* one second added below */
 			timeout = &ts;
@@ -1053,28 +1089,49 @@ openconfig(const char *confname, int kq)
 }
 
 static pid_t
-reexec(int conffd, int ud, int ld, int ud6, int ld6)
+reexec(int conffd, int *udpfds, int numudp, int *tcpfds, int numtcp)
 {
 	pid_t child;
+	char **argv;
+	int argc;
+	int i;
 
-	if (conffd != 8 || ud != 3 || ld != 4 || ud6 != 5 || ld6 != 6)
-		logerr("can't re-exec, fds are wrong");
-
-	switch ((child = fork())) {
-	case -1:
+	if ((child = fork()) == -1)
 		logerr("failed to fork");
-		break;
-	case 0:
-		execl("/usr/sbin/rebound", "rebound", "-W", NULL);
-		logerr("re-exec failed");
-	default:
-		break;
+	if (child != 0)
+		return child;
+
+	/*   = rebound -W conffd numudp [udpfds] numtcp [tcpfds] NULL */
+	argc = 1       +1 +1     +1     +numudp  +1     +numtcp  +1;
+	argv = reallocarray(NULL, argc, sizeof(char *));
+	if (!argv)
+		logerr("out of memory building argv");
+	argc = 0;
+	argv[argc++] = "rebound";
+	argv[argc++] = "-W";
+	if (asprintf(&argv[argc++], "%d", conffd) == -1)
+		logerr("out of memory building argv");
+	if (asprintf(&argv[argc++], "%d", numudp) == -1)
+		logerr("out of memory building argv");
+	for (i = 0; i < numudp; i++) {
+		if (asprintf(&argv[argc++], "%d", udpfds[i]) == -1)
+			logerr("out of memory building argv");
+	}
+	if (asprintf(&argv[argc++], "%d", numtcp) == -1)
+		logerr("out of memory building argv");
+	for (i = 0; i < numtcp; i++) {
+		if (asprintf(&argv[argc++], "%d", tcpfds[i]) == -1)
+			logerr("out of memory building argv");
 	}
-	return child;
+	argv[argc++] = NULL;
+
+	execv("/usr/sbin/rebound", argv);
+	logerr("re-exec failed");
 }
 
 static int
-monitorloop(int ud, int ld, int ud6, int ld6, const char *confname)
+monitorloop(int *udpfds, int numudp, int *tcpfds, int numtcp,
+    const char *confname)
 {
 	pid_t child;
 	struct kevent kev;
@@ -1107,7 +1164,7 @@ monitorloop(int ud, int ld, int ud6, int ld6, const char *confname)
 		if (conffd == -1)
 			conffd = openconfig(confname, kq);
 
-		child = reexec(conffd, ud, ld, ud6, ld6);
+		child = reexec(conffd, udpfds, numudp, tcpfds, numtcp);
 
 		/* monitor child */
 		EV_SET(&kev, child, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL);
@@ -1133,7 +1190,8 @@ monitorloop(int ud, int ld, int ud6, int ld6, const char *confname)
 			case EVFILT_SIGNAL:
 				if (kev.ident == SIGHUP) {
 					/* signaled. kill child. */
-					logmsg(LOG_INFO, "received HUP, restarting");
+					logmsg(LOG_INFO,
+					    "received HUP, restarting");
 					close(conffd);
 					conffd = -1;
 					hupped = 1;
@@ -1142,7 +1200,8 @@ monitorloop(int ud, int ld, int ud6, int ld6, const char *confname)
 					kill(child, SIGHUP);
 				} else if (kev.ident == SIGTERM) {
 					/* good bye */
-					logmsg(LOG_INFO, "received TERM, quitting");
+					logmsg(LOG_INFO,
+					    "received TERM, quitting");
 					kill(child, SIGTERM);
 					exit(0);
 				}
@@ -1171,6 +1230,35 @@ doublebreak:
 	return 1;
 }
 
+static void
+addfd(int fd, int **fdp, int *numfds)
+{
+	int *fds = *fdp;
+
+	fds = reallocarray(fds, *numfds + 1, sizeof(int));
+	if (fds == NULL)
+		logerr("failed to allocate port array");
+	fds[*numfds] = fd;
+	*numfds += 1;
+	*fdp = fds;
+}
+
+static int
+argvtofd(char ***argv)
+{
+	const char *errstr;
+	int n;
+
+	if (**argv == NULL)
+		logerr("missing argument in argv");
+	n = strtonum(**argv, 0, 512, &errstr);
+	if (errstr)
+		logerr("invalid fd in argv");
+	*argv += 1;
+	return n;
+}
+
+
 static void __dead
 usage(void)
 {
@@ -1182,8 +1270,13 @@ int
 main(int argc, char **argv)
 {
 	union sockun bindaddr;
-	int ld, ld6, ud, ud6, ch;
+	int *udpfds = NULL;
+	int numudp = 0;
+	int *tcpfds = NULL;
+	int numtcp = 0;
+	int ch, fd, i;
 	int one = 1;
+	int worker = 0;
 	const char *confname = "/etc/resolv.conf";
 	const char *bindname = "127.0.0.1";
 
@@ -1193,8 +1286,6 @@ main(int argc, char **argv)
 	signal(SIGPIPE, SIG_IGN);
 	signal(SIGUSR1, SIG_IGN);
 
-	https_init();
-
 	while ((ch = getopt(argc, argv, "c:dl:W")) != -1) {
 		switch (ch) {
 		case 'c':
@@ -1208,8 +1299,8 @@ main(int argc, char **argv)
 			break;
 		case 'W':
 			daemonized = 1;
-			/* parent responsible for setting up fds */
-			return workerloop(8, 3, 4, 5, 6);
+			worker = 1;
+			break;
 		default:
 			usage();
 			break;
@@ -1218,10 +1309,35 @@ main(int argc, char **argv)
 	argv += optind;
 	argc -= optind;
 
+	if (worker) {
+		int numfds;
+		int conffd;
+
+		/*
+		 * at this point we have a bunch of fds in argv
+		 * conffd numudp [udpfds] numtcp [tcpfds]
+		 */
+		conffd = argvtofd(&argv);
+		
+		numfds = argvtofd(&argv);
+		for (i = 0; i < numfds; i++) {
+			fd = argvtofd(&argv);
+			addfd(fd, &udpfds, &numudp);
+		}
+
+		numfds = argvtofd(&argv);
+		for (i = 0; i < numfds; i++) {
+			fd = argvtofd(&argv);
+			addfd(fd, &tcpfds, &numtcp);
+		}
+		if (*argv != NULL)
+			logerr("extraneous arguments for worker");
+		return workerloop(conffd, udpfds, numudp, tcpfds, numtcp);
+	}
+
 	if (argc)
 		usage();
 
-	/* make sure we consistently open fds */
 	closefrom(3);
 
 	memset(&bindaddr, 0, sizeof(bindaddr));
@@ -1230,20 +1346,22 @@ main(int argc, char **argv)
 	bindaddr.i.sin_port = htons(53);
 	inet_aton(bindname, &bindaddr.i.sin_addr);
 
-	ud = socket(AF_INET, SOCK_DGRAM, 0);
-	if (ud == -1)
+	fd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (fd == -1)
 		logerr("socket: %s", strerror(errno));
-	if (bind(ud, &bindaddr.a, bindaddr.a.sa_len) == -1)
+	if (bind(fd, &bindaddr.a, bindaddr.a.sa_len) == -1)
 		logerr("bind: %s", strerror(errno));
+	addfd(fd, &udpfds, &numudp);
 
-	ld = socket(AF_INET, SOCK_STREAM, 0);
-	if (ld == -1)
+	fd = socket(AF_INET, SOCK_STREAM, 0);
+	if (fd == -1)
 		logerr("socket: %s", strerror(errno));
-	setsockopt(ld, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
-	if (bind(ld, &bindaddr.a, bindaddr.a.sa_len) == -1)
+	setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
+	if (bind(fd, &bindaddr.a, bindaddr.a.sa_len) == -1)
 		logerr("bind: %s", strerror(errno));
-	if (listen(ld, 10) == -1)
+	if (listen(fd, 10) == -1)
 		logerr("listen: %s", strerror(errno));
+	addfd(fd, &tcpfds, &numtcp);
 
 	memset(&bindaddr, 0, sizeof(bindaddr));
 	bindaddr.i6.sin6_len = sizeof(bindaddr.i6);
@@ -1251,29 +1369,31 @@ main(int argc, char **argv)
 	bindaddr.i6.sin6_port = htons(53);
 	bindaddr.i6.sin6_addr = in6addr_loopback;
 
-	ud6 = socket(AF_INET6, SOCK_DGRAM, 0);
-	if (ud6 == -1)
+	fd = socket(AF_INET6, SOCK_DGRAM, 0);
+	if (fd == -1)
 		logerr("socket: %s", strerror(errno));
-	if (bind(ud6, &bindaddr.a, bindaddr.a.sa_len) == -1)
+	if (bind(fd, &bindaddr.a, bindaddr.a.sa_len) == -1)
 		logerr("bind: %s", strerror(errno));
+	addfd(fd, &udpfds, &numudp);
 
-	ld6 = socket(AF_INET6, SOCK_STREAM, 0);
-	if (ld6 == -1)
+	fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (fd == -1)
 		logerr("socket: %s", strerror(errno));
-	setsockopt(ld6, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
-	if (bind(ld6, &bindaddr.a, bindaddr.a.sa_len) == -1)
+	setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
+	if (bind(fd, &bindaddr.a, bindaddr.a.sa_len) == -1)
 		logerr("bind: %s", strerror(errno));
-	if (listen(ld6, 10) == -1)
+	if (listen(fd, 10) == -1)
 		logerr("listen: %s", strerror(errno));
+	addfd(fd, &tcpfds, &numtcp);
 
 	if (debug) {
 		int conffd = openconfig(confname, -1);
-		return workerloop(conffd, ud, ld, ud6, ld6);
+		return workerloop(conffd, udpfds, numudp, tcpfds, numtcp);
 	}
 
 	if (daemon(0, 0) == -1)
 		logerr("daemon: %s", strerror(errno));
 	daemonized = 1;
 
-	return monitorloop(ud, ld, ud6, ld6, confname);
+	return monitorloop(udpfds, numudp, tcpfds, numtcp, confname);
 }