telnetd: document bug in remove_iacs. reinstate band-aid

which was making it near-impossible to trigger.
remove memmove call which was happening at each network read,
and in 99%+ cases was not needed. Unfortunately, +50 bytes.
This commit is contained in:
Denis Vlasenko 2007-10-15 17:28:00 +00:00
parent 59d7c43dbe
commit 10916c5c6b

View File

@ -43,8 +43,8 @@ struct tsession {
/*char *buf1, *buf2;*/ /*char *buf1, *buf2;*/
/*#define TS_BUF1 ts->buf1*/ /*#define TS_BUF1 ts->buf1*/
/*#define TS_BUF2 TS_BUF2*/ /*#define TS_BUF2 TS_BUF2*/
#define TS_BUF1 ((char*)(ts + 1)) #define TS_BUF1 ((unsigned char*)(ts + 1))
#define TS_BUF2 (((char*)(ts + 1)) + BUFSIZE) #define TS_BUF2 (((unsigned char*)(ts + 1)) + BUFSIZE)
int rdidx1, wridx1, size1; int rdidx1, wridx1, size1;
int rdidx2, wridx2, size2; int rdidx2, wridx2, size2;
}; };
@ -82,27 +82,30 @@ static const char *issuefile = "/etc/issue.net";
FIXME - if we mean to send 0xFF to the terminal then it will be escaped, 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. what is the escape character? We aren't handling that situation here.
CR-LF ->'s CR mapping is also done here, for convenience CR-LF ->'s CR mapping is also done here, for convenience.
NB: may fail to remove iacs which wrap around buffer!
*/ */
static char * static unsigned char *
remove_iacs(struct tsession *ts, int *pnum_totty) remove_iacs(struct tsession *ts, int *pnum_totty)
{ {
unsigned char *ptr0 = (unsigned char *)TS_BUF1 + ts->wridx1; unsigned char *ptr0 = TS_BUF1 + ts->wridx1;
unsigned char *ptr = ptr0; unsigned char *ptr = ptr0;
unsigned char *totty = ptr; unsigned char *totty = ptr;
unsigned char *end = ptr + MIN(BUFSIZE - ts->wridx1, ts->size1); unsigned char *end = ptr + MIN(BUFSIZE - ts->wridx1, ts->size1);
int processed;
int num_totty; int num_totty;
while (ptr < end) { while (ptr < end) {
if (*ptr != IAC) { if (*ptr != IAC) {
int c = *ptr; char c = *ptr;
*totty++ = *ptr++;
*totty++ = c;
ptr++;
/* We now map \r\n ==> \r for pragmatic reasons. /* We now map \r\n ==> \r for pragmatic reasons.
* Many client implementations send \r\n when * Many client implementations send \r\n when
* the user hits the CarriageReturn key. * the user hits the CarriageReturn key.
*/ */
if (c == '\r' && (*ptr == '\n' || *ptr == 0) && ptr < end) if (c == '\r' && ptr < end && (*ptr == '\n' || *ptr == '\0'))
ptr++; ptr++;
} else { } else {
/* /*
@ -120,6 +123,7 @@ remove_iacs(struct tsession *ts, int *pnum_totty)
*/ */
else if (ptr[1] == SB && ptr[2] == TELOPT_NAWS) { else if (ptr[1] == SB && ptr[2] == TELOPT_NAWS) {
struct winsize ws; struct winsize ws;
if ((ptr+8) >= end) if ((ptr+8) >= end)
break; /* incomplete, can't process */ break; /* incomplete, can't process */
ws.ws_col = (ptr[3] << 8) | ptr[4]; ws.ws_col = (ptr[3] << 8) | ptr[4];
@ -137,16 +141,15 @@ remove_iacs(struct tsession *ts, int *pnum_totty)
} }
} }
processed = ptr - ptr0;
num_totty = totty - ptr0; num_totty = totty - ptr0;
/* the difference between processed and num_to tty
is all the iacs we removed from the stream.
Adjust buf1 accordingly. */
ts->wridx1 += processed - num_totty;
ts->size1 -= processed - num_totty;
*pnum_totty = num_totty; *pnum_totty = num_totty;
/* move the chars meant for the terminal towards the end of the /* the difference between ptr and totty is number of iacs
buffer. */ we removed from the stream. Adjust buf1 accordingly. */
if ((ptr - totty) == 0) /* 99.999% of cases */
return ptr0;
ts->wridx1 += ptr - totty;
ts->size1 -= ptr - totty;
/* move chars meant for the terminal towards the end of the buffer */
return memmove(ptr - num_totty, ptr0, num_totty); return memmove(ptr - num_totty, ptr0, num_totty);
} }
@ -360,7 +363,7 @@ int telnetd_main(int argc, char **argv)
{ {
fd_set rdfdset, wrfdset; fd_set rdfdset, wrfdset;
unsigned opt; unsigned opt;
int selret, maxlen, w, r; int count;
struct tsession *ts; struct tsession *ts;
#if ENABLE_FEATURE_TELNETD_STANDALONE #if ENABLE_FEATURE_TELNETD_STANDALONE
#define IS_INETD (opt & OPT_INETD) #define IS_INETD (opt & OPT_INETD)
@ -473,8 +476,8 @@ int telnetd_main(int argc, char **argv)
ts = ts->next; ts = ts->next;
} }
selret = select(maxfd + 1, &rdfdset, &wrfdset, NULL, NULL); count = select(maxfd + 1, &rdfdset, &wrfdset, NULL, NULL);
if (selret < 0) if (count < 0)
goto again; /* EINTR or ENOMEM */ goto again; /* EINTR or ENOMEM */
#if ENABLE_FEATURE_TELNETD_STANDALONE #if ENABLE_FEATURE_TELNETD_STANDALONE
@ -504,11 +507,11 @@ int telnetd_main(int argc, char **argv)
if (/*ts->size1 &&*/ FD_ISSET(ts->ptyfd, &wrfdset)) { if (/*ts->size1 &&*/ FD_ISSET(ts->ptyfd, &wrfdset)) {
int num_totty; int num_totty;
char *ptr; unsigned char *ptr;
/* Write to pty from buffer 1. */ /* Write to pty from buffer 1. */
ptr = remove_iacs(ts, &num_totty); ptr = remove_iacs(ts, &num_totty);
w = safe_write(ts->ptyfd, ptr, num_totty); count = safe_write(ts->ptyfd, ptr, num_totty);
if (w < 0) { if (count < 0) {
if (errno == EAGAIN) if (errno == EAGAIN)
goto skip1; goto skip1;
if (IS_INETD) if (IS_INETD)
@ -517,17 +520,17 @@ int telnetd_main(int argc, char **argv)
ts = next; ts = next;
continue; continue;
} }
ts->size1 -= w; ts->size1 -= count;
ts->wridx1 += w; ts->wridx1 += count;
if (ts->wridx1 >= BUFSIZE) /* actually == BUFSIZE */ if (ts->wridx1 >= BUFSIZE) /* actually == BUFSIZE */
ts->wridx1 = 0; ts->wridx1 = 0;
} }
skip1: skip1:
if (/*ts->size2 &&*/ FD_ISSET(ts->sockfd_write, &wrfdset)) { if (/*ts->size2 &&*/ FD_ISSET(ts->sockfd_write, &wrfdset)) {
/* Write to socket from buffer 2. */ /* Write to socket from buffer 2. */
maxlen = MIN(BUFSIZE - ts->wridx2, ts->size2); count = MIN(BUFSIZE - ts->wridx2, ts->size2);
w = safe_write(ts->sockfd_write, TS_BUF2 + ts->wridx2, maxlen); count = safe_write(ts->sockfd_write, TS_BUF2 + ts->wridx2, count);
if (w < 0) { if (count < 0) {
if (errno == EAGAIN) if (errno == EAGAIN)
goto skip2; goto skip2;
if (IS_INETD) if (IS_INETD)
@ -536,14 +539,18 @@ int telnetd_main(int argc, char **argv)
ts = next; ts = next;
continue; continue;
} }
ts->size2 -= w; ts->size2 -= count;
ts->wridx2 += w; ts->wridx2 += count;
if (ts->wridx2 >= BUFSIZE) /* actually == BUFSIZE */ if (ts->wridx2 >= BUFSIZE) /* actually == BUFSIZE */
ts->wridx2 = 0; ts->wridx2 = 0;
} }
skip2: skip2:
#if 0 /* Should not be needed, but... remove_iacs is actually buggy
/* Not strictly needed, but allows for bigger reads in common case */ * (it cannot process iacs which wrap around buffer's end)!
* Since properly fixing it requires writing bigger code,
* we will rely instead on this code making it virtually impossible
* to have wrapped iac (people don't type at 2k/second).
* It also allows for bigger reads in common case. */
if (ts->size1 == 0) { if (ts->size1 == 0) {
ts->rdidx1 = 0; ts->rdidx1 = 0;
ts->wridx1 = 0; ts->wridx1 = 0;
@ -552,13 +559,13 @@ int telnetd_main(int argc, char **argv)
ts->rdidx2 = 0; ts->rdidx2 = 0;
ts->wridx2 = 0; ts->wridx2 = 0;
} }
#endif
if (/*ts->size1 < BUFSIZE &&*/ FD_ISSET(ts->sockfd_read, &rdfdset)) { if (/*ts->size1 < BUFSIZE &&*/ FD_ISSET(ts->sockfd_read, &rdfdset)) {
/* Read from socket to buffer 1. */ /* Read from socket to buffer 1. */
maxlen = MIN(BUFSIZE - ts->rdidx1, BUFSIZE - ts->size1); count = MIN(BUFSIZE - ts->rdidx1, BUFSIZE - ts->size1);
r = safe_read(ts->sockfd_read, TS_BUF1 + ts->rdidx1, maxlen); count = safe_read(ts->sockfd_read, TS_BUF1 + ts->rdidx1, count);
if (r <= 0) { if (count <= 0) {
if (r < 0 && errno == EAGAIN) if (count < 0 && errno == EAGAIN)
goto skip3; goto skip3;
if (IS_INETD) if (IS_INETD)
return 0; return 0;
@ -567,22 +574,22 @@ int telnetd_main(int argc, char **argv)
continue; continue;
} }
/* Ignore trailing NUL if it is there */ /* Ignore trailing NUL if it is there */
if (!TS_BUF1[ts->rdidx1 + r - 1]) { if (!TS_BUF1[ts->rdidx1 + count - 1]) {
if (!--r) if (!--count)
goto skip3; goto skip3;
} }
ts->size1 += r; ts->size1 += count;
ts->rdidx1 += r; ts->rdidx1 += count;
if (ts->rdidx1 >= BUFSIZE) /* actually == BUFSIZE */ if (ts->rdidx1 >= BUFSIZE) /* actually == BUFSIZE */
ts->rdidx1 = 0; ts->rdidx1 = 0;
} }
skip3: skip3:
if (/*ts->size2 < BUFSIZE &&*/ FD_ISSET(ts->ptyfd, &rdfdset)) { if (/*ts->size2 < BUFSIZE &&*/ FD_ISSET(ts->ptyfd, &rdfdset)) {
/* Read from pty to buffer 2. */ /* Read from pty to buffer 2. */
maxlen = MIN(BUFSIZE - ts->rdidx2, BUFSIZE - ts->size2); count = MIN(BUFSIZE - ts->rdidx2, BUFSIZE - ts->size2);
r = safe_read(ts->ptyfd, TS_BUF2 + ts->rdidx2, maxlen); count = safe_read(ts->ptyfd, TS_BUF2 + ts->rdidx2, count);
if (r <= 0) { if (count <= 0) {
if (r < 0 && errno == EAGAIN) if (count < 0 && errno == EAGAIN)
goto skip4; goto skip4;
if (IS_INETD) if (IS_INETD)
return 0; return 0;
@ -590,8 +597,8 @@ int telnetd_main(int argc, char **argv)
ts = next; ts = next;
continue; continue;
} }
ts->size2 += r; ts->size2 += count;
ts->rdidx2 += r; ts->rdidx2 += count;
if (ts->rdidx2 >= BUFSIZE) /* actually == BUFSIZE */ if (ts->rdidx2 >= BUFSIZE) /* actually == BUFSIZE */
ts->rdidx2 = 0; ts->rdidx2 = 0;
} }