- djm@cvs.openbsd.org 2010/08/05 13:08:42
[channels.c]
Fix a trio of bugs in the local/remote window calculation for datagram
data channels (i.e. TunnelForward):
Calculate local_consumed correctly in channel_handle_wfd() by measuring
the delta to buffer_len(c->output) from when we start to when we finish.
The proximal problem here is that the output_filter we use in portable
modified the length of the dequeued datagram (to futz with the headers
for !OpenBSD).
In channel_output_poll(), don't enqueue datagrams that won't fit in the
peer's advertised packet size (highly unlikely to ever occur) or which
won't fit in the peer's remaining window (more likely).
In channel_input_data(), account for the 4-byte string header in
datagram packets that we accept from the peer and enqueue in c->output.
report, analysis and testing 2/3 cases from wierbows AT us.ibm.com;
"looks good" markus@
diff --git a/channels.c b/channels.c
index fd6244d..1cd5004 100644
--- a/channels.c
+++ b/channels.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.308 2010/07/13 23:13:16 djm Exp $ */
+/* $OpenBSD: channels.c,v 1.309 2010/08/05 13:08:42 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1644,13 +1644,14 @@
{
struct termios tio;
u_char *data = NULL, *buf;
- u_int dlen;
+ u_int dlen, olen = 0;
int len;
/* Send buffered output data to the socket. */
if (c->wfd != -1 &&
FD_ISSET(c->wfd, writeset) &&
buffer_len(&c->output) > 0) {
+ olen = buffer_len(&c->output);
if (c->output_filter != NULL) {
if ((buf = c->output_filter(c, &data, &dlen)) == NULL) {
debug2("channel %d: filter stops", c->self);
@@ -1669,7 +1670,6 @@
if (c->datagram) {
/* ignore truncated writes, datagrams might get lost */
- c->local_consumed += dlen + 4;
len = write(c->wfd, buf, dlen);
xfree(data);
if (len < 0 && (errno == EINTR || errno == EAGAIN ||
@@ -1682,7 +1682,7 @@
chan_write_failed(c);
return -1;
}
- return 1;
+ goto out;
}
#ifdef _AIX
/* XXX: Later AIX versions can't push as much data to tty */
@@ -1724,10 +1724,10 @@
}
#endif
buffer_consume(&c->output, len);
- if (compat20 && len > 0) {
- c->local_consumed += len;
- }
}
+ out:
+ if (compat20 && olen > 0)
+ c->local_consumed += olen - buffer_len(&c->output);
return 1;
}
@@ -2172,6 +2172,14 @@
data = buffer_get_string(&c->input,
&dlen);
+ if (dlen > c->remote_window ||
+ dlen > c->remote_maxpacket) {
+ debug("channel %d: datagram "
+ "too big for channel",
+ c->self);
+ xfree(data);
+ continue;
+ }
packet_start(SSH2_MSG_CHANNEL_DATA);
packet_put_int(c->remote_id);
packet_put_string(data, dlen);
@@ -2257,7 +2265,7 @@
{
int id;
char *data;
- u_int data_len;
+ u_int data_len, win_len;
Channel *c;
/* Get the channel number and verify it. */
@@ -2273,6 +2281,9 @@
/* Get the data. */
data = packet_get_string_ptr(&data_len);
+ win_len = data_len;
+ if (c->datagram)
+ win_len += 4; /* string length header */
/*
* Ignore data for protocol > 1.3 if output end is no longer open.
@@ -2283,23 +2294,23 @@
*/
if (!compat13 && c->ostate != CHAN_OUTPUT_OPEN) {
if (compat20) {
- c->local_window -= data_len;
- c->local_consumed += data_len;
+ c->local_window -= win_len;
+ c->local_consumed += win_len;
}
return;
}
if (compat20) {
- if (data_len > c->local_maxpacket) {
+ if (win_len > c->local_maxpacket) {
logit("channel %d: rcvd big packet %d, maxpack %d",
- c->self, data_len, c->local_maxpacket);
+ c->self, win_len, c->local_maxpacket);
}
- if (data_len > c->local_window) {
+ if (win_len > c->local_window) {
logit("channel %d: rcvd too much data %d, win %d",
- c->self, data_len, c->local_window);
+ c->self, win_len, c->local_window);
return;
}
- c->local_window -= data_len;
+ c->local_window -= win_len;
}
if (c->datagram)
buffer_put_string(&c->output, data, data_len);