NetworkStats: Fix race condition causing system server crashes
NetworkStatsService uses an internal boolean to know when it has
started for the purpose of preventing access to other internal
variables before they are initialized.
However that boolean is set to true in systemReady() non-atomically
with respect to the initialization of the other variables it guards,
which can cause the system server to crash.
This patch fixes this concurrency bug by moving setting the internal
boolean flag and the variable it guards in one atomic synchronized
block.
This patch also removes code checking if bandwidth control is enabled,
because this is now always true.
Bug: 132767673
Test: Compiled.
Change-Id: Ia089b5767ce271d669879c975508654d4dd03429
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 2fc78d6..5505828 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -373,14 +373,9 @@
}
public void systemReady() {
- mSystemReady = true;
-
- if (!isBandwidthControlEnabled()) {
- Slog.w(TAG, "bandwidth controls disabled, unable to track stats");
- return;
- }
-
synchronized (mStatsLock) {
+ mSystemReady = true;
+
// create data recorders along with historical rotators
mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false);
mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false);
@@ -426,7 +421,14 @@
// ignored; service lives in system_server
}
- registerPollAlarmLocked();
+ // schedule periodic pall alarm based on {@link NetworkStatsSettings#getPollInterval()}.
+ final PendingIntent pollIntent =
+ PendingIntent.getBroadcast(mContext, 0, new Intent(ACTION_NETWORK_STATS_POLL), 0);
+
+ final long currentRealtime = SystemClock.elapsedRealtime();
+ mAlarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME, currentRealtime,
+ mSettings.getPollInterval(), pollIntent);
+
registerGlobalAlert();
}
@@ -487,23 +489,6 @@
}
/**
- * Clear any existing {@link #ACTION_NETWORK_STATS_POLL} alarms, and
- * reschedule based on current {@link NetworkStatsSettings#getPollInterval()}.
- */
- private void registerPollAlarmLocked() {
- if (mPollIntent != null) {
- mAlarmManager.cancel(mPollIntent);
- }
-
- mPollIntent = PendingIntent.getBroadcast(
- mContext, 0, new Intent(ACTION_NETWORK_STATS_POLL), 0);
-
- final long currentRealtime = SystemClock.elapsedRealtime();
- mAlarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME, currentRealtime,
- mSettings.getPollInterval(), mPollIntent);
- }
-
- /**
* Register for a global alert that is delivered through
* {@link INetworkManagementEventObserver} once a threshold amount of data
* has been transferred.
@@ -548,8 +533,6 @@
}
private INetworkStatsSession openSessionInternal(final int flags, final String callingPackage) {
- assertBandwidthControlEnabled();
-
final int callingUid = Binder.getCallingUid();
final int usedFlags = isRateLimitedForPoll(callingUid)
? flags & (~NetworkStatsManager.FLAG_POLL_ON_OPEN)
@@ -742,7 +725,6 @@
private long getNetworkTotalBytes(NetworkTemplate template, long start, long end) {
assertSystemReady();
- assertBandwidthControlEnabled();
// NOTE: if callers want to get non-augmented data, they should go
// through the public API
@@ -753,7 +735,6 @@
private NetworkStats getNetworkUidBytes(NetworkTemplate template, long start, long end) {
assertSystemReady();
- assertBandwidthControlEnabled();
final NetworkStatsCollection uidComplete;
synchronized (mStatsLock) {
@@ -768,7 +749,6 @@
if (Binder.getCallingUid() != uid) {
mContext.enforceCallingOrSelfPermission(ACCESS_NETWORK_STATE, TAG);
}
- assertBandwidthControlEnabled();
// TODO: switch to data layer stats once kernel exports
// for now, read network layer stats and flatten across all ifaces
@@ -855,7 +835,6 @@
NetworkState[] networkStates,
String activeIface) {
checkNetworkStackPermission(mContext);
- assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -868,7 +847,6 @@
@Override
public void forceUpdate() {
mContext.enforceCallingOrSelfPermission(READ_NETWORK_USAGE_HISTORY, TAG);
- assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -879,8 +857,6 @@
}
private void advisePersistThreshold(long thresholdBytes) {
- assertBandwidthControlEnabled();
-
// clamp threshold into safe range
mPersistThreshold = MathUtils.constrain(thresholdBytes, 128 * KB_IN_BYTES, 2 * MB_IN_BYTES);
if (LOGV) {
@@ -1739,24 +1715,6 @@
}
}
- private void assertBandwidthControlEnabled() {
- if (!isBandwidthControlEnabled()) {
- throw new IllegalStateException("Bandwidth module disabled");
- }
- }
-
- private boolean isBandwidthControlEnabled() {
- final long token = Binder.clearCallingIdentity();
- try {
- return mNetworkManager.isBandwidthControlEnabled();
- } catch (RemoteException e) {
- // ignored; service lives in system_server
- return false;
- } finally {
- Binder.restoreCallingIdentity(token);
- }
- }
-
private class DropBoxNonMonotonicObserver implements NonMonotonicObserver<String> {
@Override
public void foundNonMonotonic(NetworkStats left, int leftIndex, NetworkStats right,