Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cc853296 authored by Miriam Polzer's avatar Miriam Polzer
Browse files

Add GuardedBy annotations to policy engine 

This will help find gaps in synchronization, but currently only fires
when code lines are changed. That means adding these annotations doesn't
necessarily surface all issues.

One can add javacflags: ["-Xep:GuardedBy:ERROR"] to enforce checks on
all lines.

Bug: 374511959
Test: m & Presubmit
Flag: EXEMPT only adding annotations
Change-Id: Ib7676dbdab7ff612610cc05580c656fba7484d3a
parent 1c2633f9

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java

+49 −14
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ import android.util.Log; 
import android.util.SparseArray;

import android.util.Xml;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.util.XmlUtils;

import com.android.modules.utils.TypedXmlPullParser;

import com.android.modules.utils.TypedXmlSerializer;
@@ -120,11 +121,13 @@ final class DevicePolicyEngine { 
    /**

     * Map of <userId, Map<policyKey, policyState>>

     */

    @GuardedBy("mLock")

    private final SparseArray<Map<PolicyKey, PolicyState<?>>> mLocalPolicies;



    /**

     * Map of <policyKey, policyState>

     */

    @GuardedBy("mLock")

    private final Map<PolicyKey, PolicyState<?>> mGlobalPolicies;



    /**
@@ -152,6 +155,7 @@ final class DevicePolicyEngine { 
        mAdminPolicySize = new SparseArray<>();

    }



    @GuardedBy("mLock")

    private void forceEnforcementRefreshIfUserRestrictionLocked(

            @NonNull PolicyDefinition<?> policyDefinition) {

        try {
@@ -185,6 +189,7 @@ final class DevicePolicyEngine { 
        return false;

    }



    @GuardedBy("mLock")

    private void forceEnforcementRefreshLocked(PolicyDefinition<Boolean> policyDefinition) {

        Binder.withCleanCallingIdentity(() -> {

            // Sync global state
@@ -296,6 +301,7 @@ final class DevicePolicyEngine { 
     *

     * <p>Passing a {@code null} value means the policy set by this admin should be removed.

     */

    @GuardedBy("mLock")

    private <V> void setNonCoexistableLocalPolicyLocked(

            PolicyDefinition<V> policyDefinition,

            PolicyState<V> localPolicyState,
@@ -440,6 +446,7 @@ final class DevicePolicyEngine { 
    /**

     * Enforces the new policy and notifies relevant admins.

     */

    @GuardedBy("mLock")

    private <V> void onLocalPolicyChangedLocked(

            @NonNull PolicyDefinition<V> policyDefinition,

            @NonNull EnforcingAdmin enforcingAdmin,
@@ -600,6 +607,7 @@ final class DevicePolicyEngine { 
    /**

     * Enforces the new policy globally and notifies relevant admins.

     */

    @GuardedBy("mLock")

    private <V> void onGlobalPolicyChangedLocked(

            @NonNull PolicyDefinition<V> policyDefinition,

            @NonNull EnforcingAdmin enforcingAdmin) {
@@ -627,6 +635,7 @@ final class DevicePolicyEngine { 
     *

     * <p>Returns {@code true} if the policy is enforced successfully on all users.

     */

    @GuardedBy("mLock")

    private <V> boolean applyGlobalPolicyOnUsersWithLocalPoliciesLocked(

            @NonNull PolicyDefinition<V> policyDefinition,

            @NonNull EnforcingAdmin enforcingAdmin,
@@ -930,6 +939,7 @@ final class DevicePolicyEngine { 
        removePoliciesForAdmin(oldAdmin);

    }



    @GuardedBy("mLock")

    private Set<UserRestrictionPolicyKey> getUserRestrictionPolicyKeysForAdminLocked(

            Map<PolicyKey, PolicyState<?>> policies,

            EnforcingAdmin admin) {
@@ -949,6 +959,7 @@ final class DevicePolicyEngine { 
        return keys;

    }



    @GuardedBy("mLock")

    private <V> boolean hasLocalPolicyLocked(PolicyDefinition<V> policyDefinition, int userId) {

        if (policyDefinition.isGlobalOnlyPolicy()) {

            return false;
@@ -963,6 +974,7 @@ final class DevicePolicyEngine { 
                .getPoliciesSetByAdmins().isEmpty();

    }



    @GuardedBy("mLock")

    private <V> boolean hasGlobalPolicyLocked(PolicyDefinition<V> policyDefinition) {

        if (policyDefinition.isLocalOnlyPolicy()) {

            return false;
@@ -974,6 +986,7 @@ final class DevicePolicyEngine { 
                .isEmpty();

    }



    @GuardedBy("mLock")

    @NonNull

    private <V> PolicyState<V> getLocalPolicyStateLocked(

            PolicyDefinition<V> policyDefinition, int userId) {
@@ -993,6 +1006,7 @@ final class DevicePolicyEngine { 
        return getPolicyStateLocked(mLocalPolicies.get(userId), policyDefinition);

    }



    @GuardedBy("mLock")

    private <V> void removeLocalPolicyStateLocked(

            PolicyDefinition<V> policyDefinition, int userId) {

        if (!mLocalPolicies.contains(userId)) {
@@ -1001,6 +1015,7 @@ final class DevicePolicyEngine { 
        mLocalPolicies.get(userId).remove(policyDefinition.getPolicyKey());

    }



    @GuardedBy("mLock")

    @NonNull

    private <V> PolicyState<V> getGlobalPolicyStateLocked(PolicyDefinition<V> policyDefinition) {

        if (policyDefinition.isLocalOnlyPolicy()) {
@@ -1015,10 +1030,12 @@ final class DevicePolicyEngine { 
        return getPolicyStateLocked(mGlobalPolicies, policyDefinition);

    }



    @GuardedBy("mLock")

    private <V> void removeGlobalPolicyStateLocked(PolicyDefinition<V> policyDefinition) {

        mGlobalPolicies.remove(policyDefinition.getPolicyKey());

    }



    @GuardedBy("mLock")

    private static <V> PolicyState<V> getPolicyStateLocked(

            Map<PolicyKey, PolicyState<?>> policies, PolicyDefinition<V> policyDefinition) {

        try {
@@ -1089,6 +1106,7 @@ final class DevicePolicyEngine { 
    }



    // TODO(b/261430877): Finalise the decision on which admins to send the updates to.

    @GuardedBy("mLock")

    private <V> void sendPolicyChangedToAdminsLocked(

            PolicyState<V> policyState,

            EnforcingAdmin callingAdmin,
@@ -1378,6 +1396,7 @@ final class DevicePolicyEngine { 
        });

    }



    @GuardedBy("mLock")

    private <V> void enforcePolicyOnUserLocked(int userId, PolicyState<V> policyState) {

        if (!policyState.getPolicyDefinition().isInheritable()) {

            return;
@@ -1509,6 +1528,7 @@ final class DevicePolicyEngine { 
     * Called after an admin policy has been added to start binding to the admin if a connection

     * was not already established.

     */

    @GuardedBy("mLock")

    private void updateDeviceAdminServiceOnPolicyAddLocked(@NonNull EnforcingAdmin enforcingAdmin) {

        int userId = enforcingAdmin.getUserId();
@@ -1537,6 +1557,7 @@ final class DevicePolicyEngine { 
     * Called after an admin policy has been removed to stop binding to the admin if they no longer

     * have any policies set.

     */

    @GuardedBy("mLock")

    private void updateDeviceAdminServiceOnPolicyRemoveLocked(

            @NonNull EnforcingAdmin enforcingAdmin) {

        if (doesAdminHavePoliciesLocked(enforcingAdmin)) {
@@ -1562,6 +1583,7 @@ final class DevicePolicyEngine { 
                /* actionForLog= */ "policy-removed");

    }



    @GuardedBy("mLock")

    private boolean doesAdminHavePoliciesLocked(@NonNull EnforcingAdmin enforcingAdmin) {

        for (PolicyKey policy : mGlobalPolicies.keySet()) {

            PolicyState<?> policyState = mGlobalPolicies.get(policy);
@@ -1785,6 +1807,7 @@ final class DevicePolicyEngine { 
        }

    }



    @GuardedBy("mLock")

    <V> void reapplyAllPoliciesOnBootLocked() {

        for (PolicyKey policy : mGlobalPolicies.keySet()) {

            PolicyState<?> policyState = mGlobalPolicies.get(policy);
@@ -1919,6 +1942,7 @@ final class DevicePolicyEngine { 
            }

        }



        @GuardedBy("mLock")

        void writeToFileLocked() {

            Log.d(TAG, "Writing to " + mFile);
@@ -1931,7 +1955,7 @@ final class DevicePolicyEngine { 
                out.startDocument(null, true);



                // Actual content

                writeInner(out);

                writeInnerLocked(out);



                out.endDocument();

                out.flush();
@@ -1948,16 +1972,19 @@ final class DevicePolicyEngine { 
            }

        }



        @GuardedBy("mLock")

        // TODO(b/256846294): Add versioning to read/write

        void writeInner(TypedXmlSerializer serializer) throws IOException {

            writeLocalPoliciesInner(serializer);

            writeGlobalPoliciesInner(serializer);

            writeEnforcingAdminsInner(serializer);

            writeEnforcingAdminSizeInner(serializer);

            writeMaxPolicySizeInner(serializer);

        void writeInnerLocked(TypedXmlSerializer serializer) throws IOException {

            writeLocalPoliciesInnerLocked(serializer);

            writeGlobalPoliciesInnerLocked(serializer);

            writeEnforcingAdminsInnerLocked(serializer);

            writeEnforcingAdminSizeInnerLocked(serializer);

            writeMaxPolicySizeInnerLocked(serializer);

        }



        private void writeLocalPoliciesInner(TypedXmlSerializer serializer) throws IOException {

        @GuardedBy("mLock")

        private void writeLocalPoliciesInnerLocked(TypedXmlSerializer serializer)

                throws IOException {

            if (mLocalPolicies != null) {

                for (int i = 0; i < mLocalPolicies.size(); i++) {

                    int userId = mLocalPolicies.keyAt(i);
@@ -1981,7 +2008,9 @@ final class DevicePolicyEngine { 
            }

        }



        private void writeGlobalPoliciesInner(TypedXmlSerializer serializer) throws IOException {

        @GuardedBy("mLock")

        private void writeGlobalPoliciesInnerLocked(TypedXmlSerializer serializer)

                throws IOException {

            if (mGlobalPolicies != null) {

                for (Map.Entry<PolicyKey, PolicyState<?>> policy : mGlobalPolicies.entrySet()) {

                    serializer.startTag(/* namespace= */ null, TAG_GLOBAL_POLICY_ENTRY);
@@ -1999,7 +2028,9 @@ final class DevicePolicyEngine { 
            }

        }



        private void writeEnforcingAdminsInner(TypedXmlSerializer serializer) throws IOException {

        @GuardedBy("mLock")

        private void writeEnforcingAdminsInnerLocked(TypedXmlSerializer serializer)

                throws IOException {

            if (mEnforcingAdmins != null) {

                for (int i = 0; i < mEnforcingAdmins.size(); i++) {

                    int userId = mEnforcingAdmins.keyAt(i);
@@ -2012,7 +2043,8 @@ final class DevicePolicyEngine { 
            }

        }



        private void writeEnforcingAdminSizeInner(TypedXmlSerializer serializer)

        @GuardedBy("mLock")

        private void writeEnforcingAdminSizeInnerLocked(TypedXmlSerializer serializer)

                throws IOException {

            if (mAdminPolicySize != null) {

                for (int i = 0; i < mAdminPolicySize.size(); i++) {
@@ -2034,7 +2066,8 @@ final class DevicePolicyEngine { 
            }

        }



        private void writeMaxPolicySizeInner(TypedXmlSerializer serializer)

        @GuardedBy("mLock")

        private void writeMaxPolicySizeInnerLocked(TypedXmlSerializer serializer)

                throws IOException {

            serializer.startTag(/* namespace= */ null, TAG_MAX_POLICY_SIZE_LIMIT);

            serializer.attributeInt(
@@ -2042,6 +2075,7 @@ final class DevicePolicyEngine { 
            serializer.endTag(/* namespace= */ null, TAG_MAX_POLICY_SIZE_LIMIT);

        }



        @GuardedBy("mLock")

        void readFromFileLocked() {

            if (!mFile.exists()) {

                Log.d(TAG, "" + mFile + " doesn't exist");
@@ -2055,7 +2089,7 @@ final class DevicePolicyEngine { 
                input = f.openRead();

                TypedXmlPullParser parser = Xml.resolvePullParser(input);



                readInner(parser);

                readInnerLocked(parser);



            } catch (XmlPullParserException | IOException | ClassNotFoundException e) {

                Slogf.wtf(TAG, "Error parsing resources file", e);
@@ -2064,7 +2098,8 @@ final class DevicePolicyEngine { 
            }

        }



        private void readInner(TypedXmlPullParser parser)

        @GuardedBy("mLock")

        private void readInnerLocked(TypedXmlPullParser parser)

                throws IOException, XmlPullParserException, ClassNotFoundException {

            int outerDepth = parser.getDepth();

            while (XmlUtils.nextElementWithin(parser, outerDepth)) {