If writing to a tty blocks, syslogd forked and tried to write again

in a background process.  A potential fork(2) at every message is
bad, so replace this with an event.  As a bonus the syslogd child
process does not need to pledge "proc" anymore.  Also limit the
number of delayed write events.
OK deraadt@

3 files changed, 78 insertions, 35 deletions

diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
index 4bde888d37a..e9806203210 100644
--- a/usr.sbin/syslogd/syslogd.c
+++ b/usr.sbin/syslogd/syslogd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: syslogd.c,v 1.199 2015/10/21 14:03:07 bluhm Exp $	*/
+/*	$OpenBSD: syslogd.c,v 1.200 2015/10/23 16:28:52 bluhm Exp $	*/
 
 /*
  * Copyright (c) 1983, 1988, 1993, 1994
@@ -53,7 +53,6 @@
  * IPv6, libevent, syslog over TCP and TLS by Alexander Bluhm
  */
 
-#define MAXLINE		8192		/* maximum line length */
 #define MAX_UDPMSG	1180		/* maximum UDP send size */
 #define MIN_MEMBUF	(MAXLINE * 4)	/* Minimum memory buffer size */
 #define MAX_MEMBUF	(256 * 1024)	/* Maximum memory buffer size */
@@ -701,7 +700,7 @@ main(int argc, char *argv[])
 	if (priv_init(ConfFile, NoDNS, lockpipe[1], nullfd, argv) < 0)
 		errx(1, "unable to privsep");
 
-	if (pledge("stdio rpath unix inet proc recvfd", NULL) == -1)
+	if (pledge("stdio rpath unix inet recvfd", NULL) == -1)
 		err(1, "pledge");
 
 	/* Process is now unprivileged and inside a chroot */
@@ -1952,8 +1951,7 @@ wallmsg(struct filed *f, struct iovec *iov)
 				break;
 			if (!strncmp(f->f_un.f_uname[i], ut.ut_name,
 			    UT_NAMESIZE)) {
-				if ((p = ttymsg(iov, 6, utline))
-				    != NULL)
+				if ((p = ttymsg(iov, 6, utline)) != NULL)
 					logerrorx(p);
 				break;
 			}
diff --git a/usr.sbin/syslogd/syslogd.h b/usr.sbin/syslogd/syslogd.h
index bf908827ae7..25c8f026b29 100644
--- a/usr.sbin/syslogd/syslogd.h
+++ b/usr.sbin/syslogd/syslogd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: syslogd.h,v 1.22 2015/10/21 14:03:07 bluhm Exp $ */
+/*	$OpenBSD: syslogd.h,v 1.23 2015/10/23 16:28:52 bluhm Exp $ */
 
 /*
  * Copyright (c) 2003 Anil Madhavapeddy <anil@recoil.org>
@@ -33,6 +33,7 @@ int   priv_getnameinfo(struct sockaddr *, socklen_t, char *, size_t);
 
 /* Terminal message */
 #define TTYMSGTIME	1		/* timeout used by ttymsg */
+#define TTYMAXDELAY	256		/* max events in ttymsg */
 char *ttymsg(struct iovec *, int, char *);
 
 /* File descriptor send/recv */
@@ -47,6 +48,7 @@ extern char *path_ctlsock;
 extern int fd_ctlsock, fd_ctlconn, fd_klog, fd_sendsys;
 extern int fd_udp, fd_udp6, fd_bind, fd_listen, fd_tls, fd_unix[MAXUNIX];
 
+#define MAXLINE		8192		/* maximum line length */
 #define ERRBUFSIZE	256
 void logdebug(const char *, ...) __attribute__((__format__ (printf, 1, 2)));
 extern int Debug;
diff --git a/usr.sbin/syslogd/ttymsg.c b/usr.sbin/syslogd/ttymsg.c
index a18ef8fa3d7..ef50b3927e2 100644
--- a/usr.sbin/syslogd/ttymsg.c
+++ b/usr.sbin/syslogd/ttymsg.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ttymsg.c,v 1.8 2015/10/21 14:03:07 bluhm Exp $	*/
+/*	$OpenBSD: ttymsg.c,v 1.9 2015/10/23 16:28:52 bluhm Exp $	*/
 /*	$NetBSD: ttymsg.c,v 1.3 1994/11/17 07:17:55 jtc Exp $	*/
 
 /*
@@ -35,6 +35,7 @@
 
 #include <dirent.h>
 #include <errno.h>
+#include <event.h>
 #include <fcntl.h>
 #include <paths.h>
 #include <signal.h>
@@ -45,9 +46,17 @@
 
 #include "syslogd.h"
 
+struct tty_delay {
+	struct event	 td_event;
+	size_t		 td_length;
+	char		 td_line[MAXLINE];
+};
+int tty_delayed = 0;
+void ttycb(int, short, void *);
+
 /*
  * Display the contents of a uio structure on a terminal.
- * Forks and finishes in child if write would block, waiting up to TTYMSGTIME
+ * Schedules an event if write would block, waiting up to TTYMSGTIME
  * seconds.  Returns pointer to error string on unexpected error;
  * string is not newline-terminated.  Various "normal" errors are ignored
  * (exclusive-use, lack of permission, etc.).
@@ -61,8 +70,6 @@ ttymsg(struct iovec *iov, int iovcnt, char *utline)
 	size_t left;
 	ssize_t wret;
 	struct iovec localiov[6];
-	int forked = 0;
-	sigset_t mask;
 
 	if (iovcnt < 0 || (size_t)iovcnt > nitems(localiov))
 		return ("too many iov's (change code in syslogd/ttymsg.c)");
@@ -122,34 +129,44 @@ ttymsg(struct iovec *iov, int iovcnt, char *utline)
 			continue;
 		}
 		if (errno == EWOULDBLOCK) {
-			int off = 0;
-			pid_t cpid;
+			struct tty_delay	*td;
+			struct timeval		 to;
 
-			if (forked) {
-				(void) close(fd);
-				_exit(1);
+			if (tty_delayed >= TTYMAXDELAY) {
+				(void) snprintf(ebuf, sizeof(ebuf),
+				    "%s: too many delayed writes", device);
+				return (ebuf);
 			}
 			logdebug("ttymsg delayed write\n");
-			cpid = fork();
-			if (cpid < 0) {
+			if (iov != localiov) {
+				bcopy(iov, localiov,
+				    iovcnt * sizeof(struct iovec));
+				iov = localiov;
+			}
+			if ((td = malloc(sizeof(*td))) == NULL) {
 				(void) snprintf(ebuf, sizeof(ebuf),
-				    "fork: %s", strerror(errno));
-				(void) close(fd);
+				    "%s: malloc: %s", device, strerror(errno));
 				return (ebuf);
 			}
-			if (cpid) {	/* parent */
-				(void) close(fd);
-				return (NULL);
+			td->td_length = 0;
+			if (left > MAXLINE)
+				left = MAXLINE;
+			while (iovcnt && left) {
+				if (iov->iov_len > left)
+					iov->iov_len = left;
+				memcpy(td->td_line + td->td_length,
+				    iov->iov_base, iov->iov_len);
+				td->td_length += iov->iov_len;
+				left -= iov->iov_len;
+				++iov;
+				--iovcnt;
 			}
-			forked++;
-			/* wait at most TTYMSGTIME seconds */
-			(void) signal(SIGALRM, SIG_DFL);
-			(void) signal(SIGTERM, SIG_DFL); /* XXX */
-			(void) sigemptyset(&mask);
-			(void) sigprocmask(SIG_SETMASK, &mask, NULL);
-			(void) alarm((u_int)TTYMSGTIME);
-			(void) fcntl(fd, O_NONBLOCK, &off);
-			continue;
+			tty_delayed++;
+			event_set(&td->td_event, fd, EV_WRITE, ttycb, td);
+			to.tv_sec = TTYMSGTIME;
+			to.tv_usec = 0;
+			event_add(&td->td_event, &to);
+			return (NULL);
 		}
 		/*
 		 * We get ENODEV on a slip line if we're running as root,
@@ -158,15 +175,41 @@ ttymsg(struct iovec *iov, int iovcnt, char *utline)
 		if (errno == ENODEV || errno == EIO)
 			break;
 		(void) close(fd);
-		if (forked)
-			_exit(1);
 		(void) snprintf(ebuf, sizeof(ebuf),
 		    "%s: %s", device, strerror(errno));
 		return (ebuf);
 	}
 
 	(void) close(fd);
-	if (forked)
-		_exit(0);
 	return (NULL);
 }
+
+void
+ttycb(int fd, short event, void *arg)
+{
+	struct tty_delay	*td = arg;
+	struct timeval		 to;
+	ssize_t			 wret;
+
+	if (event != EV_WRITE)
+		goto done;
+
+	wret = write(fd, td->td_line, td->td_length);
+	if (wret < 0 && errno != EINTR && errno != EWOULDBLOCK)
+		goto done;
+	if (wret > 0) {
+		td->td_length -= wret;
+		if (td->td_length == 0)
+			goto done;
+		memmove(td->td_line, td->td_line + wret, td->td_length);
+	}
+	to.tv_sec = TTYMSGTIME;
+	to.tv_usec = 0;
+	event_add(&td->td_event, &to);
+	return;
+
+ done:
+	tty_delayed--;
+	close(fd);
+	free(td);
+}