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

DO NOT MERGE ANYWHERE ConnectivityService: safer locking

Browse Source 
This path changes a dangerous lock path in reportNetworkConnectivity().
This methods is called outside of the main ConnectivityService handler
and takes a lock on a specific NetworkAgentInfo whose connectivity
status is being reported.

While this lock is held, reportNetworkConnectivity() goes on and query
the network policy state for that network, which may ends into
NetworkPolicyManagerService.

Instead, the lock on NetworkAgentInfo is only held long enough to make a
copy of LinkProperties, which is then passed to
NetworkPolicyManagerService without that lock.

Bug: 36902662
Test: could not repro b/36902662, reportNetworkConnectivity() works.
      $ runtest frameworks-net

Change-Id: Iac4b75bcecbdddb0ac695c8b1a87ae755f62f47f
This commit is contained in:
Hugo Benichi
2017-04-06 16:01:44 +09:00
committed by Andriy Naborskyy
parent 276e436a24
commit 57af1a02a6
1 changed files with 23 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
services/core/java/com/android/server/ConnectivityService.java
View File

@@ -1327,14 +1327,17 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
public LinkProperties getLinkProperties(Network network) {
enforceAccessPermission();
NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network);
if (nai != null) {
return getLinkProperties(getNetworkAgentInfoForNetwork(network));
}
private LinkProperties getLinkProperties(NetworkAgentInfo nai) {
if (nai == null) {
return null;
}
synchronized (nai) {
return new LinkProperties(nai.linkProperties);
}
}
return null;
}
private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) {
if (nai != null) {
@@ -3169,7 +3172,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
enforceAccessPermission();
enforceInternetPermission();
NetworkAgentInfo nai;
// TODO: execute this logic on ConnectivityService handler.
final NetworkAgentInfo nai;
if (network == null) {
nai = getDefaultNetwork();
} else {
@@ -3180,21 +3184,24 @@ public class ConnectivityService extends IConnectivityManager.Stub
return;
}
// Revalidate if the app report does not match our current validated state.
if (hasConnectivity == nai.lastValidated) return;
if (hasConnectivity == nai.lastValidated) {
return;
}
final int uid = Binder.getCallingUid();
if (DBG) {
log("reportNetworkConnectivity(" + nai.network.netId + ", " + hasConnectivity +
") by " + uid);
}
synchronized (nai) {
// Validating a network that has not yet connected could result in a call to
// rematchNetworkAndRequests() which is not meant to work on such networks.
if (!nai.everConnected) return;
if (isNetworkWithLinkPropertiesBlocked(nai.linkProperties, uid, false)) return;
nai.networkMonitor.sendMessage(NetworkMonitor.CMD_FORCE_REEVALUATION, uid);
if (!nai.everConnected) {
return;
}
LinkProperties lp = getLinkProperties(nai);
if (isNetworkWithLinkPropertiesBlocked(lp, uid, false)) {
return;
}
nai.networkMonitor.sendMessage(NetworkMonitor.CMD_FORCE_REEVALUATION, uid);
}
private ProxyInfo getDefaultProxy() {