From f7e6e852e80c22b40a8f09bc1c85074726d7078e Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Tue, 29 Sep 2009 03:41:23 -0700 Subject: [PATCH] Fix 1108: Handle corrupt or large build times state. 1108 was actually just a fencepost error in an assert, but making the state file handling code resilient is a good idea. --- ChangeLog | 5 ++++ src/or/circuitbuild.c | 65 ++++++++++++++++++++++++++++++------------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index c79c865efa..05e4373b38 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,9 @@ Changes in version 0.2.2.4-alpha - 2009-??-?? + o Major bugfixes: + - Fix another assert in the circuit_build_times code that causes Tor + to fail to start once we have accumulated 5000 build times in the + state file. Bugfix on 0.2.2.2-alpha; fixes bug 1108. + o Minor features: - Log SSL state transitions at debug level during handshake, and include SSL states in error messages. This may help debug diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 5c3a86c123..2e3465d6fe 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -372,18 +372,29 @@ circuit_build_times_update_state(circuit_build_times_t *cbt, * Stolen from http://en.wikipedia.org/wiki/Fisher\u2013Yates_shuffle */ static void -circuit_build_times_shuffle_array(circuit_build_times_t *cbt) +circuit_build_times_shuffle_and_store_array(circuit_build_times_t *cbt, + build_time_t *raw_times, + int num_times) { - int n = cbt->total_build_times; + int n = num_times; + if (num_times > NCIRCUITS_TO_OBSERVE) { + log_notice(LD_CIRC, "Decreasing circuit_build_times size from %d to %d", + num_times, NCIRCUITS_TO_OBSERVE); + } - /* This code can only be run on a compact array */ - tor_assert(cbt->total_build_times == cbt->build_times_idx); - while (n-- > 1) { - int k = crypto_rand_int(n + 1); /* 0 <= k <= n. */ - build_time_t tmp = cbt->circuit_build_times[k]; - cbt->circuit_build_times[k] = cbt->circuit_build_times[n]; - cbt->circuit_build_times[n] = tmp; - } + /* This code can only be run on a compact array */ + while (n-- > 1) { + int k = crypto_rand_int(n + 1); /* 0 <= k <= n. */ + build_time_t tmp = raw_times[k]; + raw_times[k] = raw_times[n]; + raw_times[n] = tmp; + } + + /* Since the times are now shuffled, take a random NCIRCUITS_TO_OBSERVE + * subset (ie the first NCIRCUITS_TO_OBSERVE values) */ + for (n = 0; n < MIN(num_times, NCIRCUITS_TO_OBSERVE); n++) { + circuit_build_times_add_time(cbt, raw_times[n]); + } } /** @@ -397,14 +408,14 @@ int circuit_build_times_parse_state(circuit_build_times_t *cbt, or_state_t *state, char **msg) { - int tot_values = 0, N = 0; + int tot_values = 0; + uint32_t loaded_cnt = 0, N = 0; config_line_t *line; int i; - *msg = NULL; + build_time_t *loaded_times = tor_malloc(sizeof(build_time_t) + * state->TotalBuildTimes); circuit_build_times_init(cbt); - - /* We don't support decreasing the table size yet */ - tor_assert(state->TotalBuildTimes <= NCIRCUITS_TO_OBSERVE); + *msg = NULL; for (line = state->BuildtimeHistogram; line; line = line->next) { smartlist_t *args = smartlist_create(); @@ -441,17 +452,30 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, break; } + if (loaded_cnt+count > state->TotalBuildTimes) { + log_warn(LD_CIRC, + "Too many build times in state file. " + "Stopping short before %d", + loaded_cnt+count); + break; + } + for (k = 0; k < count; k++) { - circuit_build_times_add_time(cbt, ms); + loaded_times[loaded_cnt++] = ms; } N++; SMARTLIST_FOREACH(args, char*, cp, tor_free(cp)); smartlist_free(args); } - } - circuit_build_times_shuffle_array(cbt); + if (loaded_cnt != state->TotalBuildTimes) { + log_warn(LD_CIRC, + "Corrupt state file? Build times count mismatch. " + "Read %d, file says %d", loaded_cnt, state->TotalBuildTimes); + } + + circuit_build_times_shuffle_and_store_array(cbt, loaded_times, loaded_cnt); /* Verify that we didn't overwrite any indexes */ for (i=0; i < NCIRCUITS_TO_OBSERVE; i++) { @@ -462,9 +486,10 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, log_info(LD_CIRC, "Loaded %d/%d values from %d lines in circuit time histogram", tot_values, cbt->total_build_times, N); - tor_assert(cbt->total_build_times == state->TotalBuildTimes); - tor_assert(tot_values == cbt->total_build_times); + tor_assert(cbt->total_build_times == tot_values); + tor_assert(cbt->total_build_times <= NCIRCUITS_TO_OBSERVE); circuit_build_times_set_timeout(cbt); + tor_free(loaded_times); return *msg ? -1 : 0; }