Merge "Guard mUidBlockedState with a separate lock." into tm-dev

services/core/java/com/android/server/net/NetworkPolicyManagerService.java

@@ -90,6 +90,8 @@ import static android.net.NetworkPolicyManager.RULE_REJECT_METERED;
 import static android.net.NetworkPolicyManager.RULE_REJECT_RESTRICTED_MODE;
 import static android.net.NetworkPolicyManager.RULE_TEMPORARY_ALLOW_METERED;
 import static android.net.NetworkPolicyManager.SUBSCRIPTION_OVERRIDE_UNMETERED;
+import static android.net.NetworkPolicyManager.allowedReasonsToString;
+import static android.net.NetworkPolicyManager.blockedReasonsToString;
 import static android.net.NetworkPolicyManager.isProcStateAllowedWhileIdleOrPowerSaveMode;
 import static android.net.NetworkPolicyManager.isProcStateAllowedWhileInLowPowerStandby;
 import static android.net.NetworkPolicyManager.isProcStateAllowedWhileOnRestrictBackground;
@@ -594,7 +596,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     @GuardedBy("mUidRulesFirstLock")
     private final SparseArray<UidState> mUidState = new SparseArray<>();
 
-    @GuardedBy("mUidRulesFirstLock")
+    @GuardedBy("mUidBlockedState")
     private final SparseArray<UidBlockedState> mUidBlockedState = new SparseArray<>();
 
     /** Objects used temporarily while computing the new blocked state for each uid. */
@@ -3930,7 +3932,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
                 final SparseBooleanArray knownUids = new SparseBooleanArray();
                 collectKeys(mUidState, knownUids);
+                synchronized (mUidBlockedState) {
                     collectKeys(mUidBlockedState, knownUids);
+                }
 
                 fout.println("Status for all known UIDs:");
                 fout.increaseIndent();
@@ -3948,12 +3952,14 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                         fout.print(uidState.toString());
                     }
 
+                    synchronized (mUidBlockedState) {
                         final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
                         if (uidBlockedState == null) {
                             fout.print(" blocked_state={null}");
                         } else {
                             fout.print(" blocked_state=");
-                        fout.print(uidBlockedState.toString());
+                            fout.print(uidBlockedState);
+                        }
                     }
                     fout.println();
                 }
@@ -4128,9 +4134,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         mUidFirewallRestrictedModeRules.clear();
         forEachUid("updateRestrictedModeAllowlist", uid -> {
             synchronized (mUidRulesFirstLock) {
-                final UidBlockedState uidBlockedState = updateBlockedReasonsForRestrictedModeUL(
+                final int effectiveBlockedReasons = updateBlockedReasonsForRestrictedModeUL(
                         uid);
-                final int newFirewallRule = getRestrictedModeFirewallRule(uidBlockedState);
+                final int newFirewallRule = getRestrictedModeFirewallRule(effectiveBlockedReasons);
 
                 // setUidFirewallRulesUL will allowlist all uids that are passed to it, so only add
                 // non-default rules.
@@ -4150,7 +4156,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     @VisibleForTesting
     @GuardedBy("mUidRulesFirstLock")
     void updateRestrictedModeForUidUL(int uid) {
-        final UidBlockedState uidBlockedState = updateBlockedReasonsForRestrictedModeUL(uid);
+        final int effectiveBlockedReasons = updateBlockedReasonsForRestrictedModeUL(uid);
 
         // if restricted networking mode is on, and the app has an access exemption, the uid rule
         // will not change, but the firewall rule will have to be updated.
@@ -4158,37 +4164,48 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             // Note: setUidFirewallRule also updates mUidFirewallRestrictedModeRules.
             // In this case, default firewall rules can also be added.
             setUidFirewallRuleUL(FIREWALL_CHAIN_RESTRICTED, uid,
-                    getRestrictedModeFirewallRule(uidBlockedState));
+                    getRestrictedModeFirewallRule(effectiveBlockedReasons));
         }
     }
 
     @GuardedBy("mUidRulesFirstLock")
-    private UidBlockedState updateBlockedReasonsForRestrictedModeUL(int uid) {
+    private int updateBlockedReasonsForRestrictedModeUL(int uid) {
+        final boolean hasRestrictedModeAccess = hasRestrictedModeAccess(uid);
+        final int oldEffectiveBlockedReasons;
+        final int newEffectiveBlockedReasons;
+        final int uidRules;
+        synchronized (mUidBlockedState) {
             final UidBlockedState uidBlockedState = getOrCreateUidBlockedStateForUid(
                     mUidBlockedState, uid);
-        final int oldEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
+            oldEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
             if (mRestrictedNetworkingMode) {
                 uidBlockedState.blockedReasons |= BLOCKED_REASON_RESTRICTED_MODE;
             } else {
                 uidBlockedState.blockedReasons &= ~BLOCKED_REASON_RESTRICTED_MODE;
             }
-        if (hasRestrictedModeAccess(uid)) {
+            if (hasRestrictedModeAccess) {
                 uidBlockedState.allowedReasons |= ALLOWED_REASON_RESTRICTED_MODE_PERMISSIONS;
             } else {
                 uidBlockedState.allowedReasons &= ~ALLOWED_REASON_RESTRICTED_MODE_PERMISSIONS;
             }
             uidBlockedState.updateEffectiveBlockedReasons();
-        if (oldEffectiveBlockedReasons != uidBlockedState.effectiveBlockedReasons) {
+
+            newEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
+            uidRules = oldEffectiveBlockedReasons == newEffectiveBlockedReasons
+                    ? RULE_NONE
+                    : uidBlockedState.deriveUidRules();
+        }
+        if (oldEffectiveBlockedReasons != newEffectiveBlockedReasons) {
             postBlockedReasonsChangedMsg(uid,
-                    uidBlockedState.effectiveBlockedReasons, oldEffectiveBlockedReasons);
+                    newEffectiveBlockedReasons, oldEffectiveBlockedReasons);
 
-            postUidRulesChangedMsg(uid, uidBlockedState.deriveUidRules());
+            postUidRulesChangedMsg(uid, uidRules);
         }
-        return uidBlockedState;
+        return newEffectiveBlockedReasons;
     }
 
-    private static int getRestrictedModeFirewallRule(UidBlockedState uidBlockedState) {
+    private static int getRestrictedModeFirewallRule(int effectiveBlockedReasons) {
-        if ((uidBlockedState.effectiveBlockedReasons & BLOCKED_REASON_RESTRICTED_MODE) != 0) {
+        if ((effectiveBlockedReasons & BLOCKED_REASON_RESTRICTED_MODE) != 0) {
             // rejected in restricted mode, this is the default behavior.
             return FIREWALL_RULE_DEFAULT;
         } else {
@@ -4292,12 +4309,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 mUidFirewallLowPowerStandbyModeRules.clear();
                 for (int i = mUidState.size() - 1; i >= 0; i--) {
                     final int uid = mUidState.keyAt(i);
-                    UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
+                    final int effectiveBlockedReasons = getEffectiveBlockedReasons(uid);
-                    if (hasInternetPermissionUL(uid) && uidBlockedState != null
+                    if (hasInternetPermissionUL(uid) && (effectiveBlockedReasons
-                            && (uidBlockedState.effectiveBlockedReasons
                                     & BLOCKED_REASON_LOW_POWER_STANDBY) == 0) {
-                        mUidFirewallLowPowerStandbyModeRules.put(mUidBlockedState.keyAt(i),
+                        mUidFirewallLowPowerStandbyModeRules.put(uid, FIREWALL_RULE_ALLOW);
-                                FIREWALL_RULE_ALLOW);
                     }
                 }
                 setUidFirewallRulesUL(FIREWALL_CHAIN_LOW_POWER_STANDBY,
@@ -4316,10 +4331,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             return;
         }
 
-        final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
+        final int effectiveBlockedReasons = getEffectiveBlockedReasons(uid);
-        if (mUidState.contains(uid) && uidBlockedState != null
+        if (mUidState.contains(uid)
-                && (uidBlockedState.effectiveBlockedReasons & BLOCKED_REASON_LOW_POWER_STANDBY)
+                && (effectiveBlockedReasons & BLOCKED_REASON_LOW_POWER_STANDBY) == 0) {
-                == 0) {
             mUidFirewallLowPowerStandbyModeRules.put(uid, FIREWALL_RULE_ALLOW);
             setUidFirewallRuleUL(FIREWALL_CHAIN_LOW_POWER_STANDBY, uid, FIREWALL_RULE_ALLOW);
         } else {
@@ -4448,9 +4462,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             if (!isUidValidForDenylistRulesUL(uid)) {
                 continue;
             }
-            final UidBlockedState uidBlockedState = getOrCreateUidBlockedStateForUid(
+            final int blockedReasons = getBlockedReasons(uid);
-                    mUidBlockedState, uid);
+            if (!enableChain && (blockedReasons & ~BLOCKED_METERED_REASON_MASK)
-            if (!enableChain && (uidBlockedState.blockedReasons & ~BLOCKED_METERED_REASON_MASK)
                     == BLOCKED_REASON_NONE) {
                 // Chain isn't enabled and the uid had no restrictions to begin with.
                 continue;
@@ -4692,7 +4705,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     @GuardedBy("mUidRulesFirstLock")
     private void onUidDeletedUL(int uid) {
         // First cleanup in-memory state synchronously...
+        synchronized (mUidBlockedState) {
             mUidBlockedState.delete(uid);
+        }
         mUidPolicy.delete(uid);
         mUidFirewallStandbyRules.delete(uid);
         mUidFirewallDozableRules.delete(uid);
@@ -4804,11 +4819,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         final int uidPolicy = mUidPolicy.get(uid, POLICY_NONE);
         final boolean isForeground = isUidForegroundOnRestrictBackgroundUL(uid);
         final boolean isRestrictedByAdmin = isRestrictedByAdminUL(uid);
-        final UidBlockedState uidBlockedState = getOrCreateUidBlockedStateForUid(
-                mUidBlockedState, uid);
-        final UidBlockedState previousUidBlockedState = getOrCreateUidBlockedStateForUid(
-                mTmpUidBlockedState, uid);
-        previousUidBlockedState.copyFrom(uidBlockedState);
 
         final boolean isDenied = (uidPolicy & POLICY_REJECT_METERED_BACKGROUND) != 0;
         final boolean isAllowed = (uidPolicy & POLICY_ALLOW_METERED_BACKGROUND) != 0;
@@ -4823,18 +4833,47 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         newAllowedReasons |= (isForeground ? ALLOWED_METERED_REASON_FOREGROUND : 0);
         newAllowedReasons |= (isAllowed ? ALLOWED_METERED_REASON_USER_EXEMPTED : 0);
 
+        final int oldEffectiveBlockedReasons;
+        final int newEffectiveBlockedReasons;
+        final int oldAllowedReasons;
+        final int uidRules;
+        synchronized (mUidBlockedState) {
+            final UidBlockedState uidBlockedState = getOrCreateUidBlockedStateForUid(
+                    mUidBlockedState, uid);
+            final UidBlockedState previousUidBlockedState = getOrCreateUidBlockedStateForUid(
+                    mTmpUidBlockedState, uid);
+            previousUidBlockedState.copyFrom(uidBlockedState);
+
             uidBlockedState.blockedReasons = (uidBlockedState.blockedReasons
                     & ~BLOCKED_METERED_REASON_MASK) | newBlockedReasons;
             uidBlockedState.allowedReasons = (uidBlockedState.allowedReasons
                     & ~ALLOWED_METERED_REASON_MASK) | newAllowedReasons;
             uidBlockedState.updateEffectiveBlockedReasons();
-        final int oldEffectiveBlockedReasons = previousUidBlockedState.effectiveBlockedReasons;
+
-        final int newEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
+            oldEffectiveBlockedReasons = previousUidBlockedState.effectiveBlockedReasons;
+            newEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
+            oldAllowedReasons = previousUidBlockedState.allowedReasons;
+            uidRules = (oldEffectiveBlockedReasons == newEffectiveBlockedReasons)
+                    ? RULE_NONE : uidBlockedState.deriveUidRules();
+
+            if (LOGV) {
+                Log.v(TAG, "updateRuleForRestrictBackgroundUL(" + uid + ")"
+                        + ": isForeground=" + isForeground
+                        + ", isDenied=" + isDenied
+                        + ", isAllowed=" + isAllowed
+                        + ", isRestrictedByAdmin=" + isRestrictedByAdmin
+                        + ", oldBlockedState=" + previousUidBlockedState
+                        + ", newBlockedState=" + uidBlockedState
+                        + ", newBlockedMeteredReasons=" + blockedReasonsToString(newBlockedReasons)
+                        + ", newAllowedMeteredReasons=" + allowedReasonsToString(
+                                newAllowedReasons));
+            }
+        }
         if (oldEffectiveBlockedReasons != newEffectiveBlockedReasons) {
             postBlockedReasonsChangedMsg(uid,
                     newEffectiveBlockedReasons, oldEffectiveBlockedReasons);
 
-            postUidRulesChangedMsg(uid, uidBlockedState.deriveUidRules());
+            postUidRulesChangedMsg(uid, uidRules);
         }
 
         // Note that the conditionals below are for avoiding unnecessary calls to netd.
@@ -4850,29 +4889,11 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         }
         final int allowlistReasons = ALLOWED_METERED_REASON_FOREGROUND
                 | ALLOWED_METERED_REASON_USER_EXEMPTED;
-        final int oldAllowedReasons = previousUidBlockedState.allowedReasons;
         if ((oldAllowedReasons & allowlistReasons) != ALLOWED_REASON_NONE
                 || (newAllowedReasons & allowlistReasons) != ALLOWED_REASON_NONE) {
             setMeteredNetworkAllowlist(uid,
                     (newAllowedReasons & allowlistReasons) != ALLOWED_REASON_NONE);
         }
-
-        if (LOGV) {
-            Log.v(TAG, "updateRuleForRestrictBackgroundUL(" + uid + ")"
-                    + ": isForeground=" +isForeground
-                    + ", isDenied=" + isDenied
-                    + ", isAllowed=" + isAllowed
-                    + ", isRestrictedByAdmin=" + isRestrictedByAdmin
-                    + ", oldBlockedState=" + previousUidBlockedState.toString()
-                    + ", newBlockedState=" + uidBlockedState.toString()
-                    + ", oldBlockedMeteredReasons=" + NetworkPolicyManager.blockedReasonsToString(
-                    uidBlockedState.blockedReasons & BLOCKED_METERED_REASON_MASK)
-                    + ", oldBlockedMeteredEffectiveReasons="
-                    + NetworkPolicyManager.blockedReasonsToString(
-                    uidBlockedState.effectiveBlockedReasons & BLOCKED_METERED_REASON_MASK)
-                    + ", oldAllowedMeteredReasons=" + NetworkPolicyManager.blockedReasonsToString(
-                    uidBlockedState.allowedReasons & BLOCKED_METERED_REASON_MASK));
-        }
     }
 
     /**
@@ -4932,6 +4953,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
         final boolean isWhitelisted = isWhitelistedFromPowerSaveUL(uid, mDeviceIdleMode);
 
+        final int oldEffectiveBlockedReasons;
+        final int newEffectiveBlockedReasons;
+        final int uidRules;
+        synchronized (mUidBlockedState) {
             final UidBlockedState uidBlockedState = getOrCreateUidBlockedStateForUid(
                     mUidBlockedState, uid);
             final UidBlockedState previousUidBlockedState = getOrCreateUidBlockedStateForUid(
@@ -4963,14 +4988,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             uidBlockedState.allowedReasons = (uidBlockedState.allowedReasons
                     & ALLOWED_METERED_REASON_MASK) | newAllowedReasons;
             uidBlockedState.updateEffectiveBlockedReasons();
-        if (previousUidBlockedState.effectiveBlockedReasons
-                != uidBlockedState.effectiveBlockedReasons) {
-            postBlockedReasonsChangedMsg(uid,
-                    uidBlockedState.effectiveBlockedReasons,
-                    previousUidBlockedState.effectiveBlockedReasons);
-
-            postUidRulesChangedMsg(uid, uidBlockedState.deriveUidRules());
-        }
 
             if (LOGV) {
                 Log.v(TAG, "updateRulesForPowerRestrictionsUL(" + uid + ")"
@@ -4980,8 +4997,22 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                         + ", isForeground=" + isForeground
                         + ", isTop=" + isTop
                         + ", isWhitelisted=" + isWhitelisted
-                    + ", oldUidBlockedState=" + previousUidBlockedState.toString()
+                        + ", oldUidBlockedState=" + previousUidBlockedState
-                    + ", newUidBlockedState=" + uidBlockedState.toString());
+                        + ", newUidBlockedState=" + uidBlockedState);
+            }
+
+            oldEffectiveBlockedReasons = previousUidBlockedState.effectiveBlockedReasons;
+            newEffectiveBlockedReasons = uidBlockedState.effectiveBlockedReasons;
+            uidRules = oldEffectiveBlockedReasons == newEffectiveBlockedReasons
+                    ? RULE_NONE
+                    : uidBlockedState.deriveUidRules();
+        }
+        if (oldEffectiveBlockedReasons != newEffectiveBlockedReasons) {
+            postBlockedReasonsChangedMsg(uid,
+                    oldEffectiveBlockedReasons,
+                    newEffectiveBlockedReasons);
+
+            postUidRulesChangedMsg(uid, uidRules);
         }
     }
 
@@ -5766,7 +5797,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
         mContext.enforceCallingOrSelfPermission(OBSERVE_NETWORK_POLICY, TAG);
         int blockedReasons;
-        synchronized (mUidRulesFirstLock) {
+        synchronized (mUidBlockedState) {
             final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
             blockedReasons = uidBlockedState == null
                     ? BLOCKED_REASON_NONE : uidBlockedState.effectiveBlockedReasons;
@@ -5784,7 +5815,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     @Override
     public boolean isUidRestrictedOnMeteredNetworks(int uid) {
         mContext.enforceCallingOrSelfPermission(OBSERVE_NETWORK_POLICY, TAG);
-        synchronized (mUidRulesFirstLock) {
+        synchronized (mUidBlockedState) {
             final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
             int blockedReasons = uidBlockedState == null
                     ? BLOCKED_REASON_NONE : uidBlockedState.effectiveBlockedReasons;
@@ -6032,10 +6063,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         return restrictedUids != null && restrictedUids.contains(uid);
     }
 
-    private static boolean hasRule(int uidRules, int rule) {
-        return (uidRules & rule) != 0;
-    }
-
     private static boolean getBooleanDefeatingNullable(@Nullable PersistableBundle bundle,
             String key, boolean defaultValue) {
         return (bundle != null) ? bundle.getBoolean(key, defaultValue) : defaultValue;
@@ -6051,16 +6078,39 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         return uidBlockedState;
     }
 
+    private int getEffectiveBlockedReasons(int uid) {
+        synchronized (mUidBlockedState) {
+            final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
+            return uidBlockedState == null
+                    ? BLOCKED_REASON_NONE
+                    : uidBlockedState.effectiveBlockedReasons;
+        }
+    }
+
+    private int getBlockedReasons(int uid) {
+        synchronized (mUidBlockedState) {
+            final UidBlockedState uidBlockedState = mUidBlockedState.get(uid);
+            return uidBlockedState == null
+                    ? BLOCKED_REASON_NONE
+                    : uidBlockedState.blockedReasons;
+        }
+    }
+
     @VisibleForTesting
     static final class UidBlockedState {
         public int blockedReasons;
         public int allowedReasons;
         public int effectiveBlockedReasons;
 
+        private UidBlockedState(int blockedReasons, int allowedReasons,
+                int effectiveBlockedReasons) {
+            this.blockedReasons = blockedReasons;
+            this.allowedReasons = allowedReasons;
+            this.effectiveBlockedReasons = effectiveBlockedReasons;
+        }
+
         UidBlockedState() {
-            blockedReasons = BLOCKED_REASON_NONE;
+            this(BLOCKED_REASON_NONE, ALLOWED_REASON_NONE, BLOCKED_REASON_NONE);
-            allowedReasons = ALLOWED_REASON_NONE;
-            effectiveBlockedReasons = BLOCKED_REASON_NONE;
         }
 
         void updateEffectiveBlockedReasons() {
@@ -6297,7 +6347,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 }
             }
             if (LOGV) {
-                Slog.v(TAG, "uidBlockedState=" + this.toString()
+                Slog.v(TAG, "uidBlockedState=" + this
                         + " -> uidRule=" + uidRulesToString(uidRule));
             }
             return uidRule;