diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index dc4a35b553..9c70e7c4e6 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -429,6 +429,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.function.Predicate; /** * @hide @@ -5323,12 +5324,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void handlePrivateDnsSettingsChanged() { final PrivateDnsConfig cfg = mDnsManager.getPrivateDnsConfig(); - for (NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { handlePerNetworkPrivateDnsConfig(nai, cfg); if (networkRequiresPrivateDnsValidation(nai)) { handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); } - } + }); } private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) { @@ -5651,16 +5652,16 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void maybeDisableForwardRulesForDisconnectingNai( @NonNull final NetworkAgentInfo disconnecting, final boolean sendCallbacks) { // Step 1 : maybe this network was the upstream for one or more local networks. - for (final NetworkAgentInfo local : mNetworkAgentInfos) { - if (!local.isLocalNetwork()) continue; + forEachNetworkAgentInfo(local -> { + if (!local.isLocalNetwork()) return; // return@forEach final NetworkRequest selector = local.localNetworkConfig.getUpstreamSelector(); - if (null == selector) continue; + if (null == selector) return; // return@forEach final NetworkRequestInfo nri = mNetworkRequests.get(selector); // null == nri can happen while disconnecting a network, because destroyNetwork() is // called after removing all associated NRIs from mNetworkRequests. - if (null == nri) continue; + if (null == nri) return; // return@forEach final NetworkAgentInfo satisfier = nri.getSatisfier(); - if (disconnecting != satisfier) continue; + if (disconnecting != satisfier) return; // return@forEach removeLocalNetworkUpstream(local, disconnecting); // Set the satisfier to null immediately so that the LOCAL_NETWORK_CHANGED callback // correctly contains null as an upstream. @@ -5668,7 +5669,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { nri.setSatisfier(null, null); notifyNetworkCallbacks(local, CALLBACK_LOCAL_NETWORK_INFO_CHANGED); } - } + }); // Step 2 : maybe this is a local network that had an upstream. if (!disconnecting.isLocalNetwork()) return; @@ -5841,12 +5842,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { mNetworkRequests.put(req, nri); // TODO: Consider update signal strength for other types. if (req.isListen()) { - for (final NetworkAgentInfo network : mNetworkAgentInfos) { + forEachNetworkAgentInfo(network -> { if (req.networkCapabilities.hasSignalStrength() && network.satisfiesImmutableCapabilitiesOf(req)) { updateSignalStrengthThresholds(network, "REGISTER", req); } - } + }); } else if (req.isRequest() && mNetworkRequestStateStatsMetrics != null) { mNetworkRequestStateStatsMetrics.onNetworkRequestReceived(req); } @@ -6141,13 +6142,13 @@ public class ConnectivityService extends IConnectivityManager.Stub { private void removeListenRequestFromNetworks(@NonNull final NetworkRequest req) { // listens don't have a singular affected Network. Check all networks to see // if this listen request applies and remove it. - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { nai.removeRequest(req.requestId); if (req.networkCapabilities.hasSignalStrength() && nai.satisfiesImmutableCapabilitiesOf(req)) { updateSignalStrengthThresholds(nai, "RELEASE", req); } - } + }); } /** @@ -6210,6 +6211,43 @@ public class ConnectivityService extends IConnectivityManager.Stub { } } + /** + * Perform the specified operation on all networks. + * + * This method will run |op| exactly once for each network currently registered at the + * time it is called, even if |op| adds or removes networks. + * + * @param op the operation to perform. The operation is allowed to disconnect any number of + * networks. + */ + private void forEachNetworkAgentInfo(final Consumer op) { + // Create a copy instead of iterating over the set so |op| is allowed to disconnect any + // number of networks (which removes it from mNetworkAgentInfos). The copy is cheap + // because there are at most a handful of NetworkAgents connected at any given time. + final NetworkAgentInfo[] nais = new NetworkAgentInfo[mNetworkAgentInfos.size()]; + mNetworkAgentInfos.toArray(nais); + for (NetworkAgentInfo nai : nais) { + op.accept(nai); + } + } + + /** + * Check whether the specified condition is true for any network. + * + * This method will stop evaluating as soon as the condition returns true for any network. + * The order of iteration is not contractual. + * + * @param condition the condition to verify. This method must not modify the set of networks in + * any way. + * @return whether {@code condition} returned true for any network + */ + private boolean anyNetworkAgentInfo(final Predicate condition) { + for (int i = mNetworkAgentInfos.size() - 1; i >= 0; i--) { + if (condition.test(mNetworkAgentInfos.valueAt(i))) return true; + } + return false; + } + private RequestInfoPerUidCounter getRequestCounter(NetworkRequestInfo nri) { return hasAnyPermissionOf(mContext, nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) @@ -6551,14 +6589,14 @@ public class ConnectivityService extends IConnectivityManager.Stub { ensureRunningOnConnectivityServiceThread(); // Agent info scores and offer scores depend on whether cells yields to bad wifi. final boolean avoidBadWifi = avoidBadWifi(); - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { nai.updateScoreForNetworkAgentUpdate(); if (avoidBadWifi) { // If the device is now avoiding bad wifi, remove notifications that might have // been put up when the device didn't. mNotifier.clearNotification(nai.network.getNetId(), NotificationType.LOST_INTERNET); } - } + }); // UpdateOfferScore will update mNetworkOffers inline, so make a copy first. final ArrayList offersToUpdate = new ArrayList<>(mNetworkOffers); for (final NetworkOfferInfo noi : offersToUpdate) { @@ -6896,19 +6934,15 @@ public class ConnectivityService extends IConnectivityManager.Stub { final Network underpinnedNetwork = ki.getUnderpinnedNetwork(); final Network network = ki.getNetwork(); - boolean networkFound = false; - boolean underpinnedNetworkFound = false; - for (NetworkAgentInfo n : mNetworkAgentInfos) { - if (n.network.equals(network)) networkFound = true; - if (n.everConnected() && n.network.equals(underpinnedNetwork)) { - underpinnedNetworkFound = true; - } - } + final boolean networkFound = + anyNetworkAgentInfo(n -> n.network.equals(network)); // If the network no longer exists, then the keepalive should have been // cleaned up already. There is no point trying to resume keepalives. if (!networkFound) return; + final boolean underpinnedNetworkFound = anyNetworkAgentInfo( + n -> n.everConnected() && n.network.equals(underpinnedNetwork)); if (underpinnedNetworkFound) { mKeepaliveTracker.handleMonitorAutomaticKeepalive(ki, underpinnedNetwork.netId); @@ -7368,12 +7402,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { return new UnderlyingNetworkInfo[0]; } List infoList = new ArrayList<>(); - for (NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { UnderlyingNetworkInfo info = createVpnInfo(nai); if (info != null) { infoList.add(info); } - } + }); return infoList.toArray(new UnderlyingNetworkInfo[infoList.size()]); } @@ -7451,11 +7485,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { */ private void propagateUnderlyingNetworkCapabilities(Network updatedNetwork) { ensureRunningOnConnectivityServiceThread(); - for (NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { if (updatedNetwork == null || hasUnderlyingNetwork(nai, updatedNetwork)) { updateCapabilitiesForNetwork(nai); } - } + }); } private boolean isUidBlockedByVpn(int uid, List blockedUidRanges) { @@ -7503,11 +7537,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { mPermissionMonitor.updateVpnLockdownUidRanges(requireVpn, ranges); } - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { final boolean curMetered = nai.networkCapabilities.isMetered(); maybeNotifyNetworkBlocked(nai, curMetered, curMetered, mVpnBlockedUidRanges, newVpnBlockedUidRanges); - } + }); mVpnBlockedUidRanges = newVpnBlockedUidRanges; } @@ -9071,6 +9105,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Tracks all NetworkAgents that are currently registered. // NOTE: Only should be accessed on ConnectivityServiceThread, except dump(). + // Code iterating over this set is recommended to use forAllNetworkAgentInfos(), which allows + // code within the loop to disconnect networks during iteration without causing null pointer or + // OOB exceptions. private final ArraySet mNetworkAgentInfos = new ArraySet<>(); // UID ranges for users that are currently blocked by VPNs. @@ -10445,7 +10482,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // A NetworkAgent's allowedUids may need to be updated if the app has lost // carrier config - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { if (nai.networkCapabilities.getAllowedUidsNoCopy().contains(uid) && getSubscriptionIdFromNetworkCaps(nai.networkCapabilities) == subId) { final NetworkCapabilities nc = new NetworkCapabilities(nai.networkCapabilities); @@ -10457,7 +10494,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mCarrierPrivilegeAuthenticator); updateCapabilities(nai.getScore(), nai, nc); } - } + }); } /** @@ -11374,7 +11411,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { throw new IllegalStateException("No user is available"); } - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { ArraySet allowedUidRanges = new ArraySet<>(); for (final UserHandle user : users) { final ArraySet restrictedUidRanges = @@ -11386,7 +11423,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { final UidRangeParcel[] rangesParcel = toUidRangeStableParcels(allowedUidRanges); configs.add(new NativeUidRangeConfig( nai.network.netId, rangesParcel, 0 /* subPriority */)); - } + }); // The netd API replaces the previous configs with the current configs. // Thus, for network disconnection or preference removal, no need to @@ -11608,9 +11645,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Gather the list of all relevant agents. final ArrayList nais = new ArrayList<>(); - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { - nais.add(nai); - } + forEachNetworkAgentInfo(nai -> nais.add(nai)); for (final NetworkRequestInfo nri : networkRequests) { // Non-multilayer listen requests can be ignored. @@ -11716,14 +11751,14 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Don't send CALLBACK_LOCAL_NETWORK_INFO_CHANGED yet though : they should be sent after // onAvailable so clients know what network the change is about. Store such changes in // an array that's only allocated if necessary (because it's almost never necessary). - ArrayList localInfoChangedAgents = null; - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { - if (!nai.isLocalNetwork()) continue; + final ArrayList localInfoChangedAgents = new ArrayList<>(); + forEachNetworkAgentInfo(nai -> { + if (!nai.isLocalNetwork()) return; // return@forEach final NetworkRequest nr = nai.localNetworkConfig.getUpstreamSelector(); - if (null == nr) continue; // No upstream for this local network + if (null == nr) return; // return@forEach, no upstream for this local network final NetworkRequestInfo nri = mNetworkRequests.get(nr); final NetworkReassignment.RequestReassignment change = changes.getReassignment(nri); - if (null == change) continue; // No change in upstreams for this network + if (null == change) return; // return@forEach, no change in upstreams for this network final String fromIface = nai.linkProperties.getInterfaceName(); if (!hasSameInterfaceName(change.mOldNetwork, change.mNewNetwork) || change.mOldNetwork.isDestroyed()) { @@ -11751,9 +11786,8 @@ public class ConnectivityService extends IConnectivityManager.Stub { loge("Can't update forwarding rules", e); } } - if (null == localInfoChangedAgents) localInfoChangedAgents = new ArrayList<>(); localInfoChangedAgents.add(nai); - } + }); // Notify requested networks are available after the default net is switched, but // before LegacyTypeTracker sends legacy broadcasts @@ -11804,17 +11838,14 @@ public class ConnectivityService extends IConnectivityManager.Stub { } // Send LOCAL_NETWORK_INFO_CHANGED callbacks now that onAvailable and onLost have been sent. - if (null != localInfoChangedAgents) { - for (final NetworkAgentInfo nai : localInfoChangedAgents) { - notifyNetworkCallbacks(nai, - CALLBACK_LOCAL_NETWORK_INFO_CHANGED); - } + for (final NetworkAgentInfo nai : localInfoChangedAgents) { + notifyNetworkCallbacks(nai, CALLBACK_LOCAL_NETWORK_INFO_CHANGED); } updateLegacyTypeTrackerAndVpnLockdownForRematch(changes, nais); // Tear down all unneeded networks. - for (NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { if (unneeded(nai, UnneededFor.TEARDOWN)) { if (nai.getInactivityExpiry() > 0) { // This network has active linger timers and no requests, but is not @@ -11832,7 +11863,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { teardownUnneededNetwork(nai); } } - } + }); } /** @@ -12345,7 +12376,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { * @param blockedReasons The reasons for why an uid is blocked. */ private void maybeNotifyNetworkBlockedForNewState(int uid, @BlockedReason int blockedReasons) { - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { final boolean metered = nai.networkCapabilities.isMetered(); final boolean vpnBlocked = isUidBlockedByVpn(uid, mVpnBlockedUidRanges); @@ -12353,9 +12384,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { uid, mUidBlockedReasons.get(uid, BLOCKED_REASON_NONE), metered, vpnBlocked); final int newBlockedState = getBlockedState(uid, blockedReasons, metered, vpnBlocked); - if (oldBlockedState == newBlockedState) { - continue; - } + if (oldBlockedState == newBlockedState) return; // return@forEach for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); NetworkRequestInfo nri = mNetworkRequests.get(nr); @@ -12364,7 +12393,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { newBlockedState); } } - } + }); } @VisibleForTesting @@ -12453,11 +12482,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { activeNetIds.add(nri.getSatisfier().network().netId); } } - for (NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { if (activeNetIds.contains(nai.network().netId) || nai.isVPN()) { defaultNetworks.add(nai.network); } - } + }); return defaultNetworks; } @@ -13348,15 +13377,10 @@ public class ConnectivityService extends IConnectivityManager.Stub { } private boolean ownsVpnRunningOverNetwork(int uid, Network network) { - for (NetworkAgentInfo virtual : mNetworkAgentInfos) { - if (virtual.propagateUnderlyingCapabilities() - && virtual.networkCapabilities.getOwnerUid() == uid - && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) { - return true; - } - } - - return false; + return anyNetworkAgentInfo(virtual -> + virtual.propagateUnderlyingCapabilities() + && virtual.networkCapabilities.getOwnerUid() == uid + && CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)); } @CheckResult @@ -13527,18 +13551,16 @@ public class ConnectivityService extends IConnectivityManager.Stub { @Override public void onInterfaceLinkStateChanged(@NonNull String iface, boolean up) { mHandler.post(() -> { - for (NetworkAgentInfo nai : mNetworkAgentInfos) { - nai.clatd.handleInterfaceLinkStateChanged(iface, up); - } + forEachNetworkAgentInfo(nai -> + nai.clatd.handleInterfaceLinkStateChanged(iface, up)); }); } @Override public void onInterfaceRemoved(@NonNull String iface) { mHandler.post(() -> { - for (NetworkAgentInfo nai : mNetworkAgentInfos) { - nai.clatd.handleInterfaceRemoved(iface); - } + forEachNetworkAgentInfo(nai -> + nai.clatd.handleInterfaceRemoved(iface)); }); } } @@ -14319,7 +14341,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { final long oldIngressRateLimit = mIngressRateLimit; mIngressRateLimit = ConnectivitySettingsManager.getIngressRateLimitInBytesPerSecond( mContext); - for (final NetworkAgentInfo networkAgent : mNetworkAgentInfos) { + forEachNetworkAgentInfo(networkAgent -> { if (canNetworkBeRateLimited(networkAgent)) { // If rate limit has previously been enabled, remove the old limit first. if (oldIngressRateLimit >= 0) { @@ -14330,7 +14352,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mIngressRateLimit); } } - } + }); } private boolean canNetworkBeRateLimited(@NonNull final NetworkAgentInfo networkAgent) {