From 687d8bd094be36b97bfa5e0bd9d162b844e7f47e Mon Sep 17 00:00:00 2001 From: Varun Anand Date: Thu, 7 Feb 2019 14:13:13 -0800 Subject: [PATCH 1/3] Remove ConnectivityManager and its usages from NetworkStatsService. NSS needed it for getting VpnInfo[], NetworkState[] and activeLinkProperties which it used to query via ConnectivityManager. For VpnInfo[], this was racy as NSS may ignore intermediate changes to a VPN's underlying networks. See http://b/123961098 for more context. It may also lead to deadlocks b/w ConnectivityService and NetworkStatsService. See http://b/126245192 for more info. This change will ensure that NSS is never contending on any of ConnectivityService locks. This change also is cherry-picking cleanup made to NSS in http://aosp/628368. Bug: 123961098 Bug: 126245192 Bug: 120145746 Test: atest FrameworksNetTests Change-Id: Ia687845888434c8ddd24bdf44b4c70dfe80e03f5 Merged-In: I57e117bb4e9efe491b19d6b5a479f2d58d1c58e6 --- .../android/net/INetworkStatsService.aidl | 8 ++- .../server/net/NetworkStatsService.java | 53 ++++++++----------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/core/java/android/net/INetworkStatsService.aidl b/core/java/android/net/INetworkStatsService.aidl index 8e6f272388..148b25dfa8 100644 --- a/core/java/android/net/INetworkStatsService.aidl +++ b/core/java/android/net/INetworkStatsService.aidl @@ -19,11 +19,13 @@ package android.net; import android.net.DataUsageRequest; import android.net.INetworkStatsSession; import android.net.Network; +import android.net.NetworkState; import android.net.NetworkStats; import android.net.NetworkStatsHistory; import android.net.NetworkTemplate; import android.os.IBinder; import android.os.Messenger; +import com.android.internal.net.VpnInfo; /** {@hide} */ interface INetworkStatsService { @@ -58,7 +60,11 @@ interface INetworkStatsService { void incrementOperationCount(int uid, int tag, int operationCount); /** Force update of ifaces. */ - void forceUpdateIfaces(in Network[] defaultNetworks); + void forceUpdateIfaces( + in Network[] defaultNetworks, + in VpnInfo[] vpnArray, + in NetworkState[] networkStates, + in String activeIface); /** Force update of statistics. */ void forceUpdate(); diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 17ab29d747..2fef560afd 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -82,7 +82,6 @@ import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.net.DataUsageRequest; -import android.net.IConnectivityManager; import android.net.INetworkManagementEventObserver; import android.net.INetworkStatsService; import android.net.INetworkStatsSession; @@ -161,9 +160,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // Perform polling and persist all (FLAG_PERSIST_ALL). private static final int MSG_PERFORM_POLL = 1; - private static final int MSG_UPDATE_IFACES = 2; // Perform polling, persist network, and register the global alert again. - private static final int MSG_PERFORM_POLL_REGISTER_ALERT = 3; + private static final int MSG_PERFORM_POLL_REGISTER_ALERT = 2; /** Flags to control detail level of poll event. */ private static final int FLAG_PERSIST_NETWORK = 0x1; @@ -196,8 +194,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final boolean mUseBpfTrafficStats; - private IConnectivityManager mConnManager; - @VisibleForTesting public static final String ACTION_NETWORK_STATS_POLL = "com.android.server.action.NETWORK_STATS_POLL"; @@ -259,6 +255,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final ArrayMap mActiveUidIfaces = new ArrayMap<>(); /** Current default active iface. */ + @GuardedBy("mStatsLock") private String mActiveIface; /** Set of any ifaces associated with mobile networks since boot. */ @@ -269,6 +266,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @GuardedBy("mStatsLock") private Network[] mDefaultNetworks = new Network[0]; + /** Set containing info about active VPNs and their underlying networks. */ + @GuardedBy("mStatsLock") + private VpnInfo[] mVpnInfos = new VpnInfo[0]; + private final DropBoxNonMonotonicObserver mNonMonotonicObserver = new DropBoxNonMonotonicObserver(); @@ -371,10 +372,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mHandlerCallback = callback; } - public void bindConnectivityManager(IConnectivityManager connManager) { - mConnManager = checkNotNull(connManager, "missing IConnectivityManager"); - } - public void systemReady() { mSystemReady = true; @@ -853,13 +850,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } @Override - public void forceUpdateIfaces(Network[] defaultNetworks) { + public void forceUpdateIfaces( + Network[] defaultNetworks, + VpnInfo[] vpnArray, + NetworkState[] networkStates, + String activeIface) { mContext.enforceCallingOrSelfPermission(READ_NETWORK_USAGE_HISTORY, TAG); assertBandwidthControlEnabled(); final long token = Binder.clearCallingIdentity(); try { - updateIfaces(defaultNetworks); + updateIfaces(defaultNetworks, vpnArray, networkStates, activeIface); } finally { Binder.restoreCallingIdentity(token); } @@ -1077,11 +1078,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } }; - private void updateIfaces(Network[] defaultNetworks) { + private void updateIfaces( + Network[] defaultNetworks, + VpnInfo[] vpnArray, + NetworkState[] networkStates, + String activeIface) { synchronized (mStatsLock) { mWakeLock.acquire(); try { - updateIfacesLocked(defaultNetworks); + mVpnInfos = vpnArray; + mActiveIface = activeIface; + updateIfacesLocked(defaultNetworks, networkStates); } finally { mWakeLock.release(); } @@ -1095,7 +1102,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * {@link NetworkIdentitySet}. */ @GuardedBy("mStatsLock") - private void updateIfacesLocked(Network[] defaultNetworks) { + private void updateIfacesLocked(Network[] defaultNetworks, NetworkState[] states) { if (!mSystemReady) return; if (LOGV) Slog.v(TAG, "updateIfacesLocked()"); @@ -1107,18 +1114,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // will be persisted during next alarm poll event. performPollLocked(FLAG_PERSIST_NETWORK); - final NetworkState[] states; - final LinkProperties activeLink; - try { - states = mConnManager.getAllNetworkState(); - activeLink = mConnManager.getActiveLinkProperties(); - } catch (RemoteException e) { - // ignored; service lives in system_server - return; - } - - mActiveIface = activeLink != null ? activeLink.getInterfaceName() : null; - // Rebuild active interfaces based on connected networks mActiveIfaces.clear(); mActiveUidIfaces.clear(); @@ -1230,7 +1225,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { Trace.traceEnd(TRACE_TAG_NETWORK); // For per-UID stats, pass the VPN info so VPN traffic is reattributed to responsible apps. - VpnInfo[] vpnArray = mConnManager.getAllVpnInfo(); + VpnInfo[] vpnArray = mVpnInfos; Trace.traceBegin(TRACE_TAG_NETWORK, "recordUid"); mUidRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime); Trace.traceEnd(TRACE_TAG_NETWORK); @@ -1689,10 +1684,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mService.performPoll(FLAG_PERSIST_ALL); return true; } - case MSG_UPDATE_IFACES: { - mService.updateIfaces(null); - return true; - } case MSG_PERFORM_POLL_REGISTER_ALERT: { mService.performPoll(FLAG_PERSIST_NETWORK); mService.registerGlobalAlert(); From c5965f1f4175f12f06dedc9caad21a74657e63ee Mon Sep 17 00:00:00 2001 From: Andrei Onea Date: Thu, 21 Mar 2019 13:57:17 +0000 Subject: [PATCH 2/3] Add @UnsupportedAppUsage annotations For packages: android.companion android.filterfw android.hardware.camera2.utils android.inputmethodservice android.net.nsd android.os android.preference android.security.keymaster android.service.dreams android.telecom android.telephony.ims.compat.feature android.telephony android.util android.view.accessibility android.media.effect This is an automatically generated CL. See go/UnsupportedAppUsage for more details. Exempted-From-Owner-Approval: Mechanical changes to the codebase which have been approved by Android API council and announced on android-eng@ Bug: 110868826 Test: m Change-Id: I9c2f8347952f3cc65759472b0e1a2717b285e44e --- core/java/android/net/nsd/INsdManager.aidl | 1 + 1 file changed, 1 insertion(+) diff --git a/core/java/android/net/nsd/INsdManager.aidl b/core/java/android/net/nsd/INsdManager.aidl index 3361a7b84b..9484c74bcb 100644 --- a/core/java/android/net/nsd/INsdManager.aidl +++ b/core/java/android/net/nsd/INsdManager.aidl @@ -25,6 +25,7 @@ import android.os.Messenger; */ interface INsdManager { + @UnsupportedAppUsage Messenger getMessenger(); void setEnabled(boolean enable); } From 8841fd2f1303f3f4025fad8b67b36d833583dff8 Mon Sep 17 00:00:00 2001 From: Andrei Onea Date: Fri, 22 Mar 2019 11:32:59 +0000 Subject: [PATCH 3/3] Add @UnsupportedAppUsage annotations For packages: android.companion android.filterfw android.hardware.camera2.utils android.inputmethodservice android.net.nsd android.os android.preference android.security.keymaster android.service.dreams android.telecom android.telephony.ims.compat.feature android.telephony android.util android.view.accessibility android.media.effect This is an automatically generated CL. See go/UnsupportedAppUsage for more details. Exempted-From-Owner-Approval: Mechanical changes to the codebase which have been approved by Android API council and announced on android-eng@ Bug: 110868826 Test: m Merged-In: I9c2f8347952f3cc65759472b0e1a2717b285e44e Change-Id: I14793863cf815fa3383fec6c6bf5a9365c2e17eb --- core/java/android/net/nsd/INsdManager.aidl | 1 + 1 file changed, 1 insertion(+) diff --git a/core/java/android/net/nsd/INsdManager.aidl b/core/java/android/net/nsd/INsdManager.aidl index 3361a7b84b..9484c74bcb 100644 --- a/core/java/android/net/nsd/INsdManager.aidl +++ b/core/java/android/net/nsd/INsdManager.aidl @@ -25,6 +25,7 @@ import android.os.Messenger; */ interface INsdManager { + @UnsupportedAppUsage Messenger getMessenger(); void setEnabled(boolean enable); }