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@

diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
index 4bde888..e980620 100644 (file)
--- 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 bf90882..25c8f02 100644 (file)
--- 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 a18ef8f..ef50b39 100644 (file)
--- 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);
+}