ttymsg: Overhaul

* Instead of an error string, return the usual 0 or -1 and let the caller
  figure out what, if anything, to tell the user.

* Avoid string manipulations by opening /dev first and using openat()
  with O_RESOLVE_BENEATH.

* Add a boolean argument which, if false, causes ttymsg() to return
  without sending the message if the tty's group-writable bit is not
  set.  This saves programs that respect this setting (like syslogd(8))
  from having to check before calling ttymsg().

* Update all callers.

The observable effect of this change is minimal except for slightly
different error messages when ttymsg() fails.  However, syslogd(8) will
no longer print spurious error messages on the console after trying and
failing to write a log message to an X11 session.

PR:		295171
MFC after:	1 week
Reviewed by:	jfree, markj
Differential Revision:	https://reviews.freebsd.org/D57018

libexec/talkd/announce.c

@@ -40,6 +40,7 @@
 
 #include <errno.h>
 #include <paths.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -152,7 +153,7 @@ print_mesg(const char *tty, CTL_MSG *request,
 	 * stack up processes trying to write messages to a tty
 	 * that is permanently blocked.
 	 */
-	if (ttymsg(&iovec, 1, tty, RING_WAIT - 5) != NULL)
+	if (ttymsg(&iovec, 1, tty, RING_WAIT - 5, true) != 0)
 		return (FAILED);
 
 	return (SUCCESS);

usr.bin/wall/ttymsg.c

@@ -29,15 +29,16 @@
  * SUCH DAMAGE.
  */
 
-
-
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <sys/uio.h>
+
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <paths.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -46,65 +47,72 @@
 #include "ttymsg.h"
 
 /*
- * Display the contents of a uio structure on a terminal.  Used by wall(1),
- * syslogd(8), and talkd(8).  Forks and finishes in child if write would block,
- * waiting up to tmout 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.).
+ * Display the contents of a uio structure on a terminal.  If shout is
+ * non-zero, do so even if the terminal has messages disabled.  Used by
+ * wall(1), syslogd(8), and talkd(8).  Forks and finishes in child if
+ * write would block, waiting up to timeout seconds.  Various "normal"
+ * errors are ignored (exclusive-use, lack of permission, etc.).
  */
-const char *
-ttymsg(struct iovec *iov, int iovcnt, const char *line, int tmout)
+int
+ttymsg(struct iovec *iov, int iovcnt, const char *tty, int timeout,
+    bool shout)
 {
 	struct iovec localiov[TTYMSG_IOV_MAX];
-	ssize_t left, wret;
-	int cnt, fd;
-	char device[MAXNAMLEN] = _PATH_DEV;
-	static char errbuf[1024];
-	char *p;
+	struct stat sb;
+	ssize_t wret;
+	size_t resid;
+	int cnt, dd, fd, serrno;
 	int forked;
 
 	forked = 0;
-	if (iovcnt > (int)(sizeof(localiov) / sizeof(localiov[0])))
-		return ("too many iov's (change code in wall/ttymsg.c)");
-
-	strlcat(device, line, sizeof(device));
-	p = device + sizeof(_PATH_DEV) - 1;
-	if (strncmp(p, "pts/", 4) == 0)
-		p += 4;
-	if (strchr(p, '/') != NULL) {
-		/* A slash is an attempt to break security... */
-		(void) snprintf(errbuf, sizeof(errbuf),
-		    "Too many '/' in \"%s\"", device);
-		return (errbuf);
+	if (iovcnt > (int)(sizeof(localiov) / sizeof(localiov[0]))) {
+		errno = EFBIG;
+		return (-1);
 	}
 
-	/*
-	 * open will fail on slip lines or exclusive-use lines
-	 * if not running as root; not an error.
-	 */
-	if ((fd = open(device, O_WRONLY|O_NONBLOCK, 0)) < 0) {
-		if (errno == EBUSY || errno == EACCES)
-			return (NULL);
-		(void) snprintf(errbuf, sizeof(errbuf), "%s: %s", device,
-		    strerror(errno));
-		return (errbuf);
+	dd = open(_PATH_DEV, O_SEARCH | O_DIRECTORY);
+	if (dd < 0)
+		return (-1);
+	fd = openat(dd, tty, O_WRONLY | O_NONBLOCK | O_RESOLVE_BENEATH);
+	if (fd < 0) {
+		serrno = errno;
+		close(dd);
+		/*
+		 * open will fail on slip lines or exclusive-use lines
+		 * if not running as root; not an error.
+		 */
+		if (serrno == EBUSY || serrno == EACCES)
+			return (0);
+		errno = serrno;
+		return (-1);
+	}
+	close(dd);
+	if (!shout) {
+		if (fstat(fd, &sb) != 0) {
+			serrno = errno;
+			close(fd);
+			errno = serrno;
+			return (-1);
+		}
+		if ((sb.st_mode & S_IWGRP) == 0) {
+			close(fd);
+			return (0);
+		}
 	}
 
-	for (cnt = 0, left = 0; cnt < iovcnt; ++cnt)
-		left += iov[cnt].iov_len;
+	for (cnt = 0, resid = 0; cnt < iovcnt; ++cnt)
+		resid += iov[cnt].iov_len;
 
-	for (;;) {
+	do {
 		wret = writev(fd, iov, iovcnt);
-		if (wret >= left)
-			break;
 		if (wret >= 0) {
-			left -= wret;
+			resid -= wret;
 			if (iov != localiov) {
-				bcopy(iov, localiov, 
+				bcopy(iov, localiov,
 				    iovcnt * sizeof(struct iovec));
 				iov = localiov;
 			}
-			for (cnt = 0; (size_t)wret >= iov->iov_len; ++cnt) {
+			while ((size_t)wret >= iov->iov_len) {
 				wret -= iov->iov_len;
 				++iov;
 				--iovcnt;
@@ -124,21 +132,21 @@ ttymsg(struct iovec *iov, int iovcnt, const char *line, int tmout)
 			}
 			cpid = fork();
 			if (cpid < 0) {
-				(void) snprintf(errbuf, sizeof(errbuf),
-				    "fork: %s", strerror(errno));
+				serrno = errno;
 				(void) close(fd);
-				return (errbuf);
+				errno = serrno;
+				return (-1);
 			}
 			if (cpid) {	/* parent */
 				(void) close(fd);
-				return (NULL);
+				return (0);
 			}
 			forked++;
-			/* wait at most tmout seconds */
+			/* wait at most timeout seconds */
 			(void) signal(SIGALRM, SIG_DFL);
 			(void) signal(SIGTERM, SIG_DFL); /* XXX */
 			(void) sigsetmask(0);
-			(void) alarm((u_int)tmout);
+			(void) alarm((u_int)timeout);
 			(void) fcntl(fd, F_SETFL, 0);	/* clear O_NONBLOCK */
 			continue;
 		}
@@ -146,18 +154,18 @@ ttymsg(struct iovec *iov, int iovcnt, const char *line, int tmout)
 		 * We get ENODEV on a slip line if we're running as root,
 		 * and EIO if the line just went away.
 		 */
-		if (errno == ENODEV || errno == EIO)
+		serrno = errno;
+		if (serrno == ENODEV || serrno == EIO)
 			break;
 		(void) close(fd);
 		if (forked)
 			_exit(1);
-		(void) snprintf(errbuf, sizeof(errbuf),
-		    "%s: %s", device, strerror(errno));
-		return (errbuf);
-	}
+		errno = serrno;
+		return (-1);
+	} while (resid > 0);
 
 	(void) close(fd);
 	if (forked)
 		_exit(0);
-	return (NULL);
+	return (0);
 }

usr.bin/wall/ttymsg.h

@@ -1,4 +1,4 @@
 
 #define	TTYMSG_IOV_MAX	32
 
-const char	*ttymsg(struct iovec *, int, const char *, int);
+int ttymsg(struct iovec *, int, const char *, int, bool);

usr.bin/wall/wall.c

@@ -44,6 +44,7 @@
 #include <locale.h>
 #include <paths.h>
 #include <pwd.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -90,7 +91,6 @@ main(int argc, char *argv[])
 	struct wallgroup *g;
 	struct group *grp;
 	char **np;
-	const char *p;
 	struct passwd *pw;
 
 	(void)setlocale(LC_CTYPE, "");
@@ -158,8 +158,8 @@ main(int argc, char *argv[])
 			if (ingroup == 0)
 				continue;
 		}
-		if ((p = ttymsg(&iov, 1, utmp->ut_line, 60*5)) != NULL)
-			warnx("%s", p);
+		if (ttymsg(&iov, 1, utmp->ut_line, 60 * 5, 1) != 0)
+			warn("%s", utmp->ut_line);
 	}
 	exit(0);
 }

usr.sbin/syslogd/syslogd.c

@@ -360,7 +360,6 @@ static int	evaluate_prop_filter(const struct prop_filter *filter,
 static nvlist_t *prop_filter_compile(const char *);
 static void	parsemsg(const char *, char *);
 static void	printsys(char *);
-static const char *ttymsg_check(struct iovec *, int, char *, int);
 static void	usage(void);
 static bool	validate(struct sockaddr *, const char *);
 static void	unmapped(struct sockaddr *);
@@ -1756,8 +1755,6 @@ iovlist_truncate(struct iovlist *il, size_t size)
 static void
 fprintlog_write(struct filed *f, struct iovlist *il, int flags)
 {
-	const char *msgret;
-
 	switch (f->f_type) {
 	case F_FORW: {
 		ssize_t lsent;
@@ -1910,10 +1907,10 @@ fprintlog_write(struct filed *f, struct iovlist *il, int flags)
 		dprintf(" %s%s\n", _PATH_DEV, f->f_fname);
 		iovlist_append(il, "\r\n");
 		errno = 0;	/* ttymsg() only sometimes returns an errno */
-		if ((msgret = cap_ttymsg(cap_syslogd, il->iov, il->iovcnt,
-		    f->f_fname, 10))) {
+		if (cap_ttymsg(cap_syslogd, il->iov, il->iovcnt, f->f_fname, 10,
+		    true) != 0) {
 			f->f_type = F_UNUSED;
-			logerror(msgret);
+			logerror(f->f_fname);
 		}
 		break;
 
@@ -2143,7 +2140,6 @@ wallmsg(const struct filed *f, struct iovec *iov, const int iovlen)
 	static int reenter;			/* avoid calling ourselves */
 	struct utmpx *ut;
 	int i;
-	const char *p;
 
 	if (reenter++)
 		return;
@@ -2153,19 +2149,19 @@ wallmsg(const struct filed *f, struct iovec *iov, const int iovlen)
 		if (ut->ut_type != USER_PROCESS)
 			continue;
 		if (f->f_type == F_WALL) {
-			if ((p = ttymsg(iov, iovlen, ut->ut_line,
-			    TTYMSGTIME)) != NULL)
-				dprintf("%s\n", p);
+			if (ttymsg(iov, iovlen, ut->ut_line, TTYMSGTIME,
+			    false) != 0 && errno != ENOENT)
+				dprintf("%s: %m\n", ut->ut_line);
 			continue;
 		}
 		/* should we send the message to this user? */
 		for (i = 0; i < MAXUNAMES; i++) {
 			if (!f->f_uname[i][0])
 				break;
-			if (!strcmp(f->f_uname[i], ut->ut_user)) {
-				if ((p = ttymsg_check(iov, iovlen, ut->ut_line,
-				    TTYMSGTIME)) != NULL)
-					dprintf("%s\n", p);
+			if (strcmp(f->f_uname[i], ut->ut_user) == 0) {
+				if (ttymsg(iov, iovlen, ut->ut_line, TTYMSGTIME,
+				    true) != 0 && errno != ENOENT)
+					dprintf("%s: %m\n", ut->ut_line);
 				break;
 			}
 		}
@@ -2174,29 +2170,6 @@ wallmsg(const struct filed *f, struct iovec *iov, const int iovlen)
 	reenter = 0;
 }
 
-/*
- * Wrapper routine for ttymsg() that checks the terminal for messages enabled.
- */
-static const char *
-ttymsg_check(struct iovec *iov, int iovcnt, char *line, int tmout)
-{
-	static char device[1024];
-	static char errbuf[1024];
-	struct stat sb;
-
-	(void) snprintf(device, sizeof(device), "%s%s", _PATH_DEV, line);
-
-	if (stat(device, &sb) < 0) {
-		(void) snprintf(errbuf, sizeof(errbuf),
-		    "%s: %s", device, strerror(errno));
-		return (errbuf);
-	}
-	if ((sb.st_mode & S_IWGRP) == 0)
-		/* Messages disabled. */
-		return (NULL);
-	return (ttymsg(iov, iovcnt, line, tmout));
-}
-
 /*
  * Return a printable representation of a host address.
  */

usr.sbin/syslogd/syslogd_cap.c

@@ -50,7 +50,7 @@ casper_command(const char *cmd, const nvlist_t *limits __unused,
 	else if (strcmp(cmd, "readconfigfile") == 0)
 		error = casper_readconfigfile(nvlin, nvlout);
 	else if (strcmp(cmd, "ttymsg") == 0)
-		error = casper_ttymsg(nvlin, nvlout);
+		error = casper_ttymsg(nvlin);
 	else if (strcmp(cmd, "wallmsg") == 0)
 		error = casper_wallmsg(nvlin);
 

usr.sbin/syslogd/syslogd_cap.h

@@ -62,13 +62,13 @@ extern SLIST_HEAD(cfiled_list, cap_filed) cfiled_head;
 
 int cap_p_open(cap_channel_t *, size_t, const char *, int *);
 nvlist_t *cap_readconfigfile(cap_channel_t *, const char *);
-const char *cap_ttymsg(cap_channel_t *, struct iovec *, int, const char *, int);
+int cap_ttymsg(cap_channel_t *, struct iovec *, int, const char *, int, bool);
 void cap_wallmsg(cap_channel_t *, const struct filed *, struct iovec *,
     const int);
 
 int casper_p_open(nvlist_t *, nvlist_t *);
 int casper_readconfigfile(nvlist_t *, nvlist_t *);
-int casper_ttymsg(nvlist_t *, nvlist_t *);
+int casper_ttymsg(nvlist_t *);
 int casper_wallmsg(nvlist_t *);
 
 nvlist_t *filed_to_nvlist(const struct filed *);
@@ -83,8 +83,8 @@ struct prop_filter *nvlist_to_prop_filter(const nvlist_t *nvl_prop_filter);
 	p_open(prog, rpd)
 #define	cap_readconfigfile(chan, cf) \
 	readconfigfile(cf)
-#define	cap_ttymsg(chan, iov, iovcnt, line, tmout) \
-	ttymsg(iov, iovcnt, line, tmout)
+#define	cap_ttymsg(chan, iov, iovcnt, line, timeout) \
+	ttymsg(iov, iovcnt, line, timeout)
 #define	cap_wallmsg(chan, f, iov, iovcnt) \
 	wallmsg(f, iov, iovcnt)
 

usr.sbin/syslogd/syslogd_cap_log.c

@@ -52,14 +52,12 @@ cap_p_open(cap_channel_t *chan, size_t filed_idx, const char *prog,
 		exit(1);
 	}
 	error = nvlist_get_number(nvl, "error");
-	if (error != 0) {
-		errno = error;
-		logerror("Failed to open piped command");
-	}
 	pipedesc_w = dnvlist_take_descriptor(nvl, "pipedesc_w", -1);
 	*procdesc = dnvlist_take_descriptor(nvl, "procdesc", -1);
 
 	nvlist_destroy(nvl);
+	if (error != 0)
+		errno = error;
 	return (pipedesc_w);
 }
 
@@ -81,29 +79,27 @@ casper_p_open(nvlist_t *nvlin, nvlist_t *nvlout)
 
 		pipedesc_w = p_open(prog, &procdesc);
 		if (pipedesc_w == -1)
-			return (-1);
+			return (errno);
 		nvlist_move_descriptor(nvlout, "pipedesc_w", pipedesc_w);
 		nvlist_move_descriptor(nvlout, "procdesc", procdesc);
 		return (0);
 	}
-
-	return (-1);
+	return (ECAPMODE);
 }
 
-const char *
+int
 cap_ttymsg(cap_channel_t *chan, struct iovec *iov, int iovcnt,
-    const char *line, int tmout)
+    const char *line, int timeout, bool shout)
 {
 	nvlist_t *nvl = nvlist_create(0);
-	int error;
-	static char errbuf[1024];
-	char *ret = NULL;
+	int error = 0;
 
 	nvlist_add_string(nvl, "cmd", "ttymsg");
 	for (int i = 0; i < iovcnt; ++i)
 		nvlist_append_string_array(nvl, "iov_strs", iov[i].iov_base);
 	nvlist_add_string(nvl, "line", line);
-	nvlist_add_number(nvl, "tmout", tmout);
+	nvlist_add_number(nvl, "timeout", timeout);
+	nvlist_add_bool(nvl, "shout", shout);
 
 	nvl = cap_xfer_nvlist(chan, nvl);
 	if (nvl == NULL) {
@@ -111,28 +107,23 @@ cap_ttymsg(cap_channel_t *chan, struct iovec *iov, int iovcnt,
 		exit(1);
 	}
 	error = nvlist_get_number(nvl, "error");
+	nvlist_destroy(nvl);
 	if (error != 0) {
 		errno = error;
-		logerror("Failed to ttymsg");
+		return (-1);
 	}
-	if (nvlist_exists_string(nvl, "errstr")) {
-		const char *errstr = nvlist_get_string(nvl, "errstr");
-		(void)strlcpy(errbuf, errstr, sizeof(errbuf));
-		ret = errbuf;
-	}
-
-	nvlist_destroy(nvl);
-	return (ret);
+	return (0);
 }
 
 int
-casper_ttymsg(nvlist_t *nvlin, nvlist_t *nvlout)
+casper_ttymsg(nvlist_t *nvlin)
 {
 	const char * const *nvlstrs;
 	struct iovec *iov;
-	size_t iovcnt;
-	int tmout;
 	const char *line;
+	size_t iovcnt;
+	int ret, timeout;
+	bool shout;
 
 	nvlstrs = nvlist_get_string_array(nvlin, "iov_strs", &iovcnt);
 	assert(iovcnt <= TTYMSG_IOV_MAX);
@@ -144,13 +135,12 @@ casper_ttymsg(nvlist_t *nvlin, nvlist_t *nvlout)
 		iov[i].iov_len = strlen(nvlstrs[i]);
 	}
 	line = nvlist_get_string(nvlin, "line");
-	tmout = nvlist_get_number(nvlin, "tmout");
-	line = ttymsg(iov, iovcnt, line, tmout);
-	if (line != NULL)
-		nvlist_add_string(nvlout, "errstr", line);
-
+	timeout = nvlist_get_number(nvlin, "timeout");
+	shout = nvlist_get_bool(nvlin, "shout");
+	if ((ret = ttymsg(iov, iovcnt, line, timeout, shout)) != 0)
+		ret = errno;
 	free(iov);
-	return (0);
+	return (ret);
 }
 
 void
@@ -158,7 +148,6 @@ cap_wallmsg(cap_channel_t *chan, const struct filed *f, struct iovec *iov,
     int iovcnt)
 {
 	nvlist_t *nvl = nvlist_create(0);
-	int error;
 
 	nvlist_add_string(nvl, "cmd", "wallmsg");
 	/*
@@ -175,11 +164,6 @@ cap_wallmsg(cap_channel_t *chan, const struct filed *f, struct iovec *iov,
 		logerror("Failed to xfer wallmsg nvlist");
 		exit(1);
 	}
-	error = nvlist_get_number(nvl, "error");
-	if (error != 0) {
-		errno = error;
-		logerror("Failed to wallmsg");
-	}
 	nvlist_destroy(nvl);
 }