From 499a0d5ada82acbf8a5c5d496dbf0b4570dde1af Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sun, 23 Apr 2006 12:06:03 +1000 Subject: - djm@cvs.openbsd.org 2006/04/16 00:48:52 [buffer.c buffer.h channels.c] Fix condition where we could exit with a fatal error when an input buffer became too large and the remote end had advertised a big window. The problem was a mismatch in the backoff math between the channels code and the buffer code, so make a buffer_check_alloc() function that the channels code can use to propsectivly check whether an incremental allocation will succeed. bz #1131, debugged with the assistance of cove AT wildpackets.com; ok dtucker@ deraadt@ --- ChangeLog | 11 ++++++++++- buffer.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++------------- buffer.h | 7 +++---- channels.c | 8 +++----- 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index 360cf61b3..d3abc2b68 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,15 @@ GSSAPI buffers shouldn't be nul-terminated, spotted in bugzilla #1066 by dleonard AT vintela.com. use xasprintf() to simplify code while in there; "looks right" deraadt@ + - djm@cvs.openbsd.org 2006/04/16 00:48:52 + [buffer.c buffer.h channels.c] + Fix condition where we could exit with a fatal error when an input + buffer became too large and the remote end had advertised a big window. + The problem was a mismatch in the backoff math between the channels code + and the buffer code, so make a buffer_check_alloc() function that the + channels code can use to propsectivly check whether an incremental + allocation will succeed. bz #1131, debugged with the assistance of + cove AT wildpackets.com; ok dtucker@ deraadt@ 20060421 - (djm) [Makefile.in configure.ac session.c sshpty.c] @@ -4528,4 +4537,4 @@ - (djm) Trim deprecated options from INSTALL. Mention UsePAM - (djm) Fix quote handling in sftp; Patch from admorten AT umich.edu -$Id: ChangeLog,v 1.4306 2006/04/23 02:05:46 djm Exp $ +$Id: ChangeLog,v 1.4307 2006/04/23 02:06:03 djm Exp $ diff --git a/buffer.c b/buffer.c index de404e602..ba718daf2 100644 --- a/buffer.c +++ b/buffer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: buffer.c,v 1.26 2006/03/25 13:17:01 djm Exp $ */ +/* $OpenBSD: buffer.c,v 1.27 2006/04/16 00:48:52 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -18,6 +18,10 @@ #include "buffer.h" #include "log.h" +#define BUFFER_MAX_CHUNK 0x100000 +#define BUFFER_MAX_LEN 0xa00000 +#define BUFFER_ALLOCSZ 0x008000 + /* Initializes the buffer structure. */ void @@ -66,6 +70,23 @@ buffer_append(Buffer *buffer, const void *data, u_int len) memcpy(p, data, len); } +static int +buffer_compact(Buffer *buffer) +{ + /* + * If the buffer is quite empty, but all data is at the end, move the + * data to the beginning. + */ + if (buffer->offset > MIN(buffer->alloc, BUFFER_MAX_CHUNK)) { + memmove(buffer->buf, buffer->buf + buffer->offset, + buffer->end - buffer->offset); + buffer->end -= buffer->offset; + buffer->offset = 0; + return (1); + } + return (0); +} + /* * Appends space to the buffer, expanding the buffer if necessary. This does * not actually copy the data into the buffer, but instead returns a pointer @@ -93,20 +114,13 @@ restart: buffer->end += len; return p; } - /* - * If the buffer is quite empty, but all data is at the end, move the - * data to the beginning and retry. - */ - if (buffer->offset > MIN(buffer->alloc, BUFFER_MAX_CHUNK)) { - memmove(buffer->buf, buffer->buf + buffer->offset, - buffer->end - buffer->offset); - buffer->end -= buffer->offset; - buffer->offset = 0; + + /* Compact data back to the start of the buffer if necessary */ + if (buffer_compact(buffer)) goto restart; - } - /* Increase the size of the buffer and retry. */ - newlen = buffer->alloc + len + 32768; + /* Increase the size of the buffer and retry. */ + newlen = roundup(buffer->alloc + len, BUFFER_ALLOCSZ); if (newlen > BUFFER_MAX_LEN) fatal("buffer_append_space: alloc %u not supported", newlen); @@ -116,6 +130,27 @@ restart: /* NOTREACHED */ } +/* + * Check whether an allocation of 'len' will fit in the buffer + * This must follow the same math as buffer_append_space + */ +int +buffer_check_alloc(Buffer *buffer, u_int len) +{ + if (buffer->offset == buffer->end) { + buffer->offset = 0; + buffer->end = 0; + } + restart: + if (buffer->end + len < buffer->alloc) + return (1); + if (buffer_compact(buffer)) + goto restart; + if (roundup(buffer->alloc + len, BUFFER_ALLOCSZ) <= BUFFER_MAX_LEN) + return (1); + return (0); +} + /* Returns the number of bytes of data in the buffer. */ u_int diff --git a/buffer.h b/buffer.h index abdaea349..43414ae99 100644 --- a/buffer.h +++ b/buffer.h @@ -1,4 +1,4 @@ -/* $OpenBSD: buffer.h,v 1.14 2006/03/25 22:22:42 djm Exp $ */ +/* $OpenBSD: buffer.h,v 1.15 2006/04/16 00:48:52 djm Exp $ */ /* * Author: Tatu Ylonen @@ -23,9 +23,6 @@ typedef struct { u_int end; /* Offset of last byte containing data. */ } Buffer; -#define BUFFER_MAX_CHUNK 0x100000 -#define BUFFER_MAX_LEN 0xa00000 - void buffer_init(Buffer *); void buffer_clear(Buffer *); void buffer_free(Buffer *); @@ -36,6 +33,8 @@ void *buffer_ptr(Buffer *); void buffer_append(Buffer *, const void *, u_int); void *buffer_append_space(Buffer *, u_int); +int buffer_check_alloc(Buffer *, u_int); + void buffer_get(Buffer *, void *, u_int); void buffer_consume(Buffer *, u_int); diff --git a/channels.c b/channels.c index fccb9098e..2fa997edc 100644 --- a/channels.c +++ b/channels.c @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.c,v 1.249 2006/03/30 09:41:25 djm Exp $ */ +/* $OpenBSD: channels.c,v 1.250 2006/04/16 00:48:52 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -747,12 +747,10 @@ channel_pre_open(Channel *c, fd_set *readset, fd_set *writeset) { u_int limit = compat20 ? c->remote_window : packet_get_maxsize(); - /* check buffer limits */ - limit = MIN(limit, (BUFFER_MAX_LEN - BUFFER_MAX_CHUNK - CHAN_RBUF)); - if (c->istate == CHAN_INPUT_OPEN && limit > 0 && - buffer_len(&c->input) < limit) + buffer_len(&c->input) < limit && + buffer_check_alloc(&c->input, CHAN_RBUF)) FD_SET(c->rfd, readset); if (c->ostate == CHAN_OUTPUT_OPEN || c->ostate == CHAN_OUTPUT_WAIT_DRAIN) { -- cgit v1.2.3