svn commit: trunk/busybox/networking
vda at busybox.net
vda at busybox.net
Mon Oct 15 17:28:01 UTC 2007
Author: vda
Date: 2007-10-15 10:28:00 -0700 (Mon, 15 Oct 2007)
New Revision: 20261
Log:
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.
Modified:
trunk/busybox/networking/telnetd.c
Changeset:
Modified: trunk/busybox/networking/telnetd.c
===================================================================
--- trunk/busybox/networking/telnetd.c 2007-10-15 15:19:36 UTC (rev 20260)
+++ trunk/busybox/networking/telnetd.c 2007-10-15 17:28:00 UTC (rev 20261)
@@ -43,8 +43,8 @@
/*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)
+#define TS_BUF1 ((unsigned char*)(ts + 1))
+#define TS_BUF2 (((unsigned char*)(ts + 1)) + BUFSIZE)
int rdidx1, wridx1, size1;
int rdidx2, wridx2, size2;
};
@@ -82,27 +82,30 @@
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.
- 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)
{
- unsigned char *ptr0 = (unsigned char *)TS_BUF1 + ts->wridx1;
+ unsigned char *ptr0 = TS_BUF1 + ts->wridx1;
unsigned char *ptr = ptr0;
unsigned char *totty = ptr;
unsigned char *end = ptr + MIN(BUFSIZE - ts->wridx1, ts->size1);
- int processed;
int num_totty;
while (ptr < end) {
if (*ptr != IAC) {
- int c = *ptr;
- *totty++ = *ptr++;
+ char c = *ptr;
+
+ *totty++ = c;
+ ptr++;
/* We now map \r\n ==> \r for pragmatic reasons.
* Many client implementations send \r\n when
* 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++;
} else {
/*
@@ -120,6 +123,7 @@
*/
else if (ptr[1] == SB && ptr[2] == TELOPT_NAWS) {
struct winsize ws;
+
if ((ptr+8) >= end)
break; /* incomplete, can't process */
ws.ws_col = (ptr[3] << 8) | ptr[4];
@@ -137,16 +141,15 @@
}
}
- processed = ptr - 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;
+ num_totty = totty - ptr0;
*pnum_totty = num_totty;
- /* move the chars meant for the terminal towards the end of the
- buffer. */
+ /* the difference between ptr and totty is number of iacs
+ 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);
}
@@ -360,7 +363,7 @@
{
fd_set rdfdset, wrfdset;
unsigned opt;
- int selret, maxlen, w, r;
+ int count;
struct tsession *ts;
#if ENABLE_FEATURE_TELNETD_STANDALONE
#define IS_INETD (opt & OPT_INETD)
@@ -473,8 +476,8 @@
ts = ts->next;
}
- selret = select(maxfd + 1, &rdfdset, &wrfdset, NULL, NULL);
- if (selret < 0)
+ count = select(maxfd + 1, &rdfdset, &wrfdset, NULL, NULL);
+ if (count < 0)
goto again; /* EINTR or ENOMEM */
#if ENABLE_FEATURE_TELNETD_STANDALONE
@@ -504,11 +507,11 @@
if (/*ts->size1 &&*/ FD_ISSET(ts->ptyfd, &wrfdset)) {
int num_totty;
- char *ptr;
+ unsigned char *ptr;
/* Write to pty from buffer 1. */
ptr = remove_iacs(ts, &num_totty);
- w = safe_write(ts->ptyfd, ptr, num_totty);
- if (w < 0) {
+ count = safe_write(ts->ptyfd, ptr, num_totty);
+ if (count < 0) {
if (errno == EAGAIN)
goto skip1;
if (IS_INETD)
@@ -517,17 +520,17 @@
ts = next;
continue;
}
- ts->size1 -= w;
- ts->wridx1 += w;
+ ts->size1 -= count;
+ ts->wridx1 += count;
if (ts->wridx1 >= BUFSIZE) /* actually == BUFSIZE */
ts->wridx1 = 0;
}
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);
- if (w < 0) {
+ count = MIN(BUFSIZE - ts->wridx2, ts->size2);
+ count = safe_write(ts->sockfd_write, TS_BUF2 + ts->wridx2, count);
+ if (count < 0) {
if (errno == EAGAIN)
goto skip2;
if (IS_INETD)
@@ -536,14 +539,18 @@
ts = next;
continue;
}
- ts->size2 -= w;
- ts->wridx2 += w;
+ ts->size2 -= count;
+ ts->wridx2 += count;
if (ts->wridx2 >= BUFSIZE) /* actually == BUFSIZE */
ts->wridx2 = 0;
}
skip2:
-#if 0
- /* Not strictly needed, but allows for bigger reads in common case */
+ /* Should not be needed, but... remove_iacs is actually buggy
+ * (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) {
ts->rdidx1 = 0;
ts->wridx1 = 0;
@@ -552,13 +559,13 @@
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) {
- if (r < 0 && errno == EAGAIN)
+ count = MIN(BUFSIZE - ts->rdidx1, BUFSIZE - ts->size1);
+ count = safe_read(ts->sockfd_read, TS_BUF1 + ts->rdidx1, count);
+ if (count <= 0) {
+ if (count < 0 && errno == EAGAIN)
goto skip3;
if (IS_INETD)
return 0;
@@ -567,22 +574,22 @@
continue;
}
/* Ignore trailing NUL if it is there */
- if (!TS_BUF1[ts->rdidx1 + r - 1]) {
- if (!--r)
+ if (!TS_BUF1[ts->rdidx1 + count - 1]) {
+ if (!--count)
goto skip3;
}
- ts->size1 += r;
- ts->rdidx1 += r;
+ ts->size1 += count;
+ ts->rdidx1 += count;
if (ts->rdidx1 >= BUFSIZE) /* actually == BUFSIZE */
ts->rdidx1 = 0;
}
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) {
- if (r < 0 && errno == EAGAIN)
+ count = MIN(BUFSIZE - ts->rdidx2, BUFSIZE - ts->size2);
+ count = safe_read(ts->ptyfd, TS_BUF2 + ts->rdidx2, count);
+ if (count <= 0) {
+ if (count < 0 && errno == EAGAIN)
goto skip4;
if (IS_INETD)
return 0;
@@ -590,8 +597,8 @@
ts = next;
continue;
}
- ts->size2 += r;
- ts->rdidx2 += r;
+ ts->size2 += count;
+ ts->rdidx2 += count;
if (ts->rdidx2 >= BUFSIZE) /* actually == BUFSIZE */
ts->rdidx2 = 0;
}
More information about the busybox-cvs
mailing list