← Commit list

59d7c43db

Denis Vlasenko
Denis Vlasenko
2007-10-15
FAILED Duration: 12s

Subject

telnetd: some simplifications and better error hadling. telnetd: don't SIGKILL child when closing the session. kernel will seng SIGHUP for us.

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

networking/telnetd.c +150 -120
networking/telnetd.c+150 -120
@@ -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;