LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

NetworkStats: Fix race condition causing system server crashes

Browse Source 
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
This commit is contained in:
Hugo Benichi
2019-05-15 23:38:43 +09:00
parent b8f3f2446e
commit 0308b3971d
1 changed files with 10 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
services/core/java/com/android/server/net/NetworkStatsService.java
View File

@@ -373,14 +373,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
public void systemReady() {
synchronized (mStatsLock) {
mSystemReady = true;
if (!isBandwidthControlEnabled()) {
Slog.w(TAG, "bandwidth controls disabled, unable to track stats");
return;
}
synchronized (mStatsLock) {
// 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 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// 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();
}
@@ -486,23 +488,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
}
/**
* 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
@@ -548,8 +533,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
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 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private NetworkStats getNetworkUidBytes(NetworkTemplate template, long start, long end) {
assertSystemReady();
assertBandwidthControlEnabled();
final NetworkStatsCollection uidComplete;
synchronized (mStatsLock) {
@@ -768,7 +749,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
NetworkState[] networkStates,
String activeIface) {
checkNetworkStackPermission(mContext);
assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -868,7 +847,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override
public void forceUpdate() {
mContext.enforceCallingOrSelfPermission(READ_NETWORK_USAGE_HISTORY, TAG);
assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -879,8 +857,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
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 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
}
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,