From 59d7c43dbe502ef7425f6828bb43528c5adfc9a9 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 15 Oct 2007 15:19:36 +0000 Subject: [PATCH] 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 --- networking/telnetd.c | 270 ++++++++++++++++++++++++------------------- 1 file 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 -#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; -- 2.25.1