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 };