On Thu, Jan 2, 2014 at 11:13 PM, Ryan Mallon <[email protected]> wrote: > The buf->buf buffer in do_rd() is explicitly NUL terminated. If the > total number of bytes read is equal to the buffer size, then the NUL > terminator will be written at the byte after the end of the buffer. > Fix this by reducing the size of the read by 1 byte.
Thanks. Applied this part... > Also removed the check for buf->pos being less than > (int)sizeof(buf->buf). This is unnecessary since buf->pos cannot > exceed sizeof(buf->buf), and was also incorrect since it was checked > after buf->buf[buf->pos] was assigned with a NUL value. Removing it > fixes a spurious warning from Smatch (http://smatch.sourceforge.net/). > @@ -70,7 +71,7 @@ static int do_rd(int fd, void **paramp) > p = strpbrk(cur, "\r\n"); > if (p) > *p = '\0'; > - if (!p && sz && buf->pos <= (int)sizeof(buf->buf)) > + if (!p && sz) > goto ok; > /* Terminate session. If we are in server mode, then > * fd is still in nonblocking mode - we never block here */ This check means "if we still have some space in the buffer to read into, continue reading peer's data; if not, then terminate (looks like peer is maliciously sending us too much data)" You are right that the test is off-by-one, but removing it altogether changes intended logic: now, we will wait for more data even if buffer is full and we know we can't use more data. Looks like such change will be ok: we don't expect "good" clients to send such large requests. Applying it too. Thanks! _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
