July 2016 - tor-commits - lists.torproject.org

[tor/master] Fix an integer overflow related to monotonic time on windows.
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit d97fca16d0b37723c2cbe2bacf86601af0e24805 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 11:23:58 2016 -0400 Fix an integer overflow related to monotonic time on windows. To maintain precision, to get nanoseconds, we were multiplying our tick count by a billion, then dividing by ticks-per-second. But that apparently isn't such a great idea, since ticks-per-second is sometimes a billion on its own, so our intermediate result was giving us attoseconds. When you're counting in attoseconds, you can only fit about 9 seconds into an int64_t, which is not so great for our purposes. Instead, we now simplify the 1000000000/1000000000 fraction before we start messing with nanoseconds. This has potential to mess us up if some future MS version declares that performance counters will use 1,000,000,007 units per second, but let's burn that bridge when we come to it. --- src/common/compat_time.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 4757225..7676e3a 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -374,8 +374,10 @@ monotime_diff_nsec(const monotime_t *start, /* end of "HAVE_CLOCK_GETTIME" */ #elif defined (_WIN32) -/** Result of QueryPerformanceFrequency, as an int64_t. */ -static int64_t ticks_per_second = 0; +/** Result of QueryPerformanceFrequency, in terms needed to + * convert ticks to nanoseconds. */ +static int64_t nsec_per_tick_numer = 1; +static int64_t nsec_per_tick_denom = 1; /** Lock to protect last_pctr and pctr_offset */ static CRITICAL_SECTION monotime_lock; @@ -397,7 +399,17 @@ monotime_init_internal(void) ok = QueryPerformanceFrequency(&li); tor_assert(ok); tor_assert(li.QuadPart); - ticks_per_second = li.QuadPart; + + uint64_t n = ONE_BILLION; + uint64_t d = li.QuadPart; + /* We need to simplify this or we'll probably overflow the int64. */ + simplify_fraction64(&n, &d); + tor_assert(n <= INT64_MAX); + tor_assert(d <= INT64_MAX); + + nsec_per_tick_numer = (int64_t) n; + nsec_per_tick_denom = (int64_t) d; + last_pctr = 0; pctr_offset = 0; @@ -418,7 +430,8 @@ monotime_get(monotime_t *out) #ifdef TOR_UNIT_TESTS if (monotime_mocking_enabled) { - out->pcount_ = (mock_time_nsec * ticks_per_second) / ONE_BILLION; + out->pcount_ = (mock_time_nsec * nsec_per_tick_denom) + / nsec_per_tick_numer; return; } #endif @@ -465,7 +478,7 @@ monotime_diff_nsec(const monotime_t *start, monotime_init(); } const int64_t diff_ticks = end->pcount_ - start->pcount_; - return (diff_ticks * ONE_BILLION) / ticks_per_second; + return (diff_ticks * nsec_per_tick_numer) / nsec_per_tick_denom; } int64_t

1 0

[tor/master] Add a function to simplify a fraction.
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 09c25697d74cdfdefe5fa365e4691cc9401e2129 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 11:23:34 2016 -0400 Add a function to simplify a fraction. Apparently remembering euclid's algorithm does pay off sooner or later. --- src/common/util.c | 23 +++++++++++++++++++++++ src/common/util.h | 2 ++ src/test/test_util.c | 26 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/common/util.c b/src/common/util.c index d936156..5a73098 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -580,6 +580,29 @@ add_laplace_noise(int64_t signal, double random, double delta_f, return signal + noise; } +/* Helper: return greatest common divisor of a,b */ +static uint64_t +gcd64(uint64_t a, uint64_t b) +{ + while (b) { + uint64_t t = b; + b = a % b; + a = t; + } + return a; +} + +/* Given a fraction *numer / *denom, simplify it. + * Requires that the denominator is greater than 0. */ +void +simplify_fraction64(uint64_t *numer, uint64_t *denom) +{ + tor_assert(denom); + uint64_t gcd = gcd64(*numer, *denom); + *numer /= gcd; + *denom /= gcd; +} + /** Return the number of bits set in v. */ int n_bits_set_u8(uint8_t v) diff --git a/src/common/util.h b/src/common/util.h index a6638aa..9face0c 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -152,6 +152,8 @@ int64_t add_laplace_noise(int64_t signal, double random, double delta_f, double epsilon); int n_bits_set_u8(uint8_t v); int64_t clamp_double_to_int64(double number); +void simplify_fraction64(uint64_t *numer, uint64_t *denom); + /* Compute the CEIL of a divided by b, for nonnegative a * and positive b. Works on integer types only. Not defined if a+b can diff --git a/src/test/test_util.c b/src/test/test_util.c index 38df7bb..e173569 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4657,6 +4657,31 @@ test_util_mathlog(void *arg) } static void +test_util_fraction(void *arg) +{ + uint64_t a,b; + (void)arg; + + a = 99; b = 30; + simplify_fraction64(&a,&b); + tt_u64_op(a, OP_EQ, 33); + tt_u64_op(b, OP_EQ, 10); + + a = 3000000; b = 10000000; + simplify_fraction64(&a,&b); + tt_u64_op(a, OP_EQ, 3); + tt_u64_op(b, OP_EQ, 10); + + a = 0; b = 15; + simplify_fraction64(&a,&b); + tt_u64_op(a, OP_EQ, 0); + tt_u64_op(b, OP_EQ, 1); + + done: + ; +} + +static void test_util_round_to_next_multiple_of(void *arg) { (void)arg; @@ -5479,6 +5504,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(read_file_eof_zero_bytes, 0), UTIL_TEST(write_chunks_to_file, 0), UTIL_TEST(mathlog, 0), + UTIL_TEST(fraction, 0), UTIL_TEST(weak_random, 0), { "socket_ipv4", test_util_socket, TT_FORK, &passthrough_setup, (void*)"4" },

1 0

[tor/master] fix identifier mistake :(
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 019b7ddb9f9599f7040b1477b47956626c88bb45 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 10:44:51 2016 -0400 fix identifier mistake :( --- src/common/compat_time.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index bdbf2b5..0d28439 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -484,7 +484,7 @@ monotime_diff_nsec(const monotime_t *start, #ifdef _WIN32 printf(I64_FORMAT" res\n", I64_PRINTF_ARG(result)); #endif - return res; + return result; } int64_t

1 0

[tor/master] Redux^3: Temporarily add windows verbosity to track down jenkins failures
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 160d2c6aab6016b0e5774a3b909dd8c03bb2467a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 08:56:55 2016 -0400 Redux^3: Temporarily add windows verbosity to track down jenkins failures --- src/common/compat_time.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 1ff5cc1..bdbf2b5 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -471,20 +471,20 @@ int64_t monotime_diff_nsec(const monotime_t *start, const monotime_t *end) { -#ifdef _WIN32 - puts("m_d_n: 1"); -#endif if (BUG(monotime_initialized == 0)) { monotime_init(); } -#ifdef _WIN32 - puts("m_d_n: 2"); -#endif const int64_t diff_ticks = end->pcount_ - start->pcount_; #ifdef _WIN32 puts("m_d_n: 3"); + printf(I64_FORMAT" diff_ticks\n", I64_PRINTF_ARG(diff_ticks)); + printf(I64_FORMAT" ticks per second\n", I64_PRINTF_ARG(ticks_per_second)); +#endif + const int64_t result = (diff_ticks * ONE_BILLION) / ticks_per_second; +#ifdef _WIN32 + printf(I64_FORMAT" res\n", I64_PRINTF_ARG(result)); #endif - return (diff_ticks * ONE_BILLION) / ticks_per_second; + return res; } int64_t

1 0

[tor/master] Redux^2: Temporarily add windows verbosity to track down jenkins failures
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 0cef69713c16f2ecf428c952cd909b484ed0212a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 08:56:55 2016 -0400 Redux^2: Temporarily add windows verbosity to track down jenkins failures --- src/common/compat_time.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index da30a41..1ff5cc1 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -312,7 +312,7 @@ int64_t monotime_diff_nsec(const monotime_t *start, const monotime_t *end) { - if (BUG(mach_time_info.denom == 0)) { +n if (BUG(mach_time_info.denom == 0)) { monotime_init(); } const int64_t diff_ticks = end->abstime_ - start->abstime_; @@ -471,10 +471,19 @@ int64_t monotime_diff_nsec(const monotime_t *start, const monotime_t *end) { +#ifdef _WIN32 + puts("m_d_n: 1"); +#endif if (BUG(monotime_initialized == 0)) { monotime_init(); } +#ifdef _WIN32 + puts("m_d_n: 2"); +#endif const int64_t diff_ticks = end->pcount_ - start->pcount_; +#ifdef _WIN32 + puts("m_d_n: 3"); +#endif return (diff_ticks * ONE_BILLION) / ticks_per_second; } @@ -587,11 +596,20 @@ uint64_t monotime_absolute_nsec(void) { monotime_t now; +#ifdef _WIN32 + puts("m_a_n: 1"); +#endif if (BUG(monotime_initialized == 0)) { monotime_init(); } +#ifdef _WIN32 + puts("m_a_n: 2"); +#endif monotime_get(&now); +#ifdef _WIN32 + puts("m_a_n: 3"); +#endif return monotime_diff_nsec(&initialized_at, &now); }

1 0

[tor/master] Fix integer overflow in the rate-limiter (#19435).
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 77459b97aac15949c5160ca8abb9af792f02ac73 Author: Ivan Markin <twim(a)riseup.net> Date: Fri Jun 17 03:44:58 2016 +0000 Fix integer overflow in the rate-limiter (#19435). --- changes/bug19435 | 6 ++++++ src/common/util.c | 14 ++++++++++++-- src/common/util.h | 1 + 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/changes/bug19435 b/changes/bug19435 new file mode 100644 index 0000000..ccd916b --- /dev/null +++ b/changes/bug19435 @@ -0,0 +1,6 @@ + o Major bugfixes (user interface): + - Fix an integer overflow in the rate-limiter that caused displaying of + wrong number of suppressed messages (if there are too many of them). + If the number of messages hits the limit of messages per interval the + rate-limiter drops a warning and doesn't count any further. + Fixes bug 19435. diff --git a/src/common/util.c b/src/common/util.c index 538aeb1..72efd89 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1983,7 +1983,9 @@ update_approx_time(time_t now) /** If the rate-limiter lim is ready at now, return the number * of calls to rate_limit_is_ready (including this one!) since the last time - * rate_limit_is_ready returned nonzero. Otherwise return 0. */ + * rate_limit_is_ready returned nonzero. Otherwise return 0. + * If the call number hits RATELIM_TOOMANY limit, drop a warning + * about this event and stop counting. */ static int rate_limit_is_ready(ratelim_t *lim, time_t now) { @@ -1993,7 +1995,15 @@ rate_limit_is_ready(ratelim_t *lim, time_t now) lim->n_calls_since_last_time = 0; return res; } else { - ++lim->n_calls_since_last_time; + if (lim->n_calls_since_last_time < RATELIM_TOOMANY) { + ++lim->n_calls_since_last_time; + } else if (lim->n_calls_since_last_time == RATELIM_TOOMANY) { + log_warn(LD_GENERAL, + "Enormously large number of messages (%d). It's probably a bug.", + RATELIM_TOOMANY); + ++lim->n_calls_since_last_time; + } + return 0; } } diff --git a/src/common/util.h b/src/common/util.h index 0d48eac..837d2e9 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -292,6 +292,7 @@ typedef struct ratelim_t { } ratelim_t; #define RATELIM_INIT(r) { (r), 0, 0 } +#define RATELIM_TOOMANY (16*1000) char *rate_limit_log(ratelim_t *lim, time_t now);

1 0

[tor/master] Tweaks on 19435 fix:
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit fb7f90c181dc44ae9f23cb4d16cac25609463d9f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 09:59:48 2016 -0400 Tweaks on 19435 fix: * Raise limit: 16k isn't all that high. * Don't log when limit exceded; log later on. * Say "over" when we log more than we say we log. * Add target version to changes file --- changes/bug19435 | 4 ++-- src/common/util.c | 12 ++++-------- src/common/util.h | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/changes/bug19435 b/changes/bug19435 index ccd916b..d0a29d1 100644 --- a/changes/bug19435 +++ b/changes/bug19435 @@ -2,5 +2,5 @@ - Fix an integer overflow in the rate-limiter that caused displaying of wrong number of suppressed messages (if there are too many of them). If the number of messages hits the limit of messages per interval the - rate-limiter drops a warning and doesn't count any further. - Fixes bug 19435. + rate-limiter doesn't count any further. + Fixes bug 19435; bugfix on 0.2.4.11-alpha. diff --git a/src/common/util.c b/src/common/util.c index 72efd89..d936156 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1995,12 +1995,7 @@ rate_limit_is_ready(ratelim_t *lim, time_t now) lim->n_calls_since_last_time = 0; return res; } else { - if (lim->n_calls_since_last_time < RATELIM_TOOMANY) { - ++lim->n_calls_since_last_time; - } else if (lim->n_calls_since_last_time == RATELIM_TOOMANY) { - log_warn(LD_GENERAL, - "Enormously large number of messages (%d). It's probably a bug.", - RATELIM_TOOMANY); + if (lim->n_calls_since_last_time <= RATELIM_TOOMANY) { ++lim->n_calls_since_last_time; } @@ -2020,11 +2015,12 @@ rate_limit_log(ratelim_t *lim, time_t now) return tor_strdup(""); } else { char *cp=NULL; + const char *opt_over = (n >= RATELIM_TOOMANY) ? "over " : ""; /* XXXX this is not exactly correct: the messages could have occurred * any time between the old value of lim->allowed and now. */ tor_asprintf(&cp, - " [%d similar message(s) suppressed in last %d seconds]", - n-1, lim->rate); + " [%s%d similar message(s) suppressed in last %d seconds]", + opt_over, n-1, lim->rate); return cp; } } else { diff --git a/src/common/util.h b/src/common/util.h index 837d2e9..a6638aa 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -292,7 +292,7 @@ typedef struct ratelim_t { } ratelim_t; #define RATELIM_INIT(r) { (r), 0, 0 } -#define RATELIM_TOOMANY (16*1000) +#define RATELIM_TOOMANY (16*1000*1000) char *rate_limit_log(ratelim_t *lim, time_t now);

1 0

[tor/master] debugging: print ticks-per-second on windows. is it 0?
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 264fb7eb82f5df59247f1e2f4ea906fd9a0def61 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 09:44:41 2016 -0400 debugging: print ticks-per-second on windows. is it 0? --- src/common/compat_time.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 9fac0bc..da30a41 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -404,6 +404,7 @@ monotime_init_internal(void) tor_assert(li.QuadPart); puts("win32-mii-6"); ticks_per_second = li.QuadPart; + printf(I64_FORMAT" ticks per second\n", I64_PRINTF_ARG(ticks_per_second)); last_pctr = 0; pctr_offset = 0;

1 0

[tor/master] Redux: Temporarily add windows verbosity to track down jenkins failures
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit e77c23e455513ddb8f9fa5ad59f08fa1fa43cec6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 08:56:55 2016 -0400 Redux: Temporarily add windows verbosity to track down jenkins failures --- src/test/test_util.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index 38df7bb..2dfa109 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -5296,12 +5296,31 @@ test_util_monotonic_time(void *arg) uint64_t nsec1, nsec2, usec1, msec1; uint64_t nsecc1, nsecc2, usecc1, msecc1; +#ifdef _WIN32 + puts("Test-1"); +#endif + monotime_get(&mt1); +#ifdef _WIN32 + puts("Test-2"); +#endif monotime_coarse_get(&mtc1); +#ifdef _WIN32 + puts("Test-3"); +#endif nsec1 = monotime_absolute_nsec(); +#ifdef _WIN32 + puts("Test-4"); +#endif usec1 = monotime_absolute_usec(); msec1 = monotime_absolute_msec(); +#ifdef _WIN32 + puts("Test-5"); +#endif nsecc1 = monotime_coarse_absolute_nsec(); +#ifdef _WIN32 + puts("Test-6"); +#endif usecc1 = monotime_coarse_absolute_usec(); msecc1 = monotime_coarse_absolute_msec();

1 0

[tor/master] Temporarily add some windows verbosity to track down unit test failure on jenkins.
by nickm＠torproject.org 26 Jul '16

26 Jul '16

commit 1033713c9c2ef70be7caf847562cbb709ea7db29 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 26 08:56:55 2016 -0400 Temporarily add some windows verbosity to track down unit test failure on jenkins. --- src/common/compat_time.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 4757225..9fac0bc 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -388,24 +388,33 @@ static GetTickCount64_fn_t GetTickCount64_fn = NULL; static void monotime_init_internal(void) { + puts("win32-mii-1"); tor_assert(!monotime_initialized); BOOL ok = InitializeCriticalSectionAndSpinCount(&monotime_lock, 200); tor_assert(ok); + puts("win32-mii-2"); ok = InitializeCriticalSectionAndSpinCount(&monotime_coarse_lock, 200); tor_assert(ok); + puts("win32-mii-3"); LARGE_INTEGER li; ok = QueryPerformanceFrequency(&li); + puts("win32-mii-4"); tor_assert(ok); + puts("win32-mii-5"); tor_assert(li.QuadPart); + puts("win32-mii-6"); ticks_per_second = li.QuadPart; last_pctr = 0; pctr_offset = 0; + puts("win32-mii-7"); HANDLE h = load_windows_system_library(TEXT("kernel32.dll")); + puts("win32-mii-8"); if (h) { GetTickCount64_fn = (GetTickCount64_fn_t) GetProcAddress(h, "GetTickCount64"); } + puts("win32-mii-9"); // FreeLibrary(h) ? } @@ -541,10 +550,19 @@ monotime_init(void) if (!monotime_initialized) { monotime_init_internal(); monotime_initialized = 1; +#ifdef _WIN32 + puts("mi-0"); +#endif monotime_get(&initialized_at); +#ifdef _WIN32 + puts("mi-1"); +#endif #ifdef MONOTIME_COARSE_FN_IS_DIFFERENT monotime_coarse_get(&initialized_at_coarse); #endif +#ifdef _WIN32 + puts("mi-2"); +#endif } }

1 0