From 3d55d4c02e9f8f51ef5da1ebbce7723269e821e2 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Thu, 12 Apr 2018 19:26:34 +0900 Subject: [PATCH 1/3] Add rate limiting and logging for NetworkStats. Bug: 77908520, 77808546, 77853238, 77154412 Test: atest com.android.server.net.NetworkStatsServiceTest Test: manual: flashed, verified network usage updated Change-Id: I905dbea85e00f80103916939f6d4bf8cab931d03 --- .../server/net/NetworkStatsService.java | 65 +++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 4e848f8f36..e3ff72b9a9 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -286,6 +286,16 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private long mPersistThreshold = 2 * MB_IN_BYTES; private long mGlobalAlertBytes; + private static final long POLL_RATE_LIMIT_MS = 15_000; + + private long mLastStatsSessionPoll; + + /** Map from UID to number of opened sessions */ + @GuardedBy("mOpenSessionCallsPerUid") + private final SparseIntArray mOpenSessionCallsPerUid = new SparseIntArray(); + + private final static int DUMP_STATS_SESSION_COUNT = 20; + private static @NonNull File getDefaultSystemDir() { return new File(Environment.getDataDirectory(), "system"); } @@ -509,10 +519,31 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return openSessionInternal(flags, callingPackage); } + private boolean isRateLimitedForPoll(int callingUid) { + if (callingUid == android.os.Process.SYSTEM_UID) { + return false; + } + + final long lastCallTime; + final long now = SystemClock.elapsedRealtime(); + synchronized (mOpenSessionCallsPerUid) { + int calls = mOpenSessionCallsPerUid.get(callingUid, 0); + mOpenSessionCallsPerUid.put(callingUid, calls + 1); + lastCallTime = mLastStatsSessionPoll; + mLastStatsSessionPoll = now; + } + + return now - lastCallTime < POLL_RATE_LIMIT_MS; + } + private INetworkStatsSession openSessionInternal(final int flags, final String callingPackage) { assertBandwidthControlEnabled(); - if ((flags & NetworkStatsManager.FLAG_POLL_ON_OPEN) != 0) { + final int callingUid = Binder.getCallingUid(); + final int usedFlags = isRateLimitedForPoll(callingUid) + ? flags & (~NetworkStatsManager.FLAG_POLL_ON_OPEN) + : flags; + if ((usedFlags & NetworkStatsManager.FLAG_POLL_ON_OPEN) != 0) { final long ident = Binder.clearCallingIdentity(); try { performPoll(FLAG_PERSIST_ALL); @@ -525,7 +556,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // for its lifetime; when caller closes only weak references remain. return new INetworkStatsSession.Stub() { - private final int mCallingUid = Binder.getCallingUid(); + private final int mCallingUid = callingUid; private final String mCallingPackage = callingPackage; private final @NetworkStatsAccess.Level int mAccessLevel = checkAccessLevel( callingPackage); @@ -559,20 +590,20 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public NetworkStats getDeviceSummaryForNetwork( NetworkTemplate template, long start, long end) { - return internalGetSummaryForNetwork(template, flags, start, end, mAccessLevel, + return internalGetSummaryForNetwork(template, usedFlags, start, end, mAccessLevel, mCallingUid); } @Override public NetworkStats getSummaryForNetwork( NetworkTemplate template, long start, long end) { - return internalGetSummaryForNetwork(template, flags, start, end, mAccessLevel, + return internalGetSummaryForNetwork(template, usedFlags, start, end, mAccessLevel, mCallingUid); } @Override public NetworkStatsHistory getHistoryForNetwork(NetworkTemplate template, int fields) { - return internalGetHistoryForNetwork(template, flags, fields, mAccessLevel, + return internalGetHistoryForNetwork(template, usedFlags, fields, mAccessLevel, mCallingUid); } @@ -1461,6 +1492,30 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } pw.decreaseIndent(); + // Get the top openSession callers + final SparseIntArray calls; + synchronized (mOpenSessionCallsPerUid) { + calls = mOpenSessionCallsPerUid.clone(); + } + + final int N = calls.size(); + final long[] values = new long[N]; + for (int j = 0; j < N; j++) { + values[j] = ((long) calls.valueAt(j) << 32) | calls.keyAt(j); + } + Arrays.sort(values); + + pw.println("Top openSession callers (uid=count):"); + pw.increaseIndent(); + final int end = Math.max(0, N - DUMP_STATS_SESSION_COUNT); + for (int j = N - 1; j >= end; j--) { + final int uid = (int) (values[j] & 0xffffffff); + final int count = (int) (values[j] >> 32); + pw.print(uid); pw.print("="); pw.println(count); + } + pw.decreaseIndent(); + pw.println(); + pw.println("Dev stats:"); pw.increaseIndent(); mDevRecorder.dumpLocked(pw, fullHistory); From 930aeb0c00a40369cab2793353208095eb40e438 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 13 Apr 2018 13:29:59 -0600 Subject: [PATCH 2/3] OP_GET_USAGE_STATS should be noted, not checked. Per email feedback, we should be using "noteOp" instead of "checkOp" when testing if caller holds OP_GET_USAGE_STATS, so that we record that caller used the operation. Bug: 77662908 Test: builds, boots Exempt-From-Owner-Approval: keep tests passing Change-Id: I3a60345d590534fdbc2c1248e0d30dc85a5d6772 --- .../core/java/com/android/server/net/NetworkStatsAccess.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsAccess.java b/services/core/java/com/android/server/net/NetworkStatsAccess.java index 98fe770774..cebc472178 100644 --- a/services/core/java/com/android/server/net/NetworkStatsAccess.java +++ b/services/core/java/com/android/server/net/NetworkStatsAccess.java @@ -174,7 +174,7 @@ public final class NetworkStatsAccess { AppOpsManager appOps = (AppOpsManager) context.getSystemService( Context.APP_OPS_SERVICE); - final int mode = appOps.checkOp(AppOpsManager.OP_GET_USAGE_STATS, + final int mode = appOps.noteOp(AppOpsManager.OP_GET_USAGE_STATS, callingUid, callingPackage); if (mode == AppOpsManager.MODE_DEFAULT) { // The default behavior here is to check if PackageManager has given the app From e6c2cfbe42c995bd647d054e70e1600bfb497ee3 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 13 Apr 2018 14:28:30 -0600 Subject: [PATCH 3/3] Don't process broadcasts until really ready. Someone started setting mSystemReady too early, when we really want to know SystemServer's real ready state. Add a variable to track that, and don't process broadcasts until that's set. Bug: 78020762 Test: builds, boots Change-Id: I65213e46044c95fb0a8a4b09b9aa463bb15c1844 --- .../core/java/com/android/server/net/NetworkStatsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index e3ff72b9a9..e7f92dca60 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -282,7 +282,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private Handler mHandler; private Handler.Callback mHandlerCallback; - private boolean mSystemReady; + private volatile boolean mSystemReady; private long mPersistThreshold = 2 * MB_IN_BYTES; private long mGlobalAlertBytes;