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 27dde1ae4..9ce793fb7 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;