OSDN Git Service

Gracefully handle integer overflows.
authorJeff Sharkey <jsharkey@android.com>
Fri, 1 Sep 2017 18:51:51 +0000 (12:51 -0600)
committerJeff Sharkey <jsharkey@android.com>
Fri, 1 Sep 2017 23:13:01 +0000 (17:13 -0600)
Try sticking with integer-based math as much as possible for speed,
but switch to double-based math if we detect that we'd end up
causing an overflow.

New tests to verify.

Test: bit FrameworksNetTests:com.android.server.net.NetworkStatsCollectionTest
Bug: 65257769
Change-Id: I1ae35599be134f81850c0a3d86928b057fba1eff

services/core/java/com/android/server/net/NetworkStatsCollection.java
tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java

index 8837c15..4ceb592 100644 (file)
@@ -177,6 +177,34 @@ public class NetworkStatsCollection implements FileRotator.Reader {
         }
     }
 
+    /**
+     * Safely multiple a value by a rational.
+     * <p>
+     * Internally it uses integer-based math whenever possible, but switches
+     * over to double-based math if values would overflow.
+     */
+    @VisibleForTesting
+    public static long multiplySafe(long value, long num, long den) {
+        long x = value;
+        long y = num;
+
+        // Logic shamelessly borrowed from Math.multiplyExact()
+        long r = x * y;
+        long ax = Math.abs(x);
+        long ay = Math.abs(y);
+        if (((ax | ay) >>> 31 != 0)) {
+            // Some bits greater than 2^31 that might cause overflow
+            // Check the result using the divide operator
+            // and check for the special case of Long.MIN_VALUE * -1
+            if (((y != 0) && (r / y != x)) ||
+                    (x == Long.MIN_VALUE && y == -1)) {
+                // Use double math to avoid overflowing
+                return (long) (((double) num / den) * value);
+            }
+        }
+        return r / den;
+    }
+
     public int[] getRelevantUids(@NetworkStatsAccess.Level int accessLevel) {
         return getRelevantUids(accessLevel, Binder.getCallingUid());
     }
@@ -274,8 +302,8 @@ public class NetworkStatsCollection implements FileRotator.Reader {
             final long rawRxBytes = entry.rxBytes;
             final long rawTxBytes = entry.txBytes;
             final long targetBytes = augmentPlan.getDataUsageBytes();
-            final long targetRxBytes = (rawRxBytes * targetBytes) / rawBytes;
-            final long targetTxBytes = (rawTxBytes * targetBytes) / rawBytes;
+            final long targetRxBytes = multiplySafe(targetBytes, rawRxBytes, rawBytes);
+            final long targetTxBytes = multiplySafe(targetBytes, rawTxBytes, rawBytes);
 
             // Scale all matching buckets to reach anchor target
             final long beforeTotal = combined.getTotalBytes();
@@ -283,8 +311,8 @@ public class NetworkStatsCollection implements FileRotator.Reader {
                 combined.getValues(i, entry);
                 if (entry.bucketStart >= augmentStart
                         && entry.bucketStart + entry.bucketDuration <= augmentEnd) {
-                    entry.rxBytes = (entry.rxBytes * targetRxBytes) / rawRxBytes;
-                    entry.txBytes = (entry.txBytes * targetTxBytes) / rawTxBytes;
+                    entry.rxBytes = multiplySafe(targetRxBytes, entry.rxBytes, rawRxBytes);
+                    entry.txBytes = multiplySafe(targetTxBytes, entry.txBytes, rawTxBytes);
                     // We purposefully clear out packet counters to indicate
                     // that this data has been augmented.
                     entry.rxPackets = 0;
index 0c2ad38..9c10264 100644 (file)
@@ -27,7 +27,10 @@ import static android.os.Process.myUid;
 import static android.text.format.DateUtils.HOUR_IN_MILLIS;
 import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 
+import static com.android.server.net.NetworkStatsCollection.multiplySafe;
+
 import android.content.res.Resources;
+import android.net.ConnectivityManager;
 import android.net.NetworkIdentity;
 import android.net.NetworkStats;
 import android.net.NetworkStatsHistory;
@@ -40,6 +43,7 @@ import android.test.AndroidTestCase;
 import android.test.MoreAsserts;
 import android.test.suitebuilder.annotation.SmallTest;
 import android.text.format.DateUtils;
+import android.util.RecurrenceRule;
 
 import com.android.frameworks.tests.net.R;
 
@@ -53,6 +57,9 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.List;
@@ -70,14 +77,27 @@ public class NetworkStatsCollectionTest extends AndroidTestCase {
     private static final long TIME_B = 1326110400000L; // UTC: Monday 9th January 2012 12:00:00 PM
     private static final long TIME_C = 1326132000000L; // UTC: Monday 9th January 2012 06:00:00 PM
 
+    private static Clock sOriginalClock;
+
     @Override
     public void setUp() throws Exception {
         super.setUp();
+        sOriginalClock = RecurrenceRule.sClock;
 
         // ignore any device overlay while testing
         NetworkTemplate.forceAllNetworkTypes();
     }
 
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+        RecurrenceRule.sClock = sOriginalClock;
+    }
+
+    private void setClock(Instant instant) {
+        RecurrenceRule.sClock = Clock.fixed(instant, ZoneId.systemDefault());
+    }
+
     public void testReadLegacyNetwork() throws Exception {
         final File testFile = new File(getContext().getFilesDir(), TEST_FILE);
         stageFile(R.raw.netstats_v1, testFile);
@@ -238,6 +258,9 @@ public class NetworkStatsCollectionTest extends AndroidTestCase {
         final NetworkStatsCollection collection = new NetworkStatsCollection(30 * MINUTE_IN_MILLIS);
         collection.readLegacyNetwork(testFile);
 
+        // We're in the future, but not that far off
+        setClock(Instant.parse("2012-06-01T00:00:00.00Z"));
+
         // Test a bunch of plans that should result in no augmentation
         final List<SubscriptionPlan> plans = new ArrayList<>();
 
@@ -416,6 +439,28 @@ public class NetworkStatsCollectionTest extends AndroidTestCase {
         }
     }
 
+    public void testAugmentPlanGigantic() throws Exception {
+        // We're in the future, but not that far off
+        setClock(Instant.parse("2012-06-01T00:00:00.00Z"));
+
+        // Create a simple history with a ton of measured usage
+        final NetworkStatsCollection large = new NetworkStatsCollection(HOUR_IN_MILLIS);
+        final NetworkIdentitySet ident = new NetworkIdentitySet();
+        ident.add(new NetworkIdentity(ConnectivityManager.TYPE_MOBILE, -1, TEST_IMSI, null,
+                false, true));
+        large.recordData(ident, UID_ALL, SET_ALL, TAG_NONE, TIME_A, TIME_B,
+                new NetworkStats.Entry(12_730_893_164L, 1, 0, 0, 0));
+
+        // Verify untouched total
+        assertEquals(12_730_893_164L, getHistory(large, null, TIME_A, TIME_C).getTotalBytes());
+
+        // Verify anchor that might cause overflows
+        final SubscriptionPlan plan = SubscriptionPlan.Builder
+                .createRecurringMonthly(ZonedDateTime.parse("2012-01-09T00:00:00.00Z"))
+                .setDataUsage(4_939_212_390L, TIME_B).build();
+        assertEquals(4_939_212_386L, getHistory(large, plan, TIME_A, TIME_C).getTotalBytes());
+    }
+
     public void testRounding() throws Exception {
         final NetworkStatsCollection coll = new NetworkStatsCollection(HOUR_IN_MILLIS);
 
@@ -437,6 +482,25 @@ public class NetworkStatsCollectionTest extends AndroidTestCase {
         assertEquals(TIME_A - HOUR_IN_MILLIS, coll.roundDown(TIME_A - 1));
     }
 
+    public void testMultiplySafe() {
+        assertEquals(25, multiplySafe(50, 1, 2));
+        assertEquals(100, multiplySafe(50, 2, 1));
+
+        assertEquals(-10, multiplySafe(30, -1, 3));
+        assertEquals(0, multiplySafe(30, 0, 3));
+        assertEquals(10, multiplySafe(30, 1, 3));
+        assertEquals(20, multiplySafe(30, 2, 3));
+        assertEquals(30, multiplySafe(30, 3, 3));
+        assertEquals(40, multiplySafe(30, 4, 3));
+
+        assertEquals(100_000_000_000L,
+                multiplySafe(300_000_000_000L, 10_000_000_000L, 30_000_000_000L));
+        assertEquals(100_000_000_010L,
+                multiplySafe(300_000_000_000L, 10_000_000_001L, 30_000_000_000L));
+        assertEquals(823_202_048L,
+                multiplySafe(4_939_212_288L, 2_121_815_528L, 12_730_893_165L));
+    }
+
     /**
      * Copy a {@link Resources#openRawResource(int)} into {@link File} for
      * testing purposes.