From 0682bdce92b9bf58abb9a0f48dbb16161ac69951 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 5 Jun 2024 16:57:19 +0800 Subject: [PATCH 1/2] Fix spamming of network stats updated intent While the reason of triggering frequently polls is still unknown (b/343844995), this commit prevents the spamming of network stats update intents by sending broadcast messages at intervals of at least one second. It also ensures that update requests experience a delay of no more than one second. This is safe since NETWORK_STATS_UPDATED intent is only use for signaling updated of NetworkStats, which is stateless and not very time sensitive. A kill-switch flag is included for rollback if needed. However, while the intent was not be able to deprecated, all callers should migrate to use registerDataUsageCallback with proper threshold instead. Test: atest FrameworksNetTests:android.net.connectivity.com.android.\ server.net.NetworkStatsServiceTest\ #testNetworkStatsUpdatedIntentSpam_rateLimitOn Test: atest FrameworksNetTests:android.net.connectivity.com.android.\ server.net.NetworkStatsServiceTest\ #testNetworkStatsUpdatedIntentSpam_rateLimitOff Bug: 343844995 Change-Id: I972f0778b823ed0dc1f35b1b011f1ad225ca020c --- .../server/net/NetworkStatsService.java | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 114cf2ecc2..9e1f10583a 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -257,6 +257,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ private static final int DEFAULT_PERFORM_POLL_DELAY_MS = 1000; + /** + * The delay time between to network stats update intents. + * Added to fix intent spams (b/3115462) + */ + private static final int BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS = 1000; + private static final String TAG_NETSTATS_ERROR = "netstats_error"; /** @@ -469,15 +475,31 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private long mLastStatsSessionPoll; + /** + * The latest time of broadcasting network stats updated message. + * Note that this time could be in the past for already done broadcasts, + * or in the future for scheduled broadcasts. This record is needed for + * rate limiting intents to prevent intent spams (b/3115462) + */ + @GuardedBy("mStatsLock") + private long mLatestNetworkStatsUpdatedBroadcastScheduledTime = -1; + + private final TrafficStatsRateLimitCache mTrafficStatsTotalCache; private final TrafficStatsRateLimitCache mTrafficStatsIfaceCache; private final TrafficStatsRateLimitCache mTrafficStatsUidCache; static final String TRAFFICSTATS_RATE_LIMIT_CACHE_ENABLED_FLAG = "trafficstats_rate_limit_cache_enabled_flag"; + static final String BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG = + "broadcast_network_stats_updated_rate_limit_enabled_flag"; private final boolean mAlwaysUseTrafficStatsRateLimitCache; private final int mTrafficStatsRateLimitCacheExpiryDuration; private final int mTrafficStatsRateLimitCacheMaxEntries; + private final boolean mBroadcastNetworkStatsUpdatedRateLimitEnabled; + + + private final Object mOpenSessionCallsLock = new Object(); /** @@ -669,6 +691,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mAlwaysUseTrafficStatsRateLimitCache = mDeps.alwaysUseTrafficStatsRateLimitCache(mContext); + mBroadcastNetworkStatsUpdatedRateLimitEnabled = + mDeps.enabledBroadcastNetworkStatsUpdatedRateLimiting(mContext); mTrafficStatsRateLimitCacheExpiryDuration = mDeps.getTrafficStatsRateLimitCacheExpiryDuration(); mTrafficStatsRateLimitCacheMaxEntries = @@ -926,6 +950,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { ctx, CONFIG_ENABLE_NETWORK_STATS_EVENT_LOGGER); } + /** + * Get whether broadcast network stats update rate limiting is enabled. + * + * This method should only be called once in the constructor, + * to ensure that the code does not need to deal with flag values changing at runtime. + */ + public boolean enabledBroadcastNetworkStatsUpdatedRateLimiting(Context ctx) { + return DeviceConfigUtils.isTetheringFeatureNotChickenedOut( + ctx, BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG); + } + /** * Get whether TrafficStats rate-limit cache is always applied. * @@ -2645,8 +2680,20 @@ public class NetworkStatsService extends INetworkStatsService.Stub { performSampleLocked(); } - // finally, dispatch updated event to any listeners - mHandler.sendMessage(mHandler.obtainMessage(MSG_BROADCAST_NETWORK_STATS_UPDATED)); + // Dispatch updated event to listeners, preventing intent spamming + // (b/343844995) possibly from abnormal modem RAT changes or misbehaving + // app calls (see NetworkStatsEventLogger#POLL_REASON_* for possible reasons). + // If no broadcasts are scheduled, use the time of the last broadcast + // to schedule the next one ASAP. + if (!mBroadcastNetworkStatsUpdatedRateLimitEnabled) { + mHandler.sendMessage(mHandler.obtainMessage(MSG_BROADCAST_NETWORK_STATS_UPDATED)); + } else if (mLatestNetworkStatsUpdatedBroadcastScheduledTime < SystemClock.uptimeMillis()) { + mLatestNetworkStatsUpdatedBroadcastScheduledTime = Math.max(SystemClock.uptimeMillis(), + mLatestNetworkStatsUpdatedBroadcastScheduledTime + + BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS); + mHandler.sendMessageAtTime(mHandler.obtainMessage(MSG_BROADCAST_NETWORK_STATS_UPDATED), + mLatestNetworkStatsUpdatedBroadcastScheduledTime); + } Trace.traceEnd(TRACE_TAG_NETWORK); } From fecc2ec19312af071b0f2524060faf5427567f32 Mon Sep 17 00:00:00 2001 From: Yun-Chen Cheng Date: Tue, 30 Jul 2024 06:01:27 +0000 Subject: [PATCH 2/2] Unit test for intent spam Add unit test to test feature BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG. When feature is on, broadcast message should be delivered with the designated interval(b/343844995), When feature is off, each message should be delivered regardless of previous intent. Test: atest FrameworksNetTests:android.net.connectivity.com.android\ .server.net.NetworkStatsServiceTest\ #testNetworkStatsUpdatedIntentSpam_rateLimitOn Test: atest FrameworksNetTests:android.net.connectivity.com.android\ .server.net.NetworkStatsServiceTest\ #testNetworkStatsUpdatedIntentSpam_rateLimitOff Bug: 343844995 Change-Id: I4b89497c336a310c6959db579ca9ae3c572ff70c --- .../server/net/NetworkStatsService.java | 52 +++++--- .../server/net/NetworkStatsServiceTest.java | 113 ++++++++++++++++++ 2 files changed, 149 insertions(+), 16 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 9e1f10583a..11343d2434 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -74,12 +74,12 @@ import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIV import static com.android.net.module.util.DeviceConfigUtils.getDeviceConfigPropertyInt; import static com.android.net.module.util.NetworkCapabilitiesUtils.getDisplayTransport; import static com.android.net.module.util.NetworkStatsUtils.LIMIT_GLOBAL_ALERT; -import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_PERIODIC; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_DUMPSYS; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_FORCE_UPDATE; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_GLOBAL_ALERT; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_NETWORK_STATUS_CHANGED; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_OPEN_SESSION; +import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_PERIODIC; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_RAT_CHANGED; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_REG_CALLBACK; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_REMOVE_UIDS; @@ -242,13 +242,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // A message for broadcasting ACTION_NETWORK_STATS_UPDATED in handler thread to prevent // deadlock. private static final int MSG_BROADCAST_NETWORK_STATS_UPDATED = 4; - /** Flags to control detail level of poll event. */ private static final int FLAG_PERSIST_NETWORK = 0x1; private static final int FLAG_PERSIST_UID = 0x2; private static final int FLAG_PERSIST_ALL = FLAG_PERSIST_NETWORK | FLAG_PERSIST_UID; private static final int FLAG_PERSIST_FORCE = 0x100; - /** * When global alert quota is high, wait for this delay before processing each polling, * and do not schedule further polls once there is already one queued. @@ -257,12 +255,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ private static final int DEFAULT_PERFORM_POLL_DELAY_MS = 1000; - /** - * The delay time between to network stats update intents. - * Added to fix intent spams (b/3115462) - */ - private static final int BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS = 1000; - private static final String TAG_NETSTATS_ERROR = "netstats_error"; /** @@ -319,6 +311,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { static final String TRAFFIC_STATS_CACHE_MAX_ENTRIES_NAME = "trafficstats_cache_max_entries"; static final int DEFAULT_TRAFFIC_STATS_CACHE_EXPIRY_DURATION_MS = 1000; static final int DEFAULT_TRAFFIC_STATS_CACHE_MAX_ENTRIES = 400; + /** + * The delay time between to network stats update intents. + * Added to fix intent spams (b/3115462) + */ + @VisibleForTesting(visibility = PRIVATE) + static final int BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS = 1000; private final Context mContext; private final NetworkStatsFactory mStatsFactory; @@ -391,6 +389,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { long getXtPersistBytes(long def); long getUidPersistBytes(long def); long getUidTagPersistBytes(long def); + long getBroadcastNetworkStatsUpdateDelayMs(); } private final Object mStatsLock = new Object(); @@ -476,13 +475,18 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private long mLastStatsSessionPoll; /** - * The latest time of broadcasting network stats updated message. - * Note that this time could be in the past for already done broadcasts, - * or in the future for scheduled broadcasts. This record is needed for - * rate limiting intents to prevent intent spams (b/3115462) + * The timestamp of the most recent network stats broadcast. + * + * Note that this time could be in the past for completed broadcasts, + * or in the future for scheduled broadcasts. + * + * It is initialized to {@code Long.MIN_VALUE} to ensure that the first broadcast request + * is fulfilled immediately, regardless of the delay time. + * + * This value is used to enforce rate limiting on intents, preventing intent spam. */ @GuardedBy("mStatsLock") - private long mLatestNetworkStatsUpdatedBroadcastScheduledTime = -1; + private long mLatestNetworkStatsUpdatedBroadcastScheduledTime = Long.MIN_VALUE; private final TrafficStatsRateLimitCache mTrafficStatsTotalCache; @@ -719,6 +723,15 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // TODO: Move more stuff into dependencies object. @VisibleForTesting public static class Dependencies { + /** + * Get broadcast network stats updated delay time in ms + * @return + */ + @NonNull + public long getBroadcastNetworkStatsUpdateDelayMs() { + return BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS; + } + /** * Get legacy platform stats directory. */ @@ -2688,9 +2701,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { if (!mBroadcastNetworkStatsUpdatedRateLimitEnabled) { mHandler.sendMessage(mHandler.obtainMessage(MSG_BROADCAST_NETWORK_STATS_UPDATED)); } else if (mLatestNetworkStatsUpdatedBroadcastScheduledTime < SystemClock.uptimeMillis()) { - mLatestNetworkStatsUpdatedBroadcastScheduledTime = Math.max(SystemClock.uptimeMillis(), + mLatestNetworkStatsUpdatedBroadcastScheduledTime = Math.max( mLatestNetworkStatsUpdatedBroadcastScheduledTime - + BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS); + + mSettings.getBroadcastNetworkStatsUpdateDelayMs(), + SystemClock.uptimeMillis() + ); mHandler.sendMessageAtTime(mHandler.obtainMessage(MSG_BROADCAST_NETWORK_STATS_UPDATED), mLatestNetworkStatsUpdatedBroadcastScheduledTime); } @@ -3652,6 +3667,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { public long getUidTagPersistBytes(long def) { return def; } + + @Override + public long getBroadcastNetworkStatsUpdateDelayMs() { + return BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS; + } } // TODO: Read stats by using BpfNetMapsReader. diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index 7e0a225bd0..3d2f38947d 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -69,6 +69,8 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.doThrow; import static com.android.server.net.NetworkStatsEventLogger.POLL_REASON_RAT_CHANGED; import static com.android.server.net.NetworkStatsEventLogger.PollEvent.pollReasonNameOf; import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_POLL; +import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_UPDATED; +import static com.android.server.net.NetworkStatsService.BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG; import static com.android.server.net.NetworkStatsService.DEFAULT_TRAFFIC_STATS_CACHE_EXPIRY_DURATION_MS; import static com.android.server.net.NetworkStatsService.DEFAULT_TRAFFIC_STATS_CACHE_MAX_ENTRIES; import static com.android.server.net.NetworkStatsService.NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME; @@ -82,6 +84,7 @@ import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalMatchers.aryEq; @@ -101,8 +104,10 @@ import static org.mockito.Mockito.verify; import android.annotation.NonNull; import android.app.AlarmManager; +import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; +import android.content.IntentFilter; import android.content.pm.PackageManager; import android.content.res.Resources; import android.database.ContentObserver; @@ -138,6 +143,7 @@ import android.os.UserHandle; import android.provider.Settings; import android.system.ErrnoException; import android.telephony.TelephonyManager; +import android.testing.TestableLooper; import android.text.TextUtils; import android.util.ArrayMap; import android.util.IndentingPrintWriter; @@ -150,6 +156,7 @@ import androidx.test.filters.SmallTest; import com.android.connectivity.resources.R; import com.android.internal.util.FileRotator; import com.android.internal.util.test.BroadcastInterceptingContext; +import com.android.net.module.util.ArrayTrackRecord; import com.android.net.module.util.BpfDump; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.LocationPermissionChecker; @@ -617,6 +624,12 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { return mFeatureFlags.getOrDefault(TRAFFICSTATS_RATE_LIMIT_CACHE_ENABLED_FLAG, false); } + @Override + public boolean enabledBroadcastNetworkStatsUpdatedRateLimiting(Context ctx) { + return mFeatureFlags.getOrDefault( + BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG, true); + } + @Override public int getTrafficStatsRateLimitCacheExpiryDuration() { return DEFAULT_TRAFFIC_STATS_CACHE_EXPIRY_DURATION_MS; @@ -2617,6 +2630,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private void mockDefaultSettings() throws Exception { mockSettings(HOUR_IN_MILLIS, WEEK_IN_MILLIS); + mSettings.setBroadcastNetworkStatsUpdateDelayMs( + NetworkStatsService.BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS); } private void mockSettings(long bucketDuration, long deleteAge) { @@ -2631,6 +2646,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { @NonNull private volatile Config mConfig; private final AtomicBoolean mCombineSubtypeEnabled = new AtomicBoolean(); + private long mBroadcastNetworkStatsUpdateDelayMs = + NetworkStatsService.BROADCAST_NETWORK_STATS_UPDATED_DELAY_MS; TestNetworkStatsSettings(long bucketDuration, long deleteAge) { mConfig = new Config(bucketDuration, deleteAge, deleteAge); @@ -2693,6 +2710,15 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { public boolean getAugmentEnabled() { return false; } + + @Override + public long getBroadcastNetworkStatsUpdateDelayMs() { + return mBroadcastNetworkStatsUpdateDelayMs; + } + + public void setBroadcastNetworkStatsUpdateDelayMs(long broadcastDelay) { + mBroadcastNetworkStatsUpdateDelayMs = broadcastDelay; + } } private void assertStatsFilesExist(boolean exist) { @@ -3064,4 +3090,91 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { final String dump = getDump(); assertDumpContains(dump, "Log for testing"); } + + private static class TestNetworkStatsUpdatedReceiver extends BroadcastReceiver { + private final ArrayTrackRecord.ReadHead mHistory; + + TestNetworkStatsUpdatedReceiver() { + mHistory = (new ArrayTrackRecord()).newReadHead(); + } + + @Override + public void onReceive(Context context, Intent intent) { + mHistory.add(intent); + } + + /** + * Assert no broadcast intent is received in blocking manner + */ + public void assertNoBroadcastIntentReceived() { + assertNull(mHistory.peek()); + } + + /** + * Assert an intent is received and remove it from queue + */ + public void assertBroadcastIntentReceived() { + assertNotNull(mHistory.poll(WAIT_TIMEOUT, number -> true)); + } + } + + @FeatureFlag(name = BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG) + @Test + public void testNetworkStatsUpdatedIntentSpam_rateLimitOn() throws Exception { + // Set the update delay long enough that messages won't be processed before unblocked + // Set a short time to test the behavior before reaching delay. + // Constraint: test running time < toleranceMs < update delay time + mSettings.setBroadcastNetworkStatsUpdateDelayMs(100_000L); + final long toleranceMs = 5000; + + final TestableLooper mTestableLooper = new TestableLooper(mHandlerThread.getLooper()); + final TestNetworkStatsUpdatedReceiver receiver = new TestNetworkStatsUpdatedReceiver(); + mServiceContext.registerReceiver(receiver, new IntentFilter(ACTION_NETWORK_STATS_UPDATED)); + + try { + // Test that before anything, the intent is delivered immediately + mService.forceUpdate(); + mTestableLooper.processAllMessages(); + receiver.assertBroadcastIntentReceived(); + receiver.assertNoBroadcastIntentReceived(); + + // Test that the next two intents results in exactly one intent delivered + for (int i = 0; i < 2; i++) { + mService.forceUpdate(); + } + // Test that the delay depends on our set value + mTestableLooper.moveTimeForward(mSettings.getBroadcastNetworkStatsUpdateDelayMs() + - toleranceMs); + mTestableLooper.processAllMessages(); + receiver.assertNoBroadcastIntentReceived(); + + // Unblock messages and test that the second and third update + // is broadcasted right after the delay + mTestableLooper.moveTimeForward(toleranceMs); + mTestableLooper.processAllMessages(); + receiver.assertBroadcastIntentReceived(); + receiver.assertNoBroadcastIntentReceived(); + + } finally { + mTestableLooper.destroy(); + } + } + + @FeatureFlag(name = BROADCAST_NETWORK_STATS_UPDATED_RATE_LIMIT_ENABLED_FLAG, enabled = false) + @Test + public void testNetworkStatsUpdatedIntentSpam_rateLimitOff() throws Exception { + // Set the update delay long enough to ensure that messages are processed + // despite the rate limit. + mSettings.setBroadcastNetworkStatsUpdateDelayMs(100_000L); + + final TestNetworkStatsUpdatedReceiver receiver = new TestNetworkStatsUpdatedReceiver(); + mServiceContext.registerReceiver(receiver, new IntentFilter(ACTION_NETWORK_STATS_UPDATED)); + + for (int i = 0; i < 2; i++) { + mService.forceUpdate(); + waitForIdle(); + receiver.assertBroadcastIntentReceived(); + } + receiver.assertNoBroadcastIntentReceived(); + } }