From d631c5836d795d61be73b390b451c27e5607b2cd Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 22 May 2018 11:35:29 -0600
Subject: [PATCH] Last-ditch clamping of negative NetworkStats.

We've seen reports of negative values flowing through to attempt
being recorded, which will outright crash.  This change does one
last-ditch check to see if we're about to work with negative values,
reporting any trouble and clamping them to zero so we don't crash.

This gives us the data we need to continue investigating without
triggering runtime restarts in the field.

Bug: 80057433
Test: atest android.net.NetworkStatsTest
Change-Id: I8174391c6cf5dadc2c2c10a8d841ee07e1f7d934
---
 core/java/android/net/NetworkStats.java       |  2 ++
 .../server/net/NetworkStatsRecorder.java      | 14 +++++++++++++
 .../server/net/NetworkStatsService.java       | 20 +++++++++++++++----
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java
index f0dd2629f9..edf9bc1c6b 100644
--- a/core/java/android/net/NetworkStats.java
+++ b/core/java/android/net/NetworkStats.java
@@ -1100,6 +1100,8 @@ public class NetworkStats implements Parcelable {
     public interface NonMonotonicObserver<C> {
         public void foundNonMonotonic(
                 NetworkStats left, int leftIndex, NetworkStats right, int rightIndex, C cookie);
+        public void foundNonMonotonic(
+                NetworkStats stats, int statsIndex, C cookie);
     }
 
     /**
diff --git a/services/core/java/com/android/server/net/NetworkStatsRecorder.java b/services/core/java/com/android/server/net/NetworkStatsRecorder.java
index 4bee55ef8b..a16dcf358d 100644
--- a/services/core/java/com/android/server/net/NetworkStatsRecorder.java
+++ b/services/core/java/com/android/server/net/NetworkStatsRecorder.java
@@ -241,6 +241,20 @@ public class NetworkStatsRecorder {
         NetworkStats.Entry entry = null;
         for (int i = 0; i < delta.size(); i++) {
             entry = delta.getValues(i, entry);
+
+            // As a last-ditch sanity check, report any negative values and
+            // clamp them so recording below doesn't croak.
+            if (entry.isNegative()) {
+                if (mObserver != null) {
+                    mObserver.foundNonMonotonic(delta, i, mCookie);
+                }
+                entry.rxBytes = Math.max(entry.rxBytes, 0);
+                entry.rxPackets = Math.max(entry.rxPackets, 0);
+                entry.txBytes = Math.max(entry.txBytes, 0);
+                entry.txPackets = Math.max(entry.txPackets, 0);
+                entry.operations = Math.max(entry.operations, 0);
+            }
+
             final NetworkIdentitySet ident = ifaceIdent.get(entry.iface);
             if (ident == null) {
                 unknownIfaces.add(entry.iface);
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 9ef6c66b07..3354525498 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -1713,7 +1713,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         @Override
         public void foundNonMonotonic(NetworkStats left, int leftIndex, NetworkStats right,
                 int rightIndex, String cookie) {
-            Log.w(TAG, "found non-monotonic values; saving to dropbox");
+            Log.w(TAG, "Found non-monotonic values; saving to dropbox");
 
             // record error for debugging
             final StringBuilder builder = new StringBuilder();
@@ -1722,9 +1722,21 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
             builder.append("left=").append(left).append('\n');
             builder.append("right=").append(right).append('\n');
 
-            final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService(
-                    Context.DROPBOX_SERVICE);
-            dropBox.addText(TAG_NETSTATS_ERROR, builder.toString());
+            mContext.getSystemService(DropBoxManager.class).addText(TAG_NETSTATS_ERROR,
+                    builder.toString());
+        }
+
+        @Override
+        public void foundNonMonotonic(
+                NetworkStats stats, int statsIndex, String cookie) {
+            Log.w(TAG, "Found non-monotonic values; saving to dropbox");
+
+            final StringBuilder builder = new StringBuilder();
+            builder.append("Found non-monotonic " + cookie + " values at [" + statsIndex + "]\n");
+            builder.append("stats=").append(stats).append('\n');
+
+            mContext.getSystemService(DropBoxManager.class).addText(TAG_NETSTATS_ERROR,
+                    builder.toString());
         }
     }