Bug 1245: Ignore negative and large timeouts.

This should prevent some asserts and storage of incorrect build times
for the cases where Tor is suspended during a circuit construction, or
just after completing a circuit. The idea is that if the circuit
build time is much greater than we would have cut it off at, we probably
had a suspend event along this codepath, and we should discard the
value.
This commit is contained in:
Mike Perry 2010-05-07 15:42:57 -07:00
parent e40e35507e
commit 728e946efd
2 changed files with 28 additions and 9 deletions

View File

@ -308,9 +308,9 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n)
int int
circuit_build_times_add_time(circuit_build_times_t *cbt, build_time_t time) circuit_build_times_add_time(circuit_build_times_t *cbt, build_time_t time)
{ {
tor_assert(time <= CBT_BUILD_TIME_MAX); if (time <= 0 || time > CBT_BUILD_TIME_MAX) {
if (time <= 0) {
log_warn(LD_CIRC, "Circuit build time is %u!", time); log_warn(LD_CIRC, "Circuit build time is %u!", time);
tor_fragile_assert();
return -1; return -1;
} }
@ -1760,12 +1760,20 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
long timediff; long timediff;
tor_gettimeofday(&end); tor_gettimeofday(&end);
timediff = tv_mdiff(&circ->_base.highres_created, &end); timediff = tv_mdiff(&circ->_base.highres_created, &end);
if (timediff > INT32_MAX) /*
timediff = INT32_MAX; * If the circuit build time is much greater than we would have cut
* it off at, we probably had a suspend event along this codepath,
* and we should discard the value.
*/
if (timediff < 0 || timediff > 2*circ_times.timeout_ms+1000) {
log_notice(LD_CIRC, "Strange value for circuit build time: %ld. "
"Assuming clock jump.", timediff);
} else {
circuit_build_times_add_time(&circ_times, (build_time_t)timediff); circuit_build_times_add_time(&circ_times, (build_time_t)timediff);
circuit_build_times_network_circ_success(&circ_times); circuit_build_times_network_circ_success(&circ_times);
circuit_build_times_set_timeout(&circ_times); circuit_build_times_set_timeout(&circ_times);
} }
}
log_info(LD_CIRC,"circuit built!"); log_info(LD_CIRC,"circuit built!");
circuit_reset_failure_count(0); circuit_reset_failure_count(0);
if (circ->build_state->onehop_tunnel) if (circ->build_state->onehop_tunnel)

View File

@ -350,10 +350,21 @@ circuit_expire_building(time_t now)
} else { /* circuit not open, consider recording failure as timeout */ } else { /* circuit not open, consider recording failure as timeout */
int first_hop_succeeded = TO_ORIGIN_CIRCUIT(victim)->cpath && int first_hop_succeeded = TO_ORIGIN_CIRCUIT(victim)->cpath &&
TO_ORIGIN_CIRCUIT(victim)->cpath->state == CPATH_STATE_OPEN; TO_ORIGIN_CIRCUIT(victim)->cpath->state == CPATH_STATE_OPEN;
if (circuit_build_times_add_timeout(&circ_times, first_hop_succeeded, /*
victim->timestamp_created)) * If the circuit build time is much greater than we would have cut
* it off at, we probably had a suspend event along this codepath,
* and we should discard the value.
*/
if (now - victim->timestamp_created > (2*circ_times.timeout_ms)/1000+1) {
log_notice(LD_CIRC,
"Extremely large value for circuit build timeout: %ld. "
"Assuming clock jump.", now - victim->timestamp_created);
} else if (circuit_build_times_add_timeout(&circ_times,
first_hop_succeeded,
victim->timestamp_created)) {
circuit_build_times_set_timeout(&circ_times); circuit_build_times_set_timeout(&circ_times);
} }
}
if (victim->n_conn) if (victim->n_conn)
log_info(LD_CIRC,"Abandoning circ %s:%d:%d (state %d:%s, purpose %d)", log_info(LD_CIRC,"Abandoning circ %s:%d:%d (state %d:%s, purpose %d)",