tty: stop spamming VEOF on raw terminals, and cleanly handle EOF

fd.c

@@ -5,6 +5,7 @@
  */
 
 #include <err.h>
+#include <fcntl.h>
 #include <stdarg.h>
 #include <stddef.h>
 #include <sys/socket.h>
@@ -101,3 +102,22 @@ void rebind_fds_and_close_rest(int start_fd, ...)
 		err(1, "close_range");
 	}
 }
+
+/* close_null closes fd by rebinding it to /dev/null.
+   This is done to avoid leaving the old fd number unoccupied,
+   which can cause issues for the standard file descriptor numbers. */
+void close_null(int fd)
+{
+	int nfd = open("/dev/null", O_RDWR | O_CLOEXEC);
+	if (nfd == -1) {
+		err(1, "close_null: open /dev/null");
+	}
+
+	if (dup3(nfd, fd, O_CLOEXEC) == -1) {
+		err(1, "close_null: dup2 fd %d -> /dev/null", fd);
+	}
+
+	if (close(nfd) == -1) {
+		err(1, "close_null: close");
+	}
+}

fd.h

@@ -10,5 +10,6 @@
 int recv_fd(int socket);
 void send_fd(int socket, int fd);
 void rebind_fds_and_close_rest(int start_fd, ...);
+void close_null(int fd);
 
 #endif /* !FD_H */

tty.c

@@ -97,6 +97,9 @@ static ssize_t io_copy(int out_fd, int in_fd, struct buffer *buf)
 			}
 			return -1;
 		}
+		if (rd == 0) {
+			return copied;
+		}
 		buf->size = (size_t) rd;
 		buf->index = 0;
 	}
@@ -125,29 +128,46 @@ static int send_veof(int ptm)
 		err(1, "send_veof: tcgetattr");
 	}
 
+	/* The terminal is is noncanonical mode; VEOF won't be interpreted. */
+	if (!(tios.c_lflag & ICANON)) {
+		errno = ENOTSUP;
+		return -1;
+	}
+
 	return write(ptm, &tios.c_cc[VEOF], 1);
 }
 
-void tty_parent_cleanup(void)
+static void tty_parent_resetterm(void)
 {
-	if (info.termfd >= 0) {
-		/* Drain any remaining data in the terminal buffer */
-		set_nonblock(STDOUT_FILENO, 0);
-		set_nonblock(info.termfd, 0);
-		struct buffer drain = {
-			.size = 0,
-		};
+	tcsetattr(STDIN_FILENO, TCSADRAIN, &info.orig);
+	info.stdinIsatty = false;
+}
 
-		if (io_copy(STDOUT_FILENO, info.termfd, &drain) == -1 && errno != EIO) {
-			warn("copy tty -> stdout");
-		}
+static void tty_parent_drain(void)
+{
+	/* Drain any remaining data in the terminal buffer */
+	set_nonblock(STDOUT_FILENO, 0);
+	set_nonblock(info.termfd, 0);
+	struct buffer drain = {
+		.size = 0,
+	};
+
+	if (io_copy(STDOUT_FILENO, info.termfd, &drain) == -1 && errno != EIO) {
+		warn("copy tty -> stdout");
+	}
+
+	close_null(STDOUT_FILENO);
+	close(info.termfd);
+	info.termfd = -1;
+}
 
-		close(info.termfd);
-		info.termfd = -1;
+void tty_parent_cleanup(void)
+{
+	if (info.termfd >= 0) {
+		tty_parent_drain();
 	}
 	if (info.stdinIsatty) {
-		tcsetattr(STDIN_FILENO, TCSADRAIN, &info.orig);
-		info.stdinIsatty = false;
+		tty_parent_resetterm();
 	}
 }
 
@@ -180,6 +200,11 @@ static struct buffer inbound_buffer, outbound_buffer;
 
 static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_t pid)
 {
+	/* The terminal got closed -- don't try to handle I/O any further */
+	if (info.termfd == -1) {
+		return EPOLL_HANDLER_CONTINUE;
+	}
+
 	struct epoll_handler *handler = ev->data.ptr;
 
 	if (fd == inbound_handler.fd) {
@@ -224,6 +249,11 @@ static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_
 
 		inbound_handler.ready &= ~(READ_READY|WRITE_READY);
 
+		if (copied == 0) {
+			tty_parent_drain();
+			return EPOLL_HANDLER_CONTINUE;
+		}
+
 		struct epoll_event newev = {
 			.events = EPOLLIN | EPOLLONESHOT,
 			.data.ptr = &inbound_handler,
@@ -232,7 +262,12 @@ static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_
 			err(1, "epoll_ctl_mod stdin");
 		}
 	} else if ((inbound_handler.ready & (WRITE_READY | HANGUP)) == (WRITE_READY | HANGUP)) {
+		inbound_handler.ready &= ~HANGUP;
+
 		if (send_veof(inbound_handler.peer_fd) == -1) {
+			if (errno == ENOTSUP) {
+				goto hangup;
+			}
 			err(1, "send_eof: write");
 		}
 
@@ -242,15 +277,16 @@ static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_
 		   EOF. */
 		if (send_veof(inbound_handler.peer_fd) == -1) {
 			switch (errno) {
+			case ENOTSUP:
+				goto hangup;
 			case EAGAIN:
 				/* The pty device isn't ready for a second VEOF -- that's fine,
-				   we'll just send it later */
+				   we'll just send it later, so re-set the hangup flag */
+				inbound_handler.ready |= HANGUP;
 				break;
 			default:
 				err(1, "send_eof: write");
 			}
-		} else {
-			inbound_handler.ready &= ~HANGUP;
 		}
 	}
 
@@ -259,12 +295,18 @@ static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_
 		/* outbound_handler.fd might contain our eventfd workaround */
 		int write_fd = STDOUT_FILENO;
 
-		if (io_copy(write_fd, read_fd, &outbound_buffer) == -1) {
+		ssize_t copied = io_copy(write_fd, read_fd, &outbound_buffer);
+		if (copied == -1) {
 			err(1, "copy tty -> stdout");
 		}
 
 		outbound_handler.ready = 0;
 
+		if (copied == 0) {
+			close_null(write_fd);
+			return EPOLL_HANDLER_CONTINUE;
+		}
+
 		struct epoll_event newev = {
 			.events = EPOLLOUT | EPOLLONESHOT,
 			.data.ptr = &outbound_handler,
@@ -292,6 +334,11 @@ static int tty_handle_io(int epollfd, const struct epoll_event *ev, int fd, pid_
 	}
 
 	return EPOLL_HANDLER_CONTINUE;
+
+hangup:
+	close(info.termfd);
+	info.termfd = -1;
+	return EPOLL_HANDLER_CONTINUE;
 }
 
 void tty_parent_setup(struct tty_opts *opts, int epollfd, int socket)
@@ -316,8 +363,9 @@ void tty_parent_setup(struct tty_opts *opts, int epollfd, int socket)
 		/* We changed the terminal to raw mode. Line-endings now need carriage
 		   returns in order to be palatable. */
 		err_line_ending = "\r\n";
+
+		atexit(tty_parent_resetterm);
 	}
-	atexit(tty_parent_cleanup);
 
 	// Wait for the child to create the pty pair and pass the master back.
 	info.termfd = recv_fd(socket);