diff --git a/changes/bug6174 b/changes/bug6174 new file mode 100644 index 000000000..79d2930ec --- /dev/null +++ b/changes/bug6174 @@ -0,0 +1,6 @@ + o Major bugfixes: + - When we mark a circuit as unusable for new circuits, have it + continue to be unusable for new circuits even if MaxCircuitDirtiness + is increased too much at the wrong time, or the system clock jumped + backwards. Fix for bug 6174; bugfix on 0.0.2pre26. + diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index ef3268073..ae0955d12 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1204,6 +1204,7 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, if ((!need_uptime || circ->build_state->need_uptime) && (!need_capacity || circ->build_state->need_capacity) && (internal == circ->build_state->is_internal) && + !circ->unusable_for_new_conns && circ->remaining_relay_early_cells && circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN && !circ->build_state->onehop_tunnel && @@ -1304,15 +1305,13 @@ void circuit_expire_all_dirty_circs(void) { circuit_t *circ; - const or_options_t *options = get_options(); for (circ=global_circuitlist; circ; circ = circ->next) { if (CIRCUIT_IS_ORIGIN(circ) && !circ->marked_for_close && - circ->timestamp_dirty) - /* XXXX024 This is a screwed-up way to say "This is too dirty - * for new circuits. */ - circ->timestamp_dirty -= options->MaxCircuitDirtiness; + circ->timestamp_dirty) { + mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ)); + } } } diff --git a/src/or/circuituse.c b/src/or/circuituse.c index c0612039b..7ee58fc5f 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -85,10 +85,14 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ, } if (purpose == CIRCUIT_PURPOSE_C_GENERAL || - purpose == CIRCUIT_PURPOSE_C_REND_JOINED) + purpose == CIRCUIT_PURPOSE_C_REND_JOINED) { if (circ->timestamp_dirty && circ->timestamp_dirty+get_options()->MaxCircuitDirtiness <= now) return 0; + } + + if (origin_circ->unusable_for_new_conns) + return 0; /* decide if this circ is suitable for this conn */ @@ -798,9 +802,12 @@ circuit_stream_is_being_handled(entry_connection_t *conn, circ->purpose == CIRCUIT_PURPOSE_C_GENERAL && (!circ->timestamp_dirty || circ->timestamp_dirty + get_options()->MaxCircuitDirtiness > now)) { - cpath_build_state_t *build_state = TO_ORIGIN_CIRCUIT(circ)->build_state; + origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); + cpath_build_state_t *build_state = origin_circ->build_state; if (build_state->is_internal || build_state->onehop_tunnel) continue; + if (!origin_circ->unusable_for_new_conns) + continue; exitnode = build_state_get_exit_node(build_state); if (exitnode && (!need_uptime || build_state->need_uptime)) { @@ -842,6 +849,7 @@ circuit_predict_and_launch_new(void) /* First, count how many of each type of circuit we have already. */ for (circ=global_circuitlist;circ;circ = circ->next) { cpath_build_state_t *build_state; + origin_circuit_t *origin_circ; if (!CIRCUIT_IS_ORIGIN(circ)) continue; if (circ->marked_for_close) @@ -850,7 +858,10 @@ circuit_predict_and_launch_new(void) continue; /* only count clean circs */ if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL) continue; /* only pay attention to general-purpose circs */ - build_state = TO_ORIGIN_CIRCUIT(circ)->build_state; + origin_circ = TO_ORIGIN_CIRCUIT(circ); + if (origin_circ->unusable_for_new_conns) + continue; + build_state = origin_circ->build_state; if (build_state->onehop_tunnel) continue; num++; @@ -2272,3 +2283,22 @@ circuit_change_purpose(circuit_t *circ, uint8_t new_purpose) } } +/** Mark circ so that no more connections can be attached to it. */ +void +mark_circuit_unusable_for_new_conns(origin_circuit_t *circ) +{ + const or_options_t *options = get_options(); + tor_assert(circ); + + /* XXXX025 This is a kludge; we're only keeping it around in case there's + * something that doesn't check unusable_for_new_conns, and to avoid + * deeper refactoring of our expiration logic. */ + if (! circ->base_.timestamp_dirty) + circ->base_.timestamp_dirty = approx_time(); + if (options->MaxCircuitDirtiness >= circ->base_.timestamp_dirty) + circ->base_.timestamp_dirty = 1; /* prevent underflow */ + else + circ->base_.timestamp_dirty -= options->MaxCircuitDirtiness; + + circ->unusable_for_new_conns = 1; +} diff --git a/src/or/circuituse.h b/src/or/circuituse.h index d4d68aad9..11e5a6416 100644 --- a/src/or/circuituse.h +++ b/src/or/circuituse.h @@ -55,6 +55,7 @@ void circuit_change_purpose(circuit_t *circ, uint8_t new_purpose); int hostname_in_track_host_exits(const or_options_t *options, const char *address); +void mark_circuit_unusable_for_new_conns(origin_circuit_t *circ); #endif diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index fcbcb9591..76c5f6a9f 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -674,12 +674,10 @@ connection_ap_expire_beginning(void) /* un-mark it as ending, since we're going to reuse it */ conn->edge_has_sent_end = 0; conn->end_reason = 0; - /* kludge to make us not try this circuit again, yet to allow - * current streams on it to survive if they can: make it - * unattractive to use for new streams */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->timestamp_dirty); - circ->timestamp_dirty -= options->MaxCircuitDirtiness; + /* make us not try this circuit again, but allow + * current streams on it to survive if they can */ + mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ)); + /* give our stream another 'cutoff' seconds to try */ conn->base_.timestamp_lastread += cutoff; if (entry_conn->num_socks_retries < 250) /* avoid overflow */ @@ -1806,9 +1804,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn) connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); return -1; } @@ -1899,9 +1895,7 @@ connection_ap_handshake_send_resolve(entry_connection_t *ap_conn) connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); return -1; } diff --git a/src/or/or.h b/src/or/or.h index 45eb4673c..e5c60452f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2944,6 +2944,10 @@ typedef struct origin_circuit_t { */ ENUM_BF(path_state_t) path_state : 3; + /* If this flag is set, we should not consider attaching any more + * connections to this circuit. */ + unsigned int unusable_for_new_conns : 1; + /** * Tristate variable to guard against pathbias miscounting * due to circuit purpose transitions changing the decision diff --git a/src/or/relay.c b/src/or/relay.c index 9ff9e1e1f..fb15141ef 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -17,6 +17,7 @@ #include "channel.h" #include "circuitbuild.h" #include "circuitlist.h" +#include "circuituse.h" #include "config.h" #include "connection.h" #include "connection_edge.h" @@ -851,9 +852,7 @@ connection_ap_process_end_not_open( /* We haven't retried too many times; reattach the connection. */ circuit_log_path(LOG_INFO,LD_APP,circ); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); if (conn->chosen_exit_optional) { /* stop wanting a specific exit */