Merge branch 'maint-0.3.5' into bug31571_035

7 files changed, 72 insertions, 6 deletions

diff --git a/changes/bug30894 b/changes/bug30894
new file mode 100644
index 0000000000..64c14c4e6d
--- /dev/null
+++ b/changes/bug30894
@@ -0,0 +1,4 @@
+  o Minor bugfixes (memory leaks):
+    - Fix a trivial memory leak when parsing an invalid value
+      from a download schedule in the configuration. Fixes bug
+      30894; bugfix on 0.3.4.1-alpha.
diff --git a/changes/bug31003 b/changes/bug31003
new file mode 100644
index 0000000000..6c75163380
--- /dev/null
+++ b/changes/bug31003
@@ -0,0 +1,4 @@
+  o Minor bugfixes (crash on exit):
+    - Avoid a set of possible code paths that could use try to use freed memory
+      in routerlist_free() while Tor was exiting.  Fixes bug 31003; bugfix on
+      0.1.2.2-alpha.
diff --git a/changes/ticket30871 b/changes/ticket30871
new file mode 100644
index 0000000000..81c076bb02
--- /dev/null
+++ b/changes/ticket30871
@@ -0,0 +1,6 @@
+  o Major bugfixes (circuit build, guard):
+    - When considering upgrading circuits from "waiting for guard" to "open",
+      always ignore the ones that are mark for close. Else, we can end up in
+      the situation where a subsystem is notified of that circuit opening but
+      still marked for close leading to undesirable behavior. Fixes bug 30871;
+      bugfix on 0.3.0.1-alpha.
diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 8681f648da..729e7a4478 100644
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -225,6 +225,7 @@ config_assign_value(const config_format_t *fmt, void *options,
       tor_asprintf(msg,
           "Interval '%s %s' is malformed or out of bounds.",
           c->key, c->value);
+      tor_free(tmp);
       return -1;
     }
     *(int *)lvalue = i;
diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c
index 15ec830594..819f90a6d9 100644
--- a/src/feature/client/entrynodes.c
+++ b/src/feature/client/entrynodes.c
@@ -2611,6 +2611,10 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
     entry_guard_t *guard = entry_guard_handle_get(state->guard);
     if (!guard || guard->in_selection != gs)
       continue;
+    if (TO_CIRCUIT(circ)->marked_for_close) {
+      /* Don't consider any marked for close circuits. */
+      continue;
+    }
 
     smartlist_add(all_circuits, circ);
   } SMARTLIST_FOREACH_END(circ);
diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c
index 456f930aa3..e48675aada 100644
--- a/src/feature/nodelist/routerlist.c
+++ b/src/feature/nodelist/routerlist.c
@@ -954,20 +954,18 @@ routerlist_free_(routerlist_t *rl)
   smartlist_free(rl->routers);
   smartlist_free(rl->old_routers);
   if (rl->desc_store.mmap) {
-    int res = tor_munmap_file(routerlist->desc_store.mmap);
+    int res = tor_munmap_file(rl->desc_store.mmap);
     if (res != 0) {
       log_warn(LD_FS, "Failed to munmap routerlist->desc_store.mmap");
     }
   }
   if (rl->extrainfo_store.mmap) {
-    int res = tor_munmap_file(routerlist->extrainfo_store.mmap);
+    int res = tor_munmap_file(rl->extrainfo_store.mmap);
     if (res != 0) {
       log_warn(LD_FS, "Failed to munmap routerlist->extrainfo_store.mmap");
     }
   }
   tor_free(rl);
-
-  router_dir_info_changed();
 }
 
 /** Log information about how much memory is being used for routerlist,
@@ -1426,8 +1424,10 @@ routerlist_reparse_old(routerlist_t *rl, signed_descriptor_t *sd)
 void
 routerlist_free_all(void)
 {
-  routerlist_free(routerlist);
-  routerlist = NULL;
+  routerlist_t *rl = routerlist;
+  routerlist = NULL; // Prevent internals of routerlist_free() from using
+                     // routerlist.
+  routerlist_free(rl);
   dirlist_free_all();
   if (warned_nicknames) {
     SMARTLIST_FOREACH(warned_nicknames, char *, cp, tor_free(cp));
diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c
index 275f1eced2..538f20781f 100644
--- a/src/test/test_circuitbuild.c
+++ b/src/test/test_circuitbuild.c
@@ -4,6 +4,8 @@
 /* See LICENSE for licensing information */
 
 #define CIRCUITBUILD_PRIVATE
+#define CIRCUITLIST_PRIVATE
+#define ENTRYNODES_PRIVATE
 
 #include "core/or/or.h"
 #include "test/test.h"
@@ -13,7 +15,11 @@
 #include "core/or/circuitbuild.h"
 #include "core/or/circuitlist.h"
 
+#include "core/or/cpath_build_state_st.h"
 #include "core/or/extend_info_st.h"
+#include "core/or/origin_circuit_st.h"
+
+#include "feature/client/entrynodes.h"
 
 /* Dummy nodes smartlist for testing */
 static smartlist_t dummy_nodes;
@@ -126,10 +132,51 @@ test_new_route_len_unhandled_exit(void *arg)
   UNMOCK(count_acceptable_nodes);
 }
 
+static void
+test_upgrade_from_guard_wait(void *arg)
+{
+  circuit_t *circ = NULL;
+  origin_circuit_t *orig_circ = NULL;
+  entry_guard_t *guard = NULL;
+  smartlist_t *list = NULL;
+
+  (void) arg;
+
+  circ = dummy_origin_circuit_new(0);
+  orig_circ = TO_ORIGIN_CIRCUIT(circ);
+  tt_assert(orig_circ);
+
+  orig_circ->build_state = tor_malloc_zero(sizeof(cpath_build_state_t));
+
+  circuit_set_state(circ, CIRCUIT_STATE_GUARD_WAIT);
+
+  /* Put it in guard wait state. */
+  guard = tor_malloc_zero(sizeof(*guard));
+  guard->in_selection = get_guard_selection_info();
+
+  orig_circ->guard_state =
+    circuit_guard_state_new(guard, GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD,
+                            NULL);
+
+  /* Mark the circuit for close. */
+  circuit_mark_for_close(circ, END_CIRC_REASON_TORPROTOCOL);
+  tt_int_op(circ->marked_for_close, OP_NE, 0);
+
+  /* We shouldn't pick the mark for close circuit. */
+  list = circuit_find_circuits_to_upgrade_from_guard_wait();
+  tt_assert(!list);
+
+ done:
+  circuit_free(circ);
+  entry_guard_free_(guard);
+}
+
 struct testcase_t circuitbuild_tests[] = {
   { "noexit", test_new_route_len_noexit, 0, NULL, NULL },
   { "safe_exit", test_new_route_len_safe_exit, 0, NULL, NULL },
   { "unsafe_exit", test_new_route_len_unsafe_exit, 0, NULL, NULL },
   { "unhandled_exit", test_new_route_len_unhandled_exit, 0, NULL, NULL },
+  { "upgrade_from_guard_wait", test_upgrade_from_guard_wait, TT_FORK,
+    NULL, NULL },
   END_OF_TESTCASES
 };