From dc2f10bd81cfcfdd65bad38a34c042259b1c1ece Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 6 Dec 2010 12:01:32 -0500 Subject: [PATCH] Fix a bug in calculating wakeup time on 64-bit machines. If you had TIME_MAX > INT_MAX, and your "time_to_exhaust_bw = accountingmax/expected_bandwidth_usage * 60" calculation managed to overflow INT_MAX, then your time_to_consider value could underflow and wind up being rediculously low or high. "Low" was no problem; negative values got caught by the (time_to_consider <= 0) check. "High", however, would get you a wakeup time somewhere in the distant future. The fix is to check for time_to_exhaust_bw overflowing INT_MAX, not TIME_MAX: We don't allow any accounting interval longer than a month, so if time_to_exhaust_bw is significantly larger than 31*24*60*60, we can just clip it. This is a bugfix on 0.0.9pre6, when accounting was first introduced. It fixes bug 2146, unless there are other causes there too. The fix is from boboper. (I tweaked it slightly by removing an assignment that boboper marked as dead, and lowering a variable that no longer needed to be function-scoped.) --- changes/bug2146.1 | 4 ++++ src/or/hibernate.c | 9 +++------ 2 files changed, 7 insertions(+), 6 deletions(-) create mode 100644 changes/bug2146.1 diff --git a/changes/bug2146.1 b/changes/bug2146.1 new file mode 100644 index 0000000000..5b91c59a68 --- /dev/null +++ b/changes/bug2146.1 @@ -0,0 +1,4 @@ + - Major bugfixes: + o Fix a bug that could break accounting on 64-bit systems with large + time_t values, making them hibernate for impossibly long intervals. + Bugfix on 0.0.9pre6; fix for bug 2146; fix by boboper. diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 356f9a0f55..929c1994c3 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -515,7 +515,6 @@ accounting_run_housekeeping(time_t now) static void accounting_set_wakeup_time(void) { - char buf[ISO_TIME_LEN+1]; char digest[DIGEST_LEN]; crypto_digest_env_t *d_env; int time_in_interval; @@ -530,6 +529,7 @@ accounting_set_wakeup_time(void) } if (server_identity_key_is_set()) { + char buf[ISO_TIME_LEN+1]; format_iso_time(buf, interval_start_time); crypto_pk_get_digest(get_server_identity_key(), digest); @@ -548,7 +548,6 @@ accounting_set_wakeup_time(void) char buf2[ISO_TIME_LEN+1]; format_local_iso_time(buf1, interval_start_time); format_local_iso_time(buf2, interval_end_time); - time_to_exhaust_bw = GUESS_TIME_TO_USE_BANDWIDTH; interval_wakeup_time = interval_start_time; log_notice(LD_ACCT, @@ -563,8 +562,8 @@ accounting_set_wakeup_time(void) time_to_exhaust_bw = (get_options()->AccountingMax/expected_bandwidth_usage)*60; - if (time_to_exhaust_bw > TIME_MAX) { - time_to_exhaust_bw = TIME_MAX; + if (time_to_exhaust_bw > INT_MAX) { + time_to_exhaust_bw = INT_MAX; time_to_consider = 0; } else { time_to_consider = time_in_interval - (int)time_to_exhaust_bw; @@ -582,8 +581,6 @@ accounting_set_wakeup_time(void) * to be chosen than the last half. */ interval_wakeup_time = interval_start_time + (get_uint32(digest) % time_to_consider); - - format_iso_time(buf, interval_wakeup_time); } {