commit 20c4b0169493dae7624632dbca49758812138aeb Author: Nick Mathewson <nickm@torproject.org> Date: Tue Sep 13 09:07:12 2016 -0400 Make preferred_chunk_size avoid overflow, handle big inputs better Also, add tests for the function. Closes 20081; bugfix on 0.2.0.16-alpha. This is a Guido Vranken issue. Thanks, Guido! --- changes/bug20081 | 5 +++++ src/or/buffers.c | 5 ++++- src/or/buffers.h | 1 + src/test/test_buffers.c | 22 ++++++++++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/changes/bug20081 b/changes/bug20081 new file mode 100644 index 0000000..a95161c --- /dev/null +++ b/changes/bug20081 @@ -0,0 +1,5 @@ + o Minor bugfixes (allocation): + - Change how we allocate memory for large chunks on buffers, to avoid + a (currently impossible) integer overflow, and to waste less space + when allocating unusually large chunks. Fixes bug 20081; bugfix on + 0.2.0.16-alpha. Issue identified by Guido Vranken. diff --git a/src/or/buffers.c b/src/or/buffers.c index 3198572..c08da63 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -166,9 +166,12 @@ chunk_grow(chunk_t *chunk, size_t sz) /** Return the allocation size we'd like to use to hold <b>target</b> * bytes. */ -static inline size_t +STATIC size_t preferred_chunk_size(size_t target) { + tor_assert(target <= SIZE_T_CEILING - CHUNK_HEADER_LEN); + if (CHUNK_ALLOC_SIZE(target) >= MAX_CHUNK_ALLOC) + return CHUNK_ALLOC_SIZE(target); size_t sz = MIN_CHUNK_ALLOC; while (CHUNK_SIZE_WITH_ALLOC(sz) < target) { sz <<= 1; diff --git a/src/or/buffers.h b/src/or/buffers.h index 275867c..52b21d5 100644 --- a/src/or/buffers.h +++ b/src/or/buffers.h @@ -65,6 +65,7 @@ void assert_buf_ok(buf_t *buf); STATIC int buf_find_string_offset(const buf_t *buf, const char *s, size_t n); STATIC void buf_pullup(buf_t *buf, size_t bytes); void buf_get_first_chunk_data(const buf_t *buf, const char **cp, size_t *sz); +STATIC size_t preferred_chunk_size(size_t target); #define DEBUG_CHUNK_ALLOC /** A single chunk on a buffer. */ diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c index 971dd1d..3408da3 100644 --- a/src/test/test_buffers.c +++ b/src/test/test_buffers.c @@ -744,6 +744,27 @@ test_buffers_tls_read_mocked(void *arg) buf_free(buf); } +static void +test_buffers_chunk_size(void *arg) +{ + (void)arg; + const int min = 256; + const int max = 65536; + tt_uint_op(preferred_chunk_size(3), OP_EQ, min); + tt_uint_op(preferred_chunk_size(25), OP_EQ, min); + tt_uint_op(preferred_chunk_size(0), OP_EQ, min); + tt_uint_op(preferred_chunk_size(256), OP_EQ, 512); + tt_uint_op(preferred_chunk_size(65400), OP_EQ, max); + /* Here, we're implicitly saying that the chunk header overhead is + * between 1 and 100 bytes. 24..48 would probably be more accurate. */ + tt_uint_op(preferred_chunk_size(65536), OP_GT, 65536); + tt_uint_op(preferred_chunk_size(65536), OP_LT, 65536+100); + tt_uint_op(preferred_chunk_size(165536), OP_GT, 165536); + tt_uint_op(preferred_chunk_size(165536), OP_LT, 165536+100); + done: + ; +} + struct testcase_t buffer_tests[] = { { "basic", test_buffers_basic, TT_FORK, NULL, NULL }, { "copy", test_buffer_copy, TT_FORK, NULL, NULL }, @@ -758,6 +779,7 @@ struct testcase_t buffer_tests[] = { NULL, NULL}, { "tls_read_mocked", test_buffers_tls_read_mocked, 0, NULL, NULL }, + { "chunk_size", test_buffers_chunk_size, 0, NULL, NULL }, END_OF_TESTCASES };