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

Commit 13531cca authored by Rubin Xu's avatar Rubin Xu
Browse files

Fix deadlock in KeyguardDisableHandler

At the moment KeyguardDisableHandler calls into DevicePolicyManager
to retrive the aggregated password quality for the current user
while holding the WindowManager lock. This is a lock inversion
and causes deadlock. To fix this, introduce a per-user password quality
cache in DevicePolicyCache and switch KeyguardDisableHandler
to use that instead.

Test: manual
Fix: 129087668
Change-Id: I8c02ca442dde76ed350f22ac04a52adc82d21d00
parent 9ff409c7
Loading
Loading
Loading
Loading
+11 −0
Original line number Diff line number Diff line
@@ -43,6 +43,12 @@ public abstract class DevicePolicyCache {
     */
    public abstract boolean getScreenCaptureDisabled(@UserIdInt int userHandle);

    /**
     * Caches {@link DevicePolicyManager#getPasswordQuality(android.content.ComponentName)} of the
     * given user with {@code null} passed in as argument.
     */
    public abstract int getPasswordQuality(@UserIdInt int userHandle);

    /**
     * Empty implementation.
     */
@@ -53,5 +59,10 @@ public abstract class DevicePolicyCache {
        public boolean getScreenCaptureDisabled(int userHandle) {
            return false;
        }

        @Override
        public int getPasswordQuality(int userHandle) {
            return DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED;
        }
    }
}
+2 −3
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.wm;
import static com.android.server.wm.WindowManagerDebugConfig.TAG_WITH_CLASS_NAME;
import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;

import android.app.admin.DevicePolicyCache;
import android.app.admin.DevicePolicyManager;
import android.content.Context;
import android.os.Handler;
@@ -125,9 +126,7 @@ class KeyguardDisableHandler {
        return new KeyguardDisableHandler(new Injector() {
            @Override
            public boolean dpmRequiresPassword(int userId) {
                DevicePolicyManager dpm = (DevicePolicyManager) context.getSystemService(
                        Context.DEVICE_POLICY_SERVICE);
                return dpm == null || dpm.getPasswordQuality(null, userId)
                return DevicePolicyCache.getInstance().getPasswordQuality(userId)
                        != DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED;
            }

+31 −0
Original line number Diff line number Diff line
@@ -15,11 +15,16 @@
 */
package com.android.server.devicepolicy;

import android.annotation.UserIdInt;
import android.app.admin.DevicePolicyCache;
import android.app.admin.DevicePolicyManager;
import android.util.SparseBooleanArray;
import android.util.SparseIntArray;

import com.android.internal.annotations.GuardedBy;

import java.io.PrintWriter;

/**
 * Implementation of {@link DevicePolicyCache}, to which {@link DevicePolicyManagerService} pushes
 * policies.
@@ -36,9 +41,13 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache {
    @GuardedBy("mLock")
    private final SparseBooleanArray mScreenCaptureDisabled = new SparseBooleanArray();

    @GuardedBy("mLock")
    private final SparseIntArray mPasswordQuality = new SparseIntArray();

    public void onUserRemoved(int userHandle) {
        synchronized (mLock) {
            mScreenCaptureDisabled.delete(userHandle);
            mPasswordQuality.delete(userHandle);
        }
    }

@@ -54,4 +63,26 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache {
            mScreenCaptureDisabled.put(userHandle, disabled);
        }
    }

    @Override
    public int getPasswordQuality(@UserIdInt int userHandle) {
        synchronized (mLock) {
            return mPasswordQuality.get(userHandle,
                    DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED);
        }
    }

    /** Updat the password quality cache for the given user */
    public void setPasswordQuality(int userHandle, int quality) {
        synchronized (mLock) {
            mPasswordQuality.put(userHandle, quality);
        }
    }

    /** Dump content */
    public void dump(String prefix, PrintWriter pw) {
        pw.println("Device policy cache");
        pw.println(prefix + "Screen capture disabled: " + mScreenCaptureDisabled.toString());
        pw.println(prefix + "Password quality: " + mPasswordQuality.toString());
    }
}
+54 −10
Original line number Diff line number Diff line
@@ -2331,8 +2331,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
                Slog.w(LOG_TAG, "Tried to remove device policy file for user 0! Ignoring.");
                return;
            }
            updatePasswordQualityCacheForUserGroup(userHandle);
            mPolicyCache.onUserRemoved(userHandle);
            mOwners.removeProfileOwner(userHandle);
            mOwners.writeProfileOwner(userHandle);
@@ -3642,6 +3642,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
        updateScreenCaptureDisabled(userId,
                getScreenCaptureDisabled(null, userId));
        pushUserRestrictions(userId);
        // When system user is started (device boot), load cache for all users.
        // This is to mitigate the potential race between loading the cache and keyguard
        // reading the value during user switch, due to onStartUser() being asynchronous.
        updatePasswordQualityCacheForUserGroup(
                userId == UserHandle.USER_SYSTEM ? UserHandle.USER_ALL : userId);
        startOwnerService(userId, "start-user");
    }
@@ -4127,13 +4132,19 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
        synchronized (getLockObject()) {
            ActiveAdmin ap = getActiveAdminForCallerLocked(
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);
            final long ident = mInjector.binderClearCallingIdentity();
            try {
                final PasswordMetrics metrics = ap.minimumPasswordMetrics;
                if (metrics.quality != quality) {
                    metrics.quality = quality;
                    updatePasswordValidityCheckpointLocked(userId, parent);
                    updatePasswordQualityCacheForUserGroup(userId);
                    saveSettingsLocked(userId);
                }
                maybeLogPasswordComplexitySet(who, userId, parent, metrics);
            } finally {
                mInjector.binderRestoreCallingIdentity(ident);
            }
        }
        DevicePolicyEventLogger
                .createEvent(DevicePolicyEnums.SET_PASSWORD_QUALITY)
@@ -4165,6 +4176,32 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
        saveSettingsLocked(credentialOwner);
    }
    /**
     * Update password quality values in policy cache for all users in the same user group as
     * the given user. The cached password quality for user X is the aggregated quality among all
     * admins who have influence of user X's screenlock, i.e. it's equivalent to the return value of
     * getPasswordQuality(null, user X, false).
     *
     * Caches for all users in the same user group often need to be updated alltogether because a
     * user's admin policy can affect another's aggregated password quality in some situation.
     * For example a managed profile's policy will affect the parent user if the profile has unified
     * challenge. A profile can also explicitly set a parent password quality which will affect the
     * aggregated password quality of the parent user.
     */
    private void updatePasswordQualityCacheForUserGroup(@UserIdInt int userId) {
        final List<UserInfo> users;
        if (userId == UserHandle.USER_ALL) {
            users = mUserManager.getUsers();
        } else {
            users = mUserManager.getProfiles(userId);
        }
        for (UserInfo userInfo : users) {
            final int currentUserId = userInfo.id;
            mPolicyCache.setPasswordQuality(currentUserId,
                    getPasswordQuality(null, currentUserId, false));
        }
    }
    @Override
    public int getPasswordQuality(ComponentName who, int userHandle, boolean parent) {
        if (!mHasFeature) {
@@ -8841,6 +8878,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
            mStatLogger.dump(pw, "  ");
            pw.println();
            pw.println("  Encryption Status: " + getEncryptionStatusName(getEncryptionStatus()));
            pw.println();
            mPolicyCache.dump("  ", pw);
        }
    }
@@ -11308,8 +11347,14 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
        @Override
        public void reportSeparateProfileChallengeChanged(@UserIdInt int userId) {
            final long ident = mInjector.binderClearCallingIdentity();
            try {
                synchronized (getLockObject()) {
                    updateMaximumTimeToLockLocked(userId);
                    updatePasswordQualityCacheForUserGroup(userId);
                }
            } finally {
                mInjector.binderRestoreCallingIdentity(ident);
            }
            DevicePolicyEventLogger
                    .createEvent(DevicePolicyEnums.SEPARATE_PROFILE_CHALLENGE_CHANGED)
@@ -11325,7 +11370,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
        @Override
        public CharSequence getPrintingDisabledReasonForUser(@UserIdInt int userId) {
            synchronized (getLockObject()) {
                DevicePolicyData policy = getUserData(userId);
                if (!mUserManager.hasUserRestriction(UserManager.DISALLOW_PRINTING,
                        UserHandle.of(userId))) {
                    Log.e(LOG_TAG, "printing is enabled");