diff options
author | Damien Miller <djm@mindrot.org> | 2006-04-23 12:06:03 +1000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2006-04-23 12:06:03 +1000 |
commit | 499a0d5ada82acbf8a5c5d496dbf0b4570dde1af (patch) | |
tree | bf1074febeccab92e23341d27133fc7cdc0eb567 | |
parent | 63e437f053bec9e227ba11e5e6205cd1e217baac (diff) |
- 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@
-rw-r--r-- | ChangeLog | 11 | ||||
-rw-r--r-- | buffer.c | 61 | ||||
-rw-r--r-- | buffer.h | 7 | ||||
-rw-r--r-- | channels.c | 8 |
4 files changed, 64 insertions, 23 deletions
@@ -17,6 +17,15 @@ | |||
17 | GSSAPI buffers shouldn't be nul-terminated, spotted in bugzilla #1066 | 17 | GSSAPI buffers shouldn't be nul-terminated, spotted in bugzilla #1066 |
18 | by dleonard AT vintela.com. use xasprintf() to simplify code while in | 18 | by dleonard AT vintela.com. use xasprintf() to simplify code while in |
19 | there; "looks right" deraadt@ | 19 | there; "looks right" deraadt@ |
20 | - djm@cvs.openbsd.org 2006/04/16 00:48:52 | ||
21 | [buffer.c buffer.h channels.c] | ||
22 | Fix condition where we could exit with a fatal error when an input | ||
23 | buffer became too large and the remote end had advertised a big window. | ||
24 | The problem was a mismatch in the backoff math between the channels code | ||
25 | and the buffer code, so make a buffer_check_alloc() function that the | ||
26 | channels code can use to propsectivly check whether an incremental | ||
27 | allocation will succeed. bz #1131, debugged with the assistance of | ||
28 | cove AT wildpackets.com; ok dtucker@ deraadt@ | ||
20 | 29 | ||
21 | 20060421 | 30 | 20060421 |
22 | - (djm) [Makefile.in configure.ac session.c sshpty.c] | 31 | - (djm) [Makefile.in configure.ac session.c sshpty.c] |
@@ -4528,4 +4537,4 @@ | |||
4528 | - (djm) Trim deprecated options from INSTALL. Mention UsePAM | 4537 | - (djm) Trim deprecated options from INSTALL. Mention UsePAM |
4529 | - (djm) Fix quote handling in sftp; Patch from admorten AT umich.edu | 4538 | - (djm) Fix quote handling in sftp; Patch from admorten AT umich.edu |
4530 | 4539 | ||
4531 | $Id: ChangeLog,v 1.4306 2006/04/23 02:05:46 djm Exp $ | 4540 | $Id: ChangeLog,v 1.4307 2006/04/23 02:06:03 djm Exp $ |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: buffer.c,v 1.26 2006/03/25 13:17:01 djm Exp $ */ | 1 | /* $OpenBSD: buffer.c,v 1.27 2006/04/16 00:48:52 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -18,6 +18,10 @@ | |||
18 | #include "buffer.h" | 18 | #include "buffer.h" |
19 | #include "log.h" | 19 | #include "log.h" |
20 | 20 | ||
21 | #define BUFFER_MAX_CHUNK 0x100000 | ||
22 | #define BUFFER_MAX_LEN 0xa00000 | ||
23 | #define BUFFER_ALLOCSZ 0x008000 | ||
24 | |||
21 | /* Initializes the buffer structure. */ | 25 | /* Initializes the buffer structure. */ |
22 | 26 | ||
23 | void | 27 | void |
@@ -66,6 +70,23 @@ buffer_append(Buffer *buffer, const void *data, u_int len) | |||
66 | memcpy(p, data, len); | 70 | memcpy(p, data, len); |
67 | } | 71 | } |
68 | 72 | ||
73 | static int | ||
74 | buffer_compact(Buffer *buffer) | ||
75 | { | ||
76 | /* | ||
77 | * If the buffer is quite empty, but all data is at the end, move the | ||
78 | * data to the beginning. | ||
79 | */ | ||
80 | if (buffer->offset > MIN(buffer->alloc, BUFFER_MAX_CHUNK)) { | ||
81 | memmove(buffer->buf, buffer->buf + buffer->offset, | ||
82 | buffer->end - buffer->offset); | ||
83 | buffer->end -= buffer->offset; | ||
84 | buffer->offset = 0; | ||
85 | return (1); | ||
86 | } | ||
87 | return (0); | ||
88 | } | ||
89 | |||
69 | /* | 90 | /* |
70 | * Appends space to the buffer, expanding the buffer if necessary. This does | 91 | * Appends space to the buffer, expanding the buffer if necessary. This does |
71 | * not actually copy the data into the buffer, but instead returns a pointer | 92 | * not actually copy the data into the buffer, but instead returns a pointer |
@@ -93,20 +114,13 @@ restart: | |||
93 | buffer->end += len; | 114 | buffer->end += len; |
94 | return p; | 115 | return p; |
95 | } | 116 | } |
96 | /* | 117 | |
97 | * If the buffer is quite empty, but all data is at the end, move the | 118 | /* Compact data back to the start of the buffer if necessary */ |
98 | * data to the beginning and retry. | 119 | if (buffer_compact(buffer)) |
99 | */ | ||
100 | if (buffer->offset > MIN(buffer->alloc, BUFFER_MAX_CHUNK)) { | ||
101 | memmove(buffer->buf, buffer->buf + buffer->offset, | ||
102 | buffer->end - buffer->offset); | ||
103 | buffer->end -= buffer->offset; | ||
104 | buffer->offset = 0; | ||
105 | goto restart; | 120 | goto restart; |
106 | } | ||
107 | /* Increase the size of the buffer and retry. */ | ||
108 | 121 | ||
109 | newlen = buffer->alloc + len + 32768; | 122 | /* Increase the size of the buffer and retry. */ |
123 | newlen = roundup(buffer->alloc + len, BUFFER_ALLOCSZ); | ||
110 | if (newlen > BUFFER_MAX_LEN) | 124 | if (newlen > BUFFER_MAX_LEN) |
111 | fatal("buffer_append_space: alloc %u not supported", | 125 | fatal("buffer_append_space: alloc %u not supported", |
112 | newlen); | 126 | newlen); |
@@ -116,6 +130,27 @@ restart: | |||
116 | /* NOTREACHED */ | 130 | /* NOTREACHED */ |
117 | } | 131 | } |
118 | 132 | ||
133 | /* | ||
134 | * Check whether an allocation of 'len' will fit in the buffer | ||
135 | * This must follow the same math as buffer_append_space | ||
136 | */ | ||
137 | int | ||
138 | buffer_check_alloc(Buffer *buffer, u_int len) | ||
139 | { | ||
140 | if (buffer->offset == buffer->end) { | ||
141 | buffer->offset = 0; | ||
142 | buffer->end = 0; | ||
143 | } | ||
144 | restart: | ||
145 | if (buffer->end + len < buffer->alloc) | ||
146 | return (1); | ||
147 | if (buffer_compact(buffer)) | ||
148 | goto restart; | ||
149 | if (roundup(buffer->alloc + len, BUFFER_ALLOCSZ) <= BUFFER_MAX_LEN) | ||
150 | return (1); | ||
151 | return (0); | ||
152 | } | ||
153 | |||
119 | /* Returns the number of bytes of data in the buffer. */ | 154 | /* Returns the number of bytes of data in the buffer. */ |
120 | 155 | ||
121 | u_int | 156 | u_int |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: buffer.h,v 1.14 2006/03/25 22:22:42 djm Exp $ */ | 1 | /* $OpenBSD: buffer.h,v 1.15 2006/04/16 00:48:52 djm Exp $ */ |
2 | 2 | ||
3 | /* | 3 | /* |
4 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 4 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
@@ -23,9 +23,6 @@ typedef struct { | |||
23 | u_int end; /* Offset of last byte containing data. */ | 23 | u_int end; /* Offset of last byte containing data. */ |
24 | } Buffer; | 24 | } Buffer; |
25 | 25 | ||
26 | #define BUFFER_MAX_CHUNK 0x100000 | ||
27 | #define BUFFER_MAX_LEN 0xa00000 | ||
28 | |||
29 | void buffer_init(Buffer *); | 26 | void buffer_init(Buffer *); |
30 | void buffer_clear(Buffer *); | 27 | void buffer_clear(Buffer *); |
31 | void buffer_free(Buffer *); | 28 | void buffer_free(Buffer *); |
@@ -36,6 +33,8 @@ void *buffer_ptr(Buffer *); | |||
36 | void buffer_append(Buffer *, const void *, u_int); | 33 | void buffer_append(Buffer *, const void *, u_int); |
37 | void *buffer_append_space(Buffer *, u_int); | 34 | void *buffer_append_space(Buffer *, u_int); |
38 | 35 | ||
36 | int buffer_check_alloc(Buffer *, u_int); | ||
37 | |||
39 | void buffer_get(Buffer *, void *, u_int); | 38 | void buffer_get(Buffer *, void *, u_int); |
40 | 39 | ||
41 | void buffer_consume(Buffer *, u_int); | 40 | 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 @@ | |||
1 | /* $OpenBSD: channels.c,v 1.249 2006/03/30 09:41:25 djm Exp $ */ | 1 | /* $OpenBSD: channels.c,v 1.250 2006/04/16 00:48:52 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -747,12 +747,10 @@ channel_pre_open(Channel *c, fd_set *readset, fd_set *writeset) | |||
747 | { | 747 | { |
748 | u_int limit = compat20 ? c->remote_window : packet_get_maxsize(); | 748 | u_int limit = compat20 ? c->remote_window : packet_get_maxsize(); |
749 | 749 | ||
750 | /* check buffer limits */ | ||
751 | limit = MIN(limit, (BUFFER_MAX_LEN - BUFFER_MAX_CHUNK - CHAN_RBUF)); | ||
752 | |||
753 | if (c->istate == CHAN_INPUT_OPEN && | 750 | if (c->istate == CHAN_INPUT_OPEN && |
754 | limit > 0 && | 751 | limit > 0 && |
755 | buffer_len(&c->input) < limit) | 752 | buffer_len(&c->input) < limit && |
753 | buffer_check_alloc(&c->input, CHAN_RBUF)) | ||
756 | FD_SET(c->rfd, readset); | 754 | FD_SET(c->rfd, readset); |
757 | if (c->ostate == CHAN_OUTPUT_OPEN || | 755 | if (c->ostate == CHAN_OUTPUT_OPEN || |
758 | c->ostate == CHAN_OUTPUT_WAIT_DRAIN) { | 756 | c->ostate == CHAN_OUTPUT_WAIT_DRAIN) { |