[tor-commits] [tor/release-0.3.4] Avoid integer overflow on fast 32-bit millisecond conversion.

[nickm at torproject.org]

commit f02e8b5944c238979c0c0cf3ce4a8911026b2210
Author: Nick Mathewson <nickm at torproject.org>
Date:   Fri Sep 14 08:36:33 2018 -0400

    Avoid integer overflow on fast 32-bit millisecond conversion.
    
    Multiply-then-divide is more accurate, but it runs into trouble when
    our input is above INT32_MAX/numerator.  So when our value is too
    large, do divide-then-multiply instead.
    
    Fixes part of bug 27139; bugfix on 0.3.4.1-alpha.
---
 src/common/compat_time.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/common/compat_time.c b/src/common/compat_time.c
index f92dc09c4..93b527def 100644
--- a/src/common/compat_time.c
+++ b/src/common/compat_time.c
@@ -280,6 +280,7 @@ monotime_reset_ratchets_for_testing(void)
  */
 static struct mach_timebase_info mach_time_info;
 static struct mach_timebase_info mach_time_info_msec_cvt;
+static int32_t mach_time_msec_cvt_threshold;
 static int monotime_shift = 0;
 
 static void
@@ -304,6 +305,10 @@ monotime_init_internal(void)
     // denominator here to avoid multiple multiplies.
     mach_time_info_msec_cvt.numer = mach_time_info.numer * 2048;
     mach_time_info_msec_cvt.denom = mach_time_info.denom * 1953;
+    // For any value above this amount, we should divide before multiplying,
+    // to avoid overflow.  For a value below this, we should multiply
+    // before dividing, to improve accuracy.
+    mach_time_msec_cvt_threshold = INT32_MAX / mach_time_info_msec_cvt.numer;
   }
 }
 
@@ -366,8 +371,13 @@ monotime_coarse_diff_msec32_(const monotime_coarse_t *start,
   /* We already require in di_ops.c that right-shift performs a sign-extend. */
   const int32_t diff_microticks = (int32_t)(diff_ticks >> 20);
 
-  return (diff_microticks * mach_time_info_msec_cvt.numer) /
-    mach_time_info_msec_cvt.denom;
+  if (diff_microticks >= mach_time_msec_cvt_threshold) {
+    return (diff_microticks / mach_time_info_msec_cvt.denom) *
+      mach_time_info_msec_cvt.numer;
+  } else {
+    return (diff_microticks * mach_time_info_msec_cvt.numer) /
+      mach_time_info_msec_cvt.denom;
+  }
 }
 
 uint32_t