From 959a7adfa6345f0d4407e4a4919afdaabb76d05f Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 23 Oct 2015 16:28:52 +0000 Subject: [PATCH] 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@ --- usr.sbin/syslogd/syslogd.c | 8 ++- usr.sbin/syslogd/syslogd.h | 4 +- usr.sbin/syslogd/ttymsg.c | 101 ++++++++++++++++++++++++++----------- 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 @@ -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 #include +#include #include #include #include @@ -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); +} -- 2.20.1