telnetd: some simplifications and better error hadling.
authorDenis Vlasenko <vda.linux@googlemail.com>
Mon, 15 Oct 2007 15:19:36 +0000 (15:19 -0000)
committerDenis Vlasenko <vda.linux@googlemail.com>
Mon, 15 Oct 2007 15:19:36 +0000 (15:19 -0000)
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

networking/telnetd.c

index 27dde1a..9ce793f 100644 (file)
 #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;