Merge branch 'bug11396_v2_squashed'

Conflicts:
	src/or/main.c

changes/bug11396

@@ -0,0 +1,11 @@
+  o Minor features (security):
+
+    - If you don't specify MaxMemInQueues yourself, Tor now tries to
+      pick a good value based on your total system memory. Previously,
+      the default was always 8 GB. You can still override the default by
+      setting MaxMemInQueues yourself. Resolves ticket 11396.
+
+  o Minor features (controller):
+    - Because of the fix for ticket 11396, the real limit for memory
+      usage may no longer match the configured MaxMemInQueues value.
+      The real limit is now exposed via GETINFO limits/max-mem-in-queues.

configure.ac

@@ -353,6 +353,7 @@ AC_CHECK_FUNCS(
         strtok_r \
         strtoull \
         sysconf \
+	sysctl \
         uname \
         vasprintf \
 	_vscprintf
@@ -885,6 +886,7 @@ AC_CHECK_HEADERS(
         sys/prctl.h \
         sys/resource.h \
         sys/socket.h \
+	sys/sysctl.h \
         sys/syslimits.h \
         sys/time.h \
         sys/types.h \

doc/tor.1.txt

@@ -1761,7 +1761,8 @@ is non-zero):
     it has recovered at least 10% of this memory.  Do not set this option too
     low, or your relay may be unreliable under load.  This option only
     affects some queues, so the actual process size will be larger than
-    this. (Default: 8GB)
+    this.  If this option is set to 0, Tor will try to pick a reasonable
+    default based on your system's physical memory.  (Default: 0)
 
 DIRECTORY SERVER OPTIONS
 ------------------------

src/common/compat.c

@@ -38,6 +38,9 @@
 #ifdef HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
+#ifdef HAVE_SYS_SYSCTL_H
+#include <sys/sysctl.h>
+#endif
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
@@ -3329,3 +3332,119 @@ format_win32_error(DWORD err)
 }
 #endif
 
+#if defined(HW_PHYSMEM64)
+/* This appears to be an OpenBSD thing */
+#define INT64_HW_MEM HW_PHYSMEM64
+#elif defined(HW_MEMSIZE)
+/* OSX defines this one */
+#define INT64_HW_MEM HW_MEMSIZE
+#endif
+
+/**
+ * Helper: try to detect the total system memory, and return it. On failure,
+ * return 0.
+ */
+static uint64_t
+get_total_system_memory_impl(void)
+{
+#if defined(__linux__)
+  /* On linux, sysctl is deprecated. Because proc is so awesome that you
+   * shouldn't _want_ to write portable code, I guess? */
+  unsigned long long result=0;
+  int fd = -1;
+  char *s = NULL;
+  const char *cp;
+  size_t file_size=0;
+  if (-1 == (fd = tor_open_cloexec("/proc/meminfo",O_RDONLY,0)))
+    return 0;
+  s = read_file_to_str_until_eof(fd, 65536, &file_size);
+  if (!s)
+    goto err;
+  cp = strstr(s, "MemTotal:");
+  if (!cp)
+    goto err;
+  /* Use the system sscanf so that space will match a wider number of space */
+  if (sscanf(cp, "MemTotal: %llu kB\n", &result) != 1)
+    goto err;
+
+  close(fd);
+  tor_free(s);
+  return result * 1024;
+
+ err:
+  tor_free(s);
+  close(fd);
+  return 0;
+#elif defined (_WIN32)
+  /* Windows has MEMORYSTATUSEX; pretty straightforward. */
+  MEMORYSTATUSEX ms;
+  memset(&ms, 0, sizeof(ms));
+  ms.dwLength = sizeof(ms);
+  if (! GlobalMemoryStatusEx(&ms))
+    return 0;
+
+  return ms.ullTotalPhys;
+
+#elif defined(HAVE_SYSCTL) && defined(INT64_HW_MEM)
+  /* On many systems, HW_PYHSMEM is clipped to 32 bits; let's use a better
+   * variant if we know about it. */
+  uint64_t memsize = 0;
+  size_t len = sizeof(memsize);
+  int mib[2] = {CTL_HW, INT64_HW_MEM};
+  if (sysctl(mib,2,&memsize,&len,NULL,0))
+    return 0;
+
+  return memsize;
+
+#elif defined(HAVE_SYSCTL) && defined(HW_PHYSMEM)
+  /* On some systems (like FreeBSD I hope) you can use a size_t with
+   * HW_PHYSMEM. */
+  size_t memsize=0;
+  size_t len = sizeof(memsize);
+  int mib[2] = {CTL_HW, HW_USERMEM};
+  if (sysctl(mib,2,&memsize,&len,NULL,0))
+    return -1;
+
+  return memsize;
+
+#else
+  /* I have no clue. */
+  return 0;
+#endif
+}
+
+/**
+ * Try to find out how much physical memory the system has. On success,
+ * return 0 and set *<b>mem_out</b> to that value. On failure, return -1.
+ */
+int
+get_total_system_memory(size_t *mem_out)
+{
+  static size_t mem_cached=0;
+  uint64_t m = get_total_system_memory_impl();
+  if (0 == m) {
+    /* We couldn't find our memory total */
+    if (0 == mem_cached) {
+      /* We have no cached value either */
+      *mem_out = 0;
+      return -1;
+    }
+
+    *mem_out = mem_cached;
+    return 0;
+  }
+
+#if SIZE_T_MAX != UINT64_MAX
+  if (m > SIZE_T_MAX) {
+    /* I think this could happen if we're a 32-bit Tor running on a 64-bit
+     * system: we could have more system memory than would fit in a
+     * size_t. */
+    m = SIZE_T_MAX;
+  }
+#endif
+
+  *mem_out = mem_cached = (size_t) m;
+
+  return -1;
+}
+

src/common/compat.h

@@ -638,6 +638,8 @@ char *make_path_absolute(char *fname);
 
 char **get_environment(void);
 
+int get_total_system_memory(size_t *mem_out);
+
 int spawn_func(void (*func)(void *), void *data);
 void spawn_exit(void) ATTR_NORETURN;
 

src/or/config.c

@@ -307,7 +307,7 @@ static config_var_t option_vars_[] = {
   V(MaxAdvertisedBandwidth,      MEMUNIT,  "1 GB"),
   V(MaxCircuitDirtiness,         INTERVAL, "10 minutes"),
   V(MaxClientCircuitsPending,    UINT,     "32"),
-  V(MaxMemInQueues,              MEMUNIT,  "8 GB"),
+  VAR("MaxMeminQueues",          MEMUNIT,   MaxMemInQueues_raw, "0"),
   OBSOLETE("MaxOnionsPending"),
   V(MaxOnionQueueDelay,          MSEC_INTERVAL, "1750 msec"),
   V(MinMeasuredBWsForAuthToIgnoreAdvertised, INT, "500"),
@@ -565,6 +565,8 @@ static void config_maybe_load_geoip_files_(const or_options_t *options,
 static int options_validate_cb(void *old_options, void *options,
                                void *default_options,
                                int from_setconf, char **msg);
+static uint64_t compute_real_max_mem_in_queues(const uint64_t val,
+                                               int log_guess);
 
 /** Magic value for or_options_t. */
 #define OR_OPTIONS_MAGIC 9090909
@@ -2824,11 +2826,9 @@ options_validate(or_options_t *old_options, or_options_t *options,
     REJECT("If EntryNodes is set, UseEntryGuards must be enabled.");
   }
 
-  if (options->MaxMemInQueues < (256 << 20)) {
-    log_warn(LD_CONFIG, "MaxMemInQueues must be at least 256 MB for now. "
-             "Ideally, have it as large as you can afford.");
-    options->MaxMemInQueues = (256 << 20);
-  }
+  options->MaxMemInQueues =
+    compute_real_max_mem_in_queues(options->MaxMemInQueues_raw,
+                                   server_mode(options));
 
   options->AllowInvalid_ = 0;
 
@@ -3578,6 +3578,68 @@ options_validate(or_options_t *old_options, or_options_t *options,
 #undef COMPLAIN
 }
 
+/* Given the value that the user has set for MaxMemInQueues, compute the
+ * actual maximum value.  We clip this value if it's too low, and autodetect
+ * it if it's set to 0. */
+static uint64_t
+compute_real_max_mem_in_queues(const uint64_t val, int log_guess)
+{
+  uint64_t result;
+
+  if (val == 0) {
+#define ONE_GIGABYTE (U64_LITERAL(1) << 30)
+#define ONE_MEGABYTE (U64_LITERAL(1) << 20)
+#if SIZEOF_VOID_P >= 8
+#define MAX_DEFAULT_MAXMEM (8*ONE_GIGABYTE)
+#else
+#define MAX_DEFAULT_MAXMEM (2*ONE_GIGABYTE)
+#endif
+    /* The user didn't pick a memory limit.  Choose a very large one
+     * that is still smaller than the system memory */
+    static int notice_sent = 0;
+    size_t ram = 0;
+    if (get_total_system_memory(&ram) < 0) {
+      /* We couldn't determine our total system memory!  */
+#if SIZEOF_VOID_P >= 8
+      /* 64-bit system.  Let's hope for 8 GB. */
+      result = 8 * ONE_GIGABYTE;
+#else
+      /* (presumably) 32-bit system. Let's hope for 1 GB. */
+      result = ONE_GIGABYTE;
+#endif
+    } else {
+      /* We detected it, so let's pick 3/4 of the total RAM as our limit. */
+      const uint64_t avail = (ram / 4) * 3;
+
+      /* Make sure it's in range from 0.25 GB to 8 GB. */
+      if (avail > MAX_DEFAULT_MAXMEM) {
+        /* If you want to use more than this much RAM, you need to configure
+           it yourself */
+        result = MAX_DEFAULT_MAXMEM;
+      } else if (avail < ONE_GIGABYTE / 4) {
+        result = ONE_GIGABYTE / 4;
+      } else {
+        result = avail;
+      }
+    }
+    if (log_guess && ! notice_sent) {
+      log_notice(LD_CONFIG, "%sMaxMemInQueues is set to "U64_FORMAT" MB. "
+                 "You can override this by setting MaxMemInQueues by hand.",
+                 ram ? "Based on detected system memory, " : "",
+                 U64_PRINTF_ARG(result / ONE_MEGABYTE));
+      notice_sent = 1;
+    }
+    return result;
+  } else if (val < ONE_GIGABYTE / 4) {
+    log_warn(LD_CONFIG, "MaxMemInQueues must be at least 256 MB for now. "
+             "Ideally, have it as large as you can afford.");
+    return ONE_GIGABYTE / 4;
+  } else {
+    /* The value was fine all along */
+    return val;
+  }
+}
+
 /** Helper: return true iff s1 and s2 are both NULL, or both non-NULL
  * equal strings. */
 static int

src/or/control.c

@@ -1504,6 +1504,9 @@ getinfo_helper_misc(control_connection_t *conn, const char *question,
     int max_fds=-1;
     set_max_file_descriptors(0, &max_fds);
     tor_asprintf(answer, "%d", max_fds);
+  } else if (!strcmp(question, "limits/max-mem-in-queues")) {
+    tor_asprintf(answer, U64_FORMAT,
+                 U64_PRINTF_ARG(get_options()->MaxMemInQueues));
   } else if (!strcmp(question, "dir-usage")) {
     *answer = directory_dump_request_log();
   } else if (!strcmp(question, "fingerprint")) {
@@ -2184,6 +2187,7 @@ static const getinfo_item_t getinfo_items[] = {
   ITEM("process/user", misc,
        "Username under which the tor process is running."),
   ITEM("process/descriptor-limit", misc, "File descriptor limit."),
+  ITEM("limits/max-mem-in-queues", misc, "Actual limit on memory in queues"),
   ITEM("dir-usage", misc, "Breakdown of bytes transferred over DirPort."),
   PREFIX("desc-annotations/id/", dir, "Router annotations by hexdigest."),
   PREFIX("dir/server/", dir,"Router descriptors as retrieved from a DirPort."),

src/or/main.c

@@ -2767,6 +2767,7 @@ sandbox_init_filter(void)
       tor_strdup("/dev/urandom"),
       tor_strdup("/dev/random"),
       tor_strdup("/etc/hosts"),
+      tor_strdup("/proc/meminfo"),
       NULL, 0
   );
   if (options->ServerDNSResolvConfFile)

src/or/or.h

@@ -3491,7 +3491,10 @@ typedef struct {
   config_line_t *DirPort_lines;
   config_line_t *DNSPort_lines; /**< Ports to listen on for DNS requests. */
 
-  uint64_t MaxMemInQueues; /**< If we have more memory than this allocated
+  /* MaxMemInQueues value as input by the user. We clean this up to be
+   * MaxMemInQueues. */
+  uint64_t MaxMemInQueues_raw;
+  uint64_t MaxMemInQueues;/**< If we have more memory than this allocated
                             * for queues and buffers, run the OOM handler */
 
   /** @name port booleans

src/test/test_util.c

@@ -3612,6 +3612,34 @@ test_util_socketpair(void *arg)
     tor_close_socket(fds[1]);
 }
 
+static void
+test_util_max_mem(void *arg)
+{
+  size_t memory1, memory2;
+  int r, r2;
+  (void) arg;
+
+  r = get_total_system_memory(&memory1);
+  r2 = get_total_system_memory(&memory2);
+  tt_int_op(r, ==, r2);
+  tt_int_op(memory2, ==, memory1);
+
+  TT_BLATHER(("System memory: "U64_FORMAT, U64_PRINTF_ARG(memory1)));
+
+  if (r==0) {
+    /* You have at least a megabyte. */
+    tt_int_op(memory1, >, (1<<20));
+  } else {
+    /* You do not have a petabyte. */
+#if SIZEOF_SIZE_T == SIZEOF_UINT64_T
+    tt_int_op(memory1, <, (U64_LITERAL(1)<<50));
+#endif
+  }
+
+ done:
+  ;
+}
+
 struct testcase_t util_tests[] = {
   UTIL_LEGACY(time),
   UTIL_TEST(parse_http_time, 0),
@@ -3675,6 +3703,7 @@ struct testcase_t util_tests[] = {
     (void*)"0" },
   { "socketpair_ersatz", test_util_socketpair, TT_FORK,
     &socketpair_setup, (void*)"1" },
+  UTIL_TEST(max_mem, 0),
   END_OF_TESTCASES
 };