telnetd: some simplifications and better error hadling.

telnetd: don't SIGKILL child when closing the session.
kernel will seng SIGHUP for us.

static.iacs_to_send                                    -      15     +15
.rodata                                           123418  123429     +11
make_new_session                                     549     525     -24
send_iac                                              26       -     -26
free_session                                         144     118     -26
telnetd_main                                        1303    1261     -42
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 1/3 up/down: 26/-118)           Total: -92 bytes
   text    data     bss     dec     hex filename
 676341    2538   12104  690983   a8b27 busybox_old
 676234    2538   12104  690876   a8abc busybox_unstripped

1 files changed, 150 insertions, 120 deletions

diff --git a/networking/telnetd.c b/networking/telnetd.c
index 27dde1a..9ce793f 100644
--- a/networking/telnetd.c
+++ b/networking/telnetd.c
@@ -33,63 +33,51 @@
 #include <sys/syslog.h>
 
 
-#if ENABLE_LOGIN
-static const char *loginpath = "/bin/login";
-#else
-static const char *loginpath = DEFAULT_SHELL;
-#endif
-
-static const char *issuefile = "/etc/issue.net";
-
-/* structure that describes a session */
-
+/* Structure that describes a session */
 struct tsession {
 	struct tsession *next;
 	int sockfd_read, sockfd_write, ptyfd;
-	int shell_pid;
+	/*int shell_pid;*/
+
 	/* two circular buffers */
-	char *buf1, *buf2;
+	/*char *buf1, *buf2;*/
+/*#define TS_BUF1 ts->buf1*/
+/*#define TS_BUF2 TS_BUF2*/
+#define TS_BUF1 ((char*)(ts + 1))
+#define TS_BUF2 (((char*)(ts + 1)) + BUFSIZE)
 	int rdidx1, wridx1, size1;
 	int rdidx2, wridx2, size2;
 };
 
 /* Two buffers are directly after tsession in malloced memory.
  * Make whole thing fit in 4k */
-enum { BUFSIZE = (4*1024 - sizeof(struct tsession)) / 2 };
-
-/*
-   This is how the buffers are used. The arrows indicate the movement
-   of data.
-
-   +-------+     wridx1++     +------+     rdidx1++     +----------+
-   |       | <--------------  | buf1 | <--------------  |          |
-   |       |     size1--      +------+     size1++      |          |
-   |  pty  |                                            |  socket  |
-   |       |     rdidx2++     +------+     wridx2++     |          |
-   |       |  --------------> | buf2 |  --------------> |          |
-   +-------+     size2++      +------+     size2--      +----------+
+enum { BUFSIZE = (4 * 1024 - sizeof(struct tsession)) / 2 };
 
-   Each session has got two buffers.
-*/
 
+/* Globals */
 static int maxfd;
-
 static struct tsession *sessions;
+#if ENABLE_LOGIN
+static const char *loginpath = "/bin/login";
+#else
+static const char *loginpath = DEFAULT_SHELL;
+#endif
+static const char *issuefile = "/etc/issue.net";
 
 
 /*
-   Remove all IAC's from the buffer pointed to by bf (received IACs are ignored
-   and must be removed so as to not be interpreted by the terminal).  Make an
-   uninterrupted string of characters fit for the terminal.  Do this by packing
-   all characters meant for the terminal sequentially towards the end of bf.
+   Remove all IAC's from buf1 (received IACs are ignored and must be removed
+   so as to not be interpreted by the terminal).  Make an uninterrupted
+   string of characters fit for the terminal.  Do this by packing
+   all characters meant for the terminal sequentially towards the end of buf.
 
    Return a pointer to the beginning of the characters meant for the terminal.
    and make *num_totty the number of characters that should be sent to
    the terminal.
 
    Note - If an IAC (3 byte quantity) starts before (bf + len) but extends
-   past (bf + len) then that IAC will be left unprocessed and *processed will be
-   less than len.
+   past (bf + len) then that IAC will be left unprocessed and *processed
+   will be less than len.
 
    FIXME - if we mean to send 0xFF to the terminal then it will be escaped,
    what is the escape character?  We aren't handling that situation here.
@@ -99,7 +87,7 @@ static struct tsession *sessions;
 static char *
 remove_iacs(struct tsession *ts, int *pnum_totty)
 {
-	unsigned char *ptr0 = (unsigned char *)ts->buf1 + ts->wridx1;
+	unsigned char *ptr0 = (unsigned char *)TS_BUF1 + ts->wridx1;
 	unsigned char *ptr = ptr0;
 	unsigned char *totty = ptr;
 	unsigned char *end = ptr + MIN(BUFSIZE - ts->wridx1, ts->size1);
@@ -210,19 +198,6 @@ getpty(char *line, int size)
 }
 
 
-static void
-send_iac(struct tsession *ts, unsigned char command, int option)
-{
-	/* We rely on that there is space in the buffer for now. */
-	char *b = ts->buf2 + ts->rdidx2;
-	*b++ = IAC;
-	*b++ = command;
-	*b++ = option;
-	ts->rdidx2 += 3;
-	ts->size2 += 3;
-}
-
-
 static struct tsession *
 make_new_session(
 		USE_FEATURE_TELNETD_STANDALONE(int sock_r, int sock_w)
@@ -234,25 +209,28 @@ make_new_session(
 	char tty_name[32];
 	struct tsession *ts = xzalloc(sizeof(struct tsession) + BUFSIZE * 2);
 
-	ts->buf1 = (char *)(&ts[1]);
-	ts->buf2 = ts->buf1 + BUFSIZE;
+	/*ts->buf1 = (char *)(ts + 1);*/
+	/*ts->buf2 = ts->buf1 + BUFSIZE;*/
 
 	/* Got a new connection, set up a tty. */
 	fd = getpty(tty_name, 32);
 	if (fd < 0) {
-		bb_error_msg("all terminals in use");
+		bb_error_msg("can't create pty");
 		return NULL;
 	}
 	if (fd > maxfd) maxfd = fd;
-	ndelay_on(ts->ptyfd = fd);
+	ts->ptyfd = fd;
+	ndelay_on(fd);
 #if ENABLE_FEATURE_TELNETD_STANDALONE
 	if (sock_w > maxfd) maxfd = sock_w;
+	ts->sockfd_write = sock_w;
+	ndelay_on(sock_w);
 	if (sock_r > maxfd) maxfd = sock_r;
-	ndelay_on(ts->sockfd_write = sock_w);
-	ndelay_on(ts->sockfd_read = sock_r);
+	ts->sockfd_read = sock_r;
+	ndelay_on(sock_r);
 #else
 	ts->sockfd_write = 1;
-	/* xzalloc: ts->sockfd_read = 0; */
+	/* ts->sockfd_read = 0; - done by xzalloc */
 	ndelay_on(0);
 	ndelay_on(1);
 #endif
@@ -260,26 +238,36 @@ make_new_session(
 	 * should not do it locally. We don't tell the client to run linemode,
 	 * because we want to handle line editing and tab completion and other
 	 * stuff that requires char-by-char support. */
-	send_iac(ts, DO, TELOPT_ECHO);
-	send_iac(ts, DO, TELOPT_NAWS);
-	send_iac(ts, DO, TELOPT_LFLOW);
-	send_iac(ts, WILL, TELOPT_ECHO);
-	send_iac(ts, WILL, TELOPT_SGA);
+	{
+		static const char iacs_to_send[] ALIGN1 = {
+			IAC, DO, TELOPT_ECHO,
+			IAC, DO, TELOPT_NAWS,
+			IAC, DO, TELOPT_LFLOW,
+			IAC, WILL, TELOPT_ECHO,
+			IAC, WILL, TELOPT_SGA
+		};
+		memcpy(TS_BUF2, iacs_to_send, sizeof(iacs_to_send));
+		ts->rdidx2 = sizeof(iacs_to_send);
+		ts->size2 = sizeof(iacs_to_send);
+	}
 
-	pid = fork();
+	fflush(NULL); /* flush all streams */
+	pid = vfork(); /* NOMMU-friendly */
 	if (pid < 0) {
 		free(ts);
 		close(fd);
-		bb_perror_msg("fork");
+		/* sock_r and sock_w will be closed by caller */
+		bb_perror_msg("vfork");
 		return NULL;
 	}
 	if (pid > 0) {
-		/* parent */
-		ts->shell_pid = pid;
+		/* Parent */
+		/*ts->shell_pid = pid;*/
 		return ts;
 	}
 
-	/* child */
+	/* Child */
+	/* Careful - we are after vfork! */
 
 	/* make new session and process group */
 	setsid();
@@ -298,20 +286,24 @@ make_new_session(
 	 * cooked mode, and with XTABS CRMOD enabled (see tty(4)). */
 	tcgetattr(0, &termbuf);
 	termbuf.c_lflag |= ECHO; /* if we use readline we dont want this */
-	termbuf.c_oflag |= ONLCR|XTABS;
+	termbuf.c_oflag |= ONLCR | XTABS;
 	termbuf.c_iflag |= ICRNL;
 	termbuf.c_iflag &= ~IXOFF;
 	/*termbuf.c_lflag &= ~ICANON;*/
 	tcsetattr(0, TCSANOW, &termbuf);
 
+	/* Uses FILE-based I/O to stdout, but does fflush(stdout),
+	 * so should be safe with vfork.
+	 * I fear, though, that some users will have ridiculously big
+	 * issue files, and they may block writing to fd 1. */
 	print_login_issue(issuefile, NULL);
 
-	/* exec shell / login /whatever */
+	/* Exec shell / login / whatever */
 	login_argv[0] = loginpath;
 	login_argv[1] = NULL;
-	execv(loginpath, (char **)login_argv);
-	/* Hmmm... this gets sent to the client thru fd#2! Is it ok?? */
-	bb_perror_msg_and_die("execv %s", loginpath);
+	execvp(loginpath, (char **)login_argv);
+	/* Safer with vfork, and we shouldn't send this to the client anyway */
+	_exit(1); /*bb_perror_msg_and_die("execv %s", loginpath);*/
 }
 
 #if ENABLE_FEATURE_TELNETD_STANDALONE
@@ -321,7 +313,7 @@ free_session(struct tsession *ts)
 {
 	struct tsession *t = sessions;
 
-	/* unlink this telnet session from the session list */
+	/* Unlink this telnet session from the session list */
 	if (t == ts)
 		sessions = ts->next;
 	else {
@@ -330,15 +322,18 @@ free_session(struct tsession *ts)
 		t->next = ts->next;
 	}
 
-	kill(ts->shell_pid, SIGKILL);
-	wait4(ts->shell_pid, NULL, 0, NULL);
+	/* It was said that "normal" telnetd just closes ptyfd,
+	 * doesn't send SIGKILL. When we close ptyfd,
+	 * kernel sends SIGHUP to processes having slave side opened. */
+	/*kill(ts->shell_pid, SIGKILL);
+	wait4(ts->shell_pid, NULL, 0, NULL);*/
 	close(ts->ptyfd);
 	close(ts->sockfd_read);
 	/* error if ts->sockfd_read == ts->sockfd_write. So what? ;) */
 	close(ts->sockfd_write);
 	free(ts);
 
-	/* scan all sessions and find new maxfd */
+	/* Scan all sessions and find new maxfd */
 	ts = sessions;
 	maxfd = 0;
 	while (ts) {
@@ -369,7 +364,7 @@ int telnetd_main(int argc, char **argv)
 	struct tsession *ts;
 #if ENABLE_FEATURE_TELNETD_STANDALONE
 #define IS_INETD (opt & OPT_INETD)
-	int master_fd = -1; /* be happy, gcc */
+	int master_fd = master_fd; /* be happy, gcc */
 	unsigned portnbr = 23;
 	char *opt_bindaddr = NULL;
 	char *opt_portnbr;
@@ -381,12 +376,14 @@ int telnetd_main(int argc, char **argv)
 	};
 #endif
 	enum {
-		OPT_PORT = 4 * ENABLE_FEATURE_TELNETD_STANDALONE,
-		OPT_FOREGROUND = 0x10 * ENABLE_FEATURE_TELNETD_STANDALONE,
-		OPT_INETD = 0x20 * ENABLE_FEATURE_TELNETD_STANDALONE,
+		OPT_INETD      = (1 << 2) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -i */
+		OPT_PORT       = (1 << 3) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -p */
+		OPT_FOREGROUND = (1 << 5) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -F */
 	};
 
-	opt = getopt32(argv, "f:l:" USE_FEATURE_TELNETD_STANDALONE("p:b:Fi"),
+	/* If !STANDALONE, we accept (and ignore) -i, thus people
+	 * don't need to guess whether it's ok to pass -i to us */
+	opt = getopt32(argv, "f:l:i" USE_FEATURE_TELNETD_STANDALONE("p:b:F"),
 			&issuefile, &loginpath
 			USE_FEATURE_TELNETD_STANDALONE(, &opt_portnbr, &opt_bindaddr));
 	/* Redirect log to syslog early, if needed */
@@ -410,19 +407,43 @@ int telnetd_main(int argc, char **argv)
 #if ENABLE_FEATURE_TELNETD_STANDALONE
 	if (IS_INETD) {
 		sessions = make_new_session(0, 1);
+		if (!sessions) /* pty opening or vfork problem, exit */
+			return 1; /* make_new_session prints error message */
 	} else {
+//vda: inform that we start in standalone mode?
 		master_fd = create_and_bind_stream_or_die(opt_bindaddr, portnbr);
 		xlisten(master_fd, 1);
 		if (!(opt & OPT_FOREGROUND))
+//vda: NOMMU?
 			bb_daemonize(DAEMON_CHDIR_ROOT);
 	}
 #else
 	sessions = make_new_session();
+	if (!sessions) /* pty opening or vfork problem, exit */
+		return 1; /* make_new_session prints error message */
 #endif
 
 	/* We don't want to die if just one session is broken */
 	signal(SIGPIPE, SIG_IGN);
 
+/*
+   This is how the buffers are used. The arrows indicate the movement
+   of data.
+   +-------+     wridx1++     +------+     rdidx1++     +----------+
+   |       | <--------------  | buf1 | <--------------  |          |
+   |       |     size1--      +------+     size1++      |          |
+   |  pty  |                                            |  socket  |
+   |       |     rdidx2++     +------+     wridx2++     |          |
+   |       |  --------------> | buf2 |  --------------> |          |
+   +-------+     size2++      +------+     size2--      +----------+
+
+   size1: "how many bytes are buffered for pty between rdidx1 and wridx1?"
+   size2: "how many bytes are buffered for socket between rdidx2 and wridx2?"
+
+   Each session has got two buffers. Buffers are circular. If sizeN == 0,
+   buffer is empty. If sizeN == BUFSIZE, buffer is full. In both these cases
+   rdidxN == wridxN.
+*/
  again:
 	FD_ZERO(&rdfdset);
 	FD_ZERO(&wrfdset);
@@ -435,12 +456,12 @@ int telnetd_main(int argc, char **argv)
 			maxfd = master_fd;
 	}
 
-	/* select on the master socket, all telnet sockets and their
-	 * ptys if there is room in their session buffers. */
+	/* Select on the master socket, all telnet sockets and their
+	 * ptys if there is room in their session buffers.
+	 * NB: scalability problem: we recalculate entire bitmap
+	 * before each select. Can be a problem with 500+ connections. */
 	ts = sessions;
 	while (ts) {
-		/* buf1 is used from socket to pty
-		 * buf2 is used from pty to socket */
 		if (ts->size1 > 0)       /* can write to pty */
 			FD_SET(ts->ptyfd, &wrfdset);
 		if (ts->size1 < BUFSIZE) /* can read from socket */
@@ -452,9 +473,9 @@ int telnetd_main(int argc, char **argv)
 		ts = ts->next;
 	}
 
-	selret = select(maxfd + 1, &rdfdset, &wrfdset, 0, 0);
-	if (!selret)
-		return 0;
+	selret = select(maxfd + 1, &rdfdset, &wrfdset, NULL, NULL);
+	if (selret < 0)
+		goto again; /* EINTR or ENOMEM */
 
 #if ENABLE_FEATURE_TELNETD_STANDALONE
 	/* First check for and accept new sessions. */
@@ -462,7 +483,7 @@ int telnetd_main(int argc, char **argv)
 		int fd;
 		struct tsession *new_ts;
 
-		fd = accept(master_fd, NULL, 0);
+		fd = accept(master_fd, NULL, NULL);
 		if (fd < 0)
 			goto again;
 		/* Create a new session and link it into our active list */
@@ -481,91 +502,100 @@ int telnetd_main(int argc, char **argv)
 	while (ts) { /* For all sessions... */
 		struct tsession *next = ts->next; /* in case we free ts. */
 
-		if (ts->size1 && FD_ISSET(ts->ptyfd, &wrfdset)) {
+		if (/*ts->size1 &&*/ FD_ISSET(ts->ptyfd, &wrfdset)) {
 			int num_totty;
 			char *ptr;
 			/* Write to pty from buffer 1. */
 			ptr = remove_iacs(ts, &num_totty);
 			w = safe_write(ts->ptyfd, ptr, num_totty);
-			/* needed? if (w < 0 && errno == EAGAIN) continue; */
 			if (w < 0) {
+				if (errno == EAGAIN)
+					goto skip1;
 				if (IS_INETD)
 					return 0;
 				free_session(ts);
 				ts = next;
 				continue;
 			}
-			ts->wridx1 += w;
 			ts->size1 -= w;
-			if (ts->wridx1 == BUFSIZE)
+			ts->wridx1 += w;
+			if (ts->wridx1 >= BUFSIZE) /* actually == BUFSIZE */
 				ts->wridx1 = 0;
 		}
-
-		if (ts->size2 && FD_ISSET(ts->sockfd_write, &wrfdset)) {
+ skip1:
+		if (/*ts->size2 &&*/ FD_ISSET(ts->sockfd_write, &wrfdset)) {
 			/* Write to socket from buffer 2. */
 			maxlen = MIN(BUFSIZE - ts->wridx2, ts->size2);
-			w = safe_write(ts->sockfd_write, ts->buf2 + ts->wridx2, maxlen);
-			/* needed? if (w < 0 && errno == EAGAIN) continue; */
+			w = safe_write(ts->sockfd_write, TS_BUF2 + ts->wridx2, maxlen);
 			if (w < 0) {
+				if (errno == EAGAIN)
+					goto skip2;
 				if (IS_INETD)
 					return 0;
 				free_session(ts);
 				ts = next;
 				continue;
 			}
-			ts->wridx2 += w;
 			ts->size2 -= w;
-			if (ts->wridx2 == BUFSIZE)
+			ts->wridx2 += w;
+			if (ts->wridx2 >= BUFSIZE) /* actually == BUFSIZE */
 				ts->wridx2 = 0;
 		}
-
-		if (ts->size1 < BUFSIZE && FD_ISSET(ts->sockfd_read, &rdfdset)) {
+ skip2:
+#if 0
+		/* Not strictly needed, but allows for bigger reads in common case */
+		if (ts->size1 == 0) {
+			ts->rdidx1 = 0;
+			ts->wridx1 = 0;
+		}
+		if (ts->size2 == 0) {
+			ts->rdidx2 = 0;
+			ts->wridx2 = 0;
+		}
+#endif
+		if (/*ts->size1 < BUFSIZE &&*/ FD_ISSET(ts->sockfd_read, &rdfdset)) {
 			/* Read from socket to buffer 1. */
 			maxlen = MIN(BUFSIZE - ts->rdidx1, BUFSIZE - ts->size1);
-			r = safe_read(ts->sockfd_read, ts->buf1 + ts->rdidx1, maxlen);
-			if (r < 0 && errno == EAGAIN) continue;
+			r = safe_read(ts->sockfd_read, TS_BUF1 + ts->rdidx1, maxlen);
 			if (r <= 0) {
+				if (r < 0 && errno == EAGAIN)
+					goto skip3;
 				if (IS_INETD)
 					return 0;
 				free_session(ts);
 				ts = next;
 				continue;
 			}
-			if (!ts->buf1[ts->rdidx1 + r - 1])
+			/* Ignore trailing NUL if it is there */
+			if (!TS_BUF1[ts->rdidx1 + r - 1]) {
 				if (!--r)
-					continue;
-			ts->rdidx1 += r;
+					goto skip3;
+			}
 			ts->size1 += r;
-			if (ts->rdidx1 == BUFSIZE)
+			ts->rdidx1 += r;
+			if (ts->rdidx1 >= BUFSIZE) /* actually == BUFSIZE */
 				ts->rdidx1 = 0;
 		}
-
-		if (ts->size2 < BUFSIZE && FD_ISSET(ts->ptyfd, &rdfdset)) {
+ skip3:
+		if (/*ts->size2 < BUFSIZE &&*/ FD_ISSET(ts->ptyfd, &rdfdset)) {
 			/* Read from pty to buffer 2. */
 			maxlen = MIN(BUFSIZE - ts->rdidx2, BUFSIZE - ts->size2);
-			r = safe_read(ts->ptyfd, ts->buf2 + ts->rdidx2, maxlen);
-			if (r < 0 && errno == EAGAIN) continue;
+			r = safe_read(ts->ptyfd, TS_BUF2 + ts->rdidx2, maxlen);
 			if (r <= 0) {
+				if (r < 0 && errno == EAGAIN)
+					goto skip4;
 				if (IS_INETD)
 					return 0;
 				free_session(ts);
 				ts = next;
 				continue;
 			}
-			ts->rdidx2 += r;
 			ts->size2 += r;
-			if (ts->rdidx2 == BUFSIZE)
+			ts->rdidx2 += r;
+			if (ts->rdidx2 >= BUFSIZE) /* actually == BUFSIZE */
 				ts->rdidx2 = 0;
 		}
-
-		if (ts->size1 == 0) {
-			ts->rdidx1 = 0;
-			ts->wridx1 = 0;
-		}
-		if (ts->size2 == 0) {
-			ts->rdidx2 = 0;
-			ts->wridx2 = 0;
-		}
+ skip4:
 		ts = next;
 	}
 	goto again;