Merge remote-tracking branch 'andrea/bug11476'

changes/bug11476

@@ -0,0 +1,5 @@
+  o Bugfixes:
+    - Add configure options controlling allocator tricks like mempools and
+      freelists, and turn them off by default; on most platforms malloc is
+      reasonable enough for this not to be necessary, and a similar feature
+      in OpenSSL exacerbated Heartbleed.  Fixes bug #11476.

configure.ac

@@ -24,7 +24,9 @@ CPPFLAGS="$CPPFLAGS -I\${top_srcdir}/src/common"
 
 #XXXX020 We should make these enabled or not, before 0.2.0.x-final
 AC_ARG_ENABLE(buf-freelists,
-   AS_HELP_STRING(--disable-buf-freelists, disable freelists for buffer RAM))
+   AS_HELP_STRING(--enable-buf-freelists, enable freelists for buffer RAM))
+AC_ARG_ENABLE(mempools,
+   AS_HELP_STRING(--enable-mempools, enable mempools for relay cells))
 AC_ARG_ENABLE(openbsd-malloc,
    AS_HELP_STRING(--enable-openbsd-malloc, Use malloc code from openbsd.  Linux only))
 AC_ARG_ENABLE(instrument-downloads,
@@ -54,10 +56,17 @@ if test "$enable_static_tor" = "yes"; then
   CFLAGS="$CFLAGS -static"
 fi
 
-if test x$enable_buf_freelists != xno; then
+if test x$enable_buf_freelists = xyes; then
   AC_DEFINE(ENABLE_BUF_FREELISTS, 1,
             [Defined if we try to use freelists for buffer RAM chunks])
 fi
+
+AM_CONDITIONAL(USE_MEMPOOLS, test x$enable_mempools = xyes)
+if test x$enable_mempools = xyes; then
+  AC_DEFINE(ENABLE_MEMPOOLS, 1,
+            [Defined if we try to use mempools for cells being relayed])
+fi
+
 AM_CONDITIONAL(USE_OPENBSD_MALLOC, test x$enable_openbsd_malloc = xyes)
 if test x$enable_instrument_downloads = xyes; then
   AC_DEFINE(INSTRUMENT_DOWNLOADS, 1,

src/common/include.am

@@ -24,6 +24,14 @@ else
 libor_extra_source=
 endif
 
+if USE_MEMPOOLS
+libor_mempool_source=src/common/mempool.c
+libor_mempool_header=src/common/mempool.h
+else
+libor_mempool_source=
+libor_mempool_header=
+endif
+
 src_common_libcurve25519_donna_a_CFLAGS=
 
 if BUILD_CURVE25519_DONNA
@@ -56,13 +64,13 @@ LIBOR_A_SOURCES = \
   src/common/di_ops.c					\
   src/common/log.c					\
   src/common/memarea.c					\
-  src/common/mempool.c					\
   src/common/procmon.c					\
   src/common/util.c					\
   src/common/util_codedigest.c				\
   src/common/sandbox.c					\
   src/ext/csiphash.c					\
-  $(libor_extra_source)
+  $(libor_extra_source)					\
+  $(libor_mempool_source)
 
 LIBOR_CRYPTO_A_SOURCES = \
   src/common/aes.c		\
@@ -102,7 +110,6 @@ COMMONHEADERS = \
   src/common/crypto_curve25519.h		\
   src/common/di_ops.h				\
   src/common/memarea.h				\
-  src/common/mempool.h				\
   src/common/linux_syscalls.inc			\
   src/common/procmon.h				\
   src/common/sandbox.h				\
@@ -111,7 +118,8 @@ COMMONHEADERS = \
   src/common/torint.h				\
   src/common/torlog.h				\
   src/common/tortls.h				\
-  src/common/util.h
+  src/common/util.h				\
+  $(libor_mempool_header)
 
 noinst_HEADERS+= $(COMMONHEADERS)
 

src/or/circuitlist.c

@@ -1983,7 +1983,9 @@ circuits_handle_oom(size_t current_allocation)
       break;
   } SMARTLIST_FOREACH_END(circ);
 
+#ifdef ENABLE_MEMPOOLS
   clean_cell_pool(); /* In case this helps. */
+#endif /* ENABLE_MEMPOOLS */
   buf_shrink_freelists(1); /* This is necessary to actually release buffer
                               chunks. */
 

src/or/main.c

@@ -1522,7 +1522,9 @@ run_scheduled_events(time_t now)
         if (conn->inbuf)
           buf_shrink(conn->inbuf);
       });
+#ifdef ENABLE_MEMPOOL
     clean_cell_pool();
+#endif /* ENABLE_MEMPOOL */
     buf_shrink_freelists(0);
 /** How often do we check buffers and pools for empty space that can be
  * deallocated? */
@@ -1927,8 +1929,10 @@ do_main_loop(void)
     }
   }
 
+#ifdef ENABLE_MEMPOOLS
   /* Set up the packed_cell_t memory pool. */
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 
   /* Set up our buckets */
   connection_bucket_init();
@@ -2545,7 +2549,9 @@ tor_free_all(int postfork)
     router_free_all();
     policies_free_all();
   }
+#ifdef ENABLE_MEMPOOLS
   free_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
   if (!postfork) {
     tor_tls_free_all();
 #ifndef _WIN32

src/or/relay.c

@@ -26,7 +26,9 @@
 #include "control.h"
 #include "geoip.h"
 #include "main.h"
+#ifdef ENABLE_MEMPOOLS
 #include "mempool.h"
+#endif
 #include "networkstatus.h"
 #include "nodelist.h"
 #include "onion.h"
@@ -2231,9 +2233,10 @@ circuit_consider_sending_sendme(circuit_t *circ, crypt_path_t *layer_hint)
 #define assert_cmux_ok_paranoid(chan)
 #endif
 
-/** The total number of cells we have allocated from the memory pool. */
+/** The total number of cells we have allocated. */
 static size_t total_cells_allocated = 0;
 
+#ifdef ENABLE_MEMPOOLS
 /** A memory pool to allocate packed_cell_t objects. */
 static mp_pool_t *cell_pool = NULL;
 
@@ -2265,12 +2268,30 @@ clean_cell_pool(void)
   mp_pool_clean(cell_pool, 0, 1);
 }
 
+#define relay_alloc_cell() \
+  mp_pool_get(cell_pool)
+#define relay_free_cell(cell) \
+  mp_pool_release(cell)
+
+#define RELAY_CELL_MEM_COST (sizeof(packed_cell_t) + MP_POOL_ITEM_OVERHEAD)
+
+#else /* !ENABLE_MEMPOOLS case */
+
+#define relay_alloc_cell() \
+  tor_malloc_zero(sizeof(packed_cell_t))
+#define relay_free_cell(cell) \
+  tor_free(cell)
+
+#define RELAY_CELL_MEM_COST (sizeof(packed_cell_t))
+
+#endif /* ENABLE_MEMPOOLS */
+
 /** Release storage held by <b>cell</b>. */
 static INLINE void
 packed_cell_free_unchecked(packed_cell_t *cell)
 {
   --total_cells_allocated;
-  mp_pool_release(cell);
+  relay_free_cell(cell);
 }
 
 /** Allocate and return a new packed_cell_t. */
@@ -2278,7 +2299,7 @@ STATIC packed_cell_t *
 packed_cell_new(void)
 {
   ++total_cells_allocated;
-  return mp_pool_get(cell_pool);
+  return relay_alloc_cell();
 }
 
 /** Return a packed cell used outside by channel_t lower layer */
@@ -2307,7 +2328,9 @@ dump_cell_pool_usage(int severity)
   tor_log(severity, LD_MM,
           "%d cells allocated on %d circuits. %d cells leaked.",
           n_cells, n_circs, (int)total_cells_allocated - n_cells);
+#ifdef ENABLE_MEMPOOLS
   mp_pool_log_status(cell_pool, severity);
+#endif
 }
 
 /** Allocate a new copy of packed <b>cell</b>. */
@@ -2387,7 +2410,7 @@ cell_queue_pop(cell_queue_t *queue)
 size_t
 packed_cell_mem_cost(void)
 {
-  return sizeof(packed_cell_t) + MP_POOL_ITEM_OVERHEAD;
+  return RELAY_CELL_MEM_COST;
 }
 
 /** DOCDOC */

src/or/relay.h

@@ -42,9 +42,11 @@ extern uint64_t stats_n_data_bytes_packaged;
 extern uint64_t stats_n_data_cells_received;
 extern uint64_t stats_n_data_bytes_received;
 
+#ifdef ENABLE_MEMPOOLS
 void init_cell_pool(void);
 void free_cell_pool(void);
 void clean_cell_pool(void);
+#endif /* ENABLE_MEMPOOLS */
 void dump_cell_pool_usage(int severity);
 size_t packed_cell_mem_cost(void);
 

src/test/test.c

@@ -52,7 +52,9 @@ double fabs(double x);
 #include "rendcommon.h"
 #include "test.h"
 #include "torgzip.h"
+#ifdef ENABLE_MEMPOOLS
 #include "mempool.h"
+#endif
 #include "memarea.h"
 #include "onion.h"
 #include "onion_ntor.h"

src/test/test_cell_queue.c

@@ -16,7 +16,10 @@ test_cq_manip(void *arg)
   cell_t cell;
   (void) arg;
 
+#ifdef ENABLE_MEMPOOLS
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
+
   cell_queue_init(&cq);
   tt_int_op(cq.n, ==, 0);
 
@@ -96,7 +99,10 @@ test_cq_manip(void *arg)
   packed_cell_free(pc_tmp);
 
   cell_queue_clear(&cq);
+
+#ifdef ENABLE_MEMPOOLS
   free_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 }
 
 static void
@@ -108,7 +114,9 @@ test_circuit_n_cells(void *arg)
 
   (void)arg;
 
+#ifdef ENABLE_MEMPOOLS
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 
   pc1 = packed_cell_new();
   pc2 = packed_cell_new();
@@ -137,7 +145,9 @@ test_circuit_n_cells(void *arg)
   circuit_free(TO_CIRCUIT(or_c));
   circuit_free(TO_CIRCUIT(origin_c));
 
+#ifdef ENABLE_MEMPOOLS
   free_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 }
 
 struct testcase_t cell_queue_tests[] = {

src/test/test_circuitmux.c

@@ -36,7 +36,9 @@ test_cmux_destroy_cell_queue(void *arg)
   cell_queue_t *cq = NULL;
   packed_cell_t *pc = NULL;
 
+#ifdef ENABLE_MEMPOOLS
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
   (void) arg;
 
   cmux = circuitmux_alloc();
@@ -74,7 +76,9 @@ test_cmux_destroy_cell_queue(void *arg)
   channel_free(ch);
   packed_cell_free(pc);
 
+#ifdef ENABLE_MEMPOOLS
   free_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 }
 
 struct testcase_t circuitmux_tests[] = {

src/test/test_oom.c

@@ -13,7 +13,9 @@
 #include "compat_libevent.h"
 #include "connection.h"
 #include "config.h"
+#ifdef ENABLE_MEMPOOLS
 #include "mempool.h"
+#endif
 #include "relay.h"
 #include "test.h"
 
@@ -131,7 +133,10 @@ test_oom_circbuf(void *arg)
   (void) arg;
 
   MOCK(circuit_mark_for_close_, circuit_mark_for_close_dummy_);
+
+#ifdef ENABLE_MEMPOOLS
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 
   /* Far too low for real life. */
   options->MaxMemInQueues = 256*packed_cell_mem_cost();
@@ -150,8 +155,13 @@ test_oom_circbuf(void *arg)
   tor_gettimeofday_cache_set(&tv);
   c2 = dummy_or_circuit_new(20, 20);
 
+#ifdef ENABLE_MEMPOOLS
   tt_int_op(packed_cell_mem_cost(), ==,
             sizeof(packed_cell_t) + MP_POOL_ITEM_OVERHEAD);
+#else
+  tt_int_op(packed_cell_mem_cost(), ==,
+            sizeof(packed_cell_t));
+#endif /* ENABLE_MEMPOOLS */
   tt_int_op(cell_queues_get_total_allocation(), ==,
             packed_cell_mem_cost() * 70);
   tt_int_op(cell_queues_check_size(), ==, 0); /* We are still not OOM */
@@ -222,7 +232,10 @@ test_oom_streambuf(void *arg)
   (void) arg;
 
   MOCK(circuit_mark_for_close_, circuit_mark_for_close_dummy_);
+
+#ifdef ENABLE_MEMPOOLS
   init_cell_pool();
+#endif /* ENABLE_MEMPOOLS */
 
   /* Far too low for real life. */
   options->MaxMemInQueues = 81*packed_cell_mem_cost() + 4096 * 34;

src/test/test_util.c

@@ -12,7 +12,9 @@
 #include "config.h"
 #include "control.h"
 #include "test.h"
+#ifdef ENABLE_MEMPOOLS
 #include "mempool.h"
+#endif /* ENABLE_MEMPOOLS */
 #include "memarea.h"
 
 #ifdef _WIN32
@@ -1899,6 +1901,8 @@ test_util_path_is_relative(void)
   ;
 }
 
+#ifdef ENABLE_MEMPOOLS
+
 /** Run unittests for memory pool allocator */
 static void
 test_util_mempool(void)
@@ -1957,6 +1961,8 @@ test_util_mempool(void)
     mp_pool_destroy(pool);
 }
 
+#endif /* ENABLE_MEMPOOLS */
+
 /** Run unittests for memory area allocator */
 static void
 test_util_memarea(void)
@@ -3661,7 +3667,9 @@ struct testcase_t util_tests[] = {
   UTIL_LEGACY(pow2),
   UTIL_LEGACY(gzip),
   UTIL_LEGACY(datadir),
+#ifdef ENABLE_MEMPOOLS
   UTIL_LEGACY(mempool),
+#endif
   UTIL_LEGACY(memarea),
   UTIL_LEGACY(control_formats),
   UTIL_LEGACY(mmap),