Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 25 May 2026 17:03:02 +0000
From:      Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 05e8f2bf0906 - main - ttymsg: Overhaul
Message-ID:  <6a1480c6.37b13.546d11ae@gitrepo.freebsd.org>

index | next in thread | raw e-mail

The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=05e8f2bf0906875e666469e0338f922d1113d034

commit 05e8f2bf0906875e666469e0338f922d1113d034
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2026-05-25 16:51:23 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2026-05-25 16:52:15 +0000

    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           |   3 +-
 usr.bin/wall/ttymsg.c              | 120 ++++++++++++++++++++-----------------
 usr.bin/wall/ttymsg.h              |   2 +-
 usr.bin/wall/wall.c                |   6 +-
 usr.sbin/syslogd/syslogd.c         |  47 ++++-----------
 usr.sbin/syslogd/syslogd_cap.c     |   2 +-
 usr.sbin/syslogd/syslogd_cap.h     |   8 +--
 usr.sbin/syslogd/syslogd_cap_log.c |  58 +++++++-----------
 8 files changed, 106 insertions(+), 140 deletions(-)

diff --git a/libexec/talkd/announce.c b/libexec/talkd/announce.c
index 80c663ba48b4..cfe8b4eae65e 100644
--- a/libexec/talkd/announce.c
+++ b/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);
diff --git a/usr.bin/wall/ttymsg.c b/usr.bin/wall/ttymsg.c
index 4f1c367c505f..3eee72d6805b 100644
--- a/usr.bin/wall/ttymsg.c
+++ b/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);
 }
diff --git a/usr.bin/wall/ttymsg.h b/usr.bin/wall/ttymsg.h
index be97592f5e1c..840a49875fbb 100644
--- a/usr.bin/wall/ttymsg.h
+++ b/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);
diff --git a/usr.bin/wall/wall.c b/usr.bin/wall/wall.c
index fcfcdcb3fbce..e29b896f838f 100644
--- a/usr.bin/wall/wall.c
+++ b/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);
 }
diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
index 59cb56fd5970..70cb705c0cb7 100644
--- a/usr.sbin/syslogd/syslogd.c
+++ b/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.
  */
diff --git a/usr.sbin/syslogd/syslogd_cap.c b/usr.sbin/syslogd/syslogd_cap.c
index 7539e6b8661b..0149d09dab99 100644
--- a/usr.sbin/syslogd/syslogd_cap.c
+++ b/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);
 
diff --git a/usr.sbin/syslogd/syslogd_cap.h b/usr.sbin/syslogd/syslogd_cap.h
index 2e52c57bcdf8..60106d7862bd 100644
--- a/usr.sbin/syslogd/syslogd_cap.h
+++ b/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)
 
diff --git a/usr.sbin/syslogd/syslogd_cap_log.c b/usr.sbin/syslogd/syslogd_cap_log.c
index 5e2034abd9eb..901349c4dc5a 100644
--- a/usr.sbin/syslogd/syslogd_cap_log.c
+++ b/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");
-	}
-	if (nvlist_exists_string(nvl, "errstr")) {
-		const char *errstr = nvlist_get_string(nvl, "errstr");
-		(void)strlcpy(errbuf, errstr, sizeof(errbuf));
-		ret = errbuf;
+		return (-1);
 	}
-
-	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);
 }
 


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a1480c6.37b13.546d11ae>