Merge branch 'bug20081'

4 files changed, 32 insertions, 1 deletions

diff --git a/changes/bug20081 b/changes/bug20081
new file mode 100644
index 0000000000..a95161cdc3
--- /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 3198572392..c08da63a0d 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 275867c70a..52b21d5885 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 971dd1d889..3408da3aa9 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
 };