Don't call DPM from UserManager to avoid lock inversion
- Also make sure DPMS.mOwners is always guarded with DPMS.this.
(and remove synchronization from Owners.)
Bug 25796840
Change-Id: I83f7b78e7b437d9c2a2b1d6e714346cd15f95330
diff --git a/core/java/android/os/UserManagerInternal.java b/core/java/android/os/UserManagerInternal.java
index 26c7a1e..898b6cf 100644
--- a/core/java/android/os/UserManagerInternal.java
+++ b/core/java/android/os/UserManagerInternal.java
@@ -69,4 +69,16 @@
/** Remove a {@link UserRestrictionsListener}. */
public abstract void removeUserRestrictionsListener(UserRestrictionsListener listener);
+
+ /**
+ * Called by {@link com.android.server.devicepolicy.DevicePolicyManagerService} to update
+ * whether the device is managed by device owner.
+ */
+ public abstract void setDeviceManaged(boolean isManaged);
+
+ /**
+ * Called by {@link com.android.server.devicepolicy.DevicePolicyManagerService} to update
+ * whether the user is managed by profile owner.
+ */
+ public abstract void setUserManaged(int userId, boolean isManaged);
}
diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index ff829ff..424b902 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -24,7 +24,6 @@
import android.app.ActivityManagerNative;
import android.app.IActivityManager;
import android.app.IStopUserCallback;
-import android.app.admin.DevicePolicyManager;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
@@ -216,14 +215,15 @@
private final SparseArray<Bundle> mAppliedUserRestrictions = new SparseArray<>();
/**
- * User restrictions set by {@link DevicePolicyManager} that should be applied to all users,
- * including guests.
+ * User restrictions set by {@link com.android.server.devicepolicy.DevicePolicyManagerService}
+ * that should be applied to all users, including guests.
*/
@GuardedBy("mRestrictionsLock")
private Bundle mDevicePolicyGlobalUserRestrictions;
/**
- * User restrictions set by {@link DevicePolicyManager} for each user.
+ * User restrictions set by {@link com.android.server.devicepolicy.DevicePolicyManagerService}
+ * for each user.
*/
@GuardedBy("mRestrictionsLock")
private final SparseArray<Bundle> mDevicePolicyLocalUserRestrictions = new SparseArray<>();
@@ -248,6 +248,12 @@
private final LocalService mLocalService;
+ @GuardedBy("mUsersLock")
+ private boolean mIsDeviceManaged;
+
+ @GuardedBy("mUsersLock")
+ private final SparseBooleanArray mIsUserManaged = new SparseBooleanArray();
+
@GuardedBy("mUserRestrictionsListeners")
private final ArrayList<UserRestrictionsListener> mUserRestrictionsListeners =
new ArrayList<>();
@@ -509,11 +515,9 @@
if (!userInfo.isAdmin()) {
return false;
}
+ // restricted profile can be created if there is no DO set and the admin user has no PO;
+ return !mIsDeviceManaged && !mIsUserManaged.get(userId);
}
- DevicePolicyManager dpm = (DevicePolicyManager) mContext.getSystemService(
- Context.DEVICE_POLICY_SERVICE);
- // restricted profile can be created if there is no DO set and the admin user has no PO
- return !dpm.isDeviceManaged() && dpm.getProfileOwnerAsUser(userId) == null;
}
/*
@@ -1596,11 +1600,10 @@
if (UserManager.isSplitSystemUser()
&& !isGuest && !isManagedProfile && getPrimaryUser() == null) {
flags |= UserInfo.FLAG_PRIMARY;
- DevicePolicyManager devicePolicyManager = (DevicePolicyManager)
- mContext.getSystemService(Context.DEVICE_POLICY_SERVICE);
- if (devicePolicyManager == null
- || !devicePolicyManager.isDeviceManaged()) {
- flags |= UserInfo.FLAG_ADMIN;
+ synchronized (mUsersLock) {
+ if (!mIsDeviceManaged) {
+ flags |= UserInfo.FLAG_ADMIN;
+ }
}
}
userId = getNextAvailableId();
@@ -1865,6 +1868,13 @@
// Remove this user from the list
synchronized (mUsersLock) {
mUsers.remove(userHandle);
+ mIsUserManaged.delete(userHandle);
+ }
+ synchronized (mRestrictionsLock) {
+ mBaseUserRestrictions.remove(userHandle);
+ mAppliedUserRestrictions.remove(userHandle);
+ mCachedEffectiveUserRestrictions.remove(userHandle);
+ mDevicePolicyLocalUserRestrictions.remove(userHandle);
}
// Remove user file
AtomicFile userFile = new AtomicFile(new File(mUsersDir, userHandle + XML_SUFFIX));
@@ -2376,9 +2386,10 @@
if (user == null) {
continue;
}
+ final int userId = user.id;
pw.print(" "); pw.print(user);
pw.print(" serialNo="); pw.print(user.serialNumber);
- if (mRemovingUserIds.get(mUsers.keyAt(i))) {
+ if (mRemovingUserIds.get(userId)) {
pw.print(" <removing> ");
}
if (user.partial) {
@@ -2403,6 +2414,8 @@
sb.append(" ago");
pw.println(sb);
}
+ pw.print(" Has profile owner: ");
+ pw.println(mIsUserManaged.get(userId));
pw.println(" Restrictions:");
synchronized (mRestrictionsLock) {
UserRestrictionsUtils.dumpRestrictions(
@@ -2427,6 +2440,10 @@
synchronized (mGuestRestrictions) {
UserRestrictionsUtils.dumpRestrictions(pw, " ", mGuestRestrictions);
}
+ synchronized (mUsersLock) {
+ pw.println();
+ pw.println(" Device managed: " + mIsDeviceManaged);
+ }
}
}
@@ -2507,6 +2524,20 @@
mUserRestrictionsListeners.remove(listener);
}
}
+
+ @Override
+ public void setDeviceManaged(boolean isManaged) {
+ synchronized (mUsersLock) {
+ mIsDeviceManaged = isManaged;
+ }
+ }
+
+ @Override
+ public void setUserManaged(int userId, boolean isManaged) {
+ synchronized (mUsersLock) {
+ mIsUserManaged.put(userId, isManaged);
+ }
+ }
}
private class Shell extends ShellCommand {
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index f80a611..8199250 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -1071,7 +1071,7 @@
}
Owners newOwners() {
- return new Owners(mContext);
+ return new Owners(mContext, getUserManager(), getUserManagerInternal());
}
UserManager getUserManager() {
@@ -2150,20 +2150,24 @@
}
private void ensureDeviceOwnerUserStarted() {
- if (mOwners.hasDeviceOwner()) {
- final int userId = mOwners.getDeviceOwnerUserId();
- if (VERBOSE_LOG) {
- Log.v(LOG_TAG, "Starting non-system DO user: " + userId);
+ final int userId;
+ synchronized (this) {
+ if (!mOwners.hasDeviceOwner()) {
+ return;
}
- if (userId != UserHandle.USER_SYSTEM) {
- try {
- mInjector.getIActivityManager().startUserInBackground(userId);
+ userId = mOwners.getDeviceOwnerUserId();
+ }
+ if (VERBOSE_LOG) {
+ Log.v(LOG_TAG, "Starting non-system DO user: " + userId);
+ }
+ if (userId != UserHandle.USER_SYSTEM) {
+ try {
+ mInjector.getIActivityManager().startUserInBackground(userId);
- // STOPSHIP Prevent the DO user from being killed.
+ // STOPSHIP Prevent the DO user from being killed.
- } catch (RemoteException e) {
- Slog.w(LOG_TAG, "Exception starting user", e);
- }
+ } catch (RemoteException e) {
+ Slog.w(LOG_TAG, "Exception starting user", e);
}
}
}
@@ -4584,7 +4588,7 @@
+ " for device owner");
}
synchronized (this) {
- enforceCanSetDeviceOwner(userId);
+ enforceCanSetDeviceOwnerLocked(userId);
// Shutting down backup manager service permanently.
long ident = mInjector.binderClearCallingIdentity();
@@ -4751,7 +4755,7 @@
+ " not installed for userId:" + userHandle);
}
synchronized (this) {
- enforceCanSetProfileOwner(userHandle);
+ enforceCanSetProfileOwnerLocked(userHandle);
mOwners.setProfileOwner(who, ownerName, userHandle);
mOwners.writeProfileOwner(userHandle);
return true;
@@ -4953,7 +4957,7 @@
* - SYSTEM_UID
* - adb if there are not accounts.
*/
- private void enforceCanSetProfileOwner(int userHandle) {
+ private void enforceCanSetProfileOwnerLocked(int userHandle) {
UserInfo info = mUserManager.getUserInfo(userHandle);
if (info == null) {
// User doesn't exist.
@@ -4995,7 +4999,7 @@
* The device owner can only be set before the setup phase of the primary user has completed,
* except for adb if no accounts or additional users are present on the device.
*/
- private void enforceCanSetDeviceOwner(int userId) {
+ private void enforceCanSetDeviceOwnerLocked(int userId) {
if (mOwners.hasDeviceOwner()) {
throw new IllegalStateException("Trying to set the device owner, but device owner "
+ "is already set.");
@@ -6819,20 +6823,22 @@
public boolean isProvisioningAllowed(String action) {
final int callingUserId = mInjector.userHandleGetCallingUserId();
if (DevicePolicyManager.ACTION_PROVISION_MANAGED_PROFILE.equals(action)) {
- if (mOwners.hasDeviceOwner()) {
- if (!mInjector.userManagerIsSplitSystemUser()) {
- // Only split-system-user systems support managed-profiles in combination with
- // device-owner.
- return false;
- }
- if (mOwners.getDeviceOwnerUserId() != UserHandle.USER_SYSTEM) {
- // Only system device-owner supports managed-profiles. Non-system device-owner
- // doesn't.
- return false;
- }
- if (callingUserId == UserHandle.USER_SYSTEM) {
- // Managed-profiles cannot be setup on the system user, only regular users.
- return false;
+ synchronized (this) {
+ if (mOwners.hasDeviceOwner()) {
+ if (!mInjector.userManagerIsSplitSystemUser()) {
+ // Only split-system-user systems support managed-profiles in combination with
+ // device-owner.
+ return false;
+ }
+ if (mOwners.getDeviceOwnerUserId() != UserHandle.USER_SYSTEM) {
+ // Only system device-owner supports managed-profiles. Non-system device-owner
+ // doesn't.
+ return false;
+ }
+ if (callingUserId == UserHandle.USER_SYSTEM) {
+ // Managed-profiles cannot be setup on the system user, only regular users.
+ return false;
+ }
}
}
if (getProfileOwner(callingUserId) != null) {
@@ -6877,8 +6883,10 @@
}
private boolean isDeviceOwnerProvisioningAllowed(int callingUserId) {
- if (mOwners.hasDeviceOwner()) {
- return false;
+ synchronized (this) {
+ if (mOwners.hasDeviceOwner()) {
+ return false;
+ }
}
if (getProfileOwner(callingUserId) != null) {
return false;
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java b/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
index 435de7a..f7de0b3 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
@@ -23,6 +23,7 @@
import android.os.Environment;
import android.os.UserHandle;
import android.os.UserManager;
+import android.os.UserManagerInternal;
import android.util.ArrayMap;
import android.util.AtomicFile;
import android.util.Log;
@@ -50,6 +51,9 @@
/**
* Stores and restores state for the Device and Profile owners. By definition there can be
* only one device owner, but there may be a profile owner for each user.
+ *
+ * <p>This class is not thread safe. (i.e. access to this class must always be synchronized
+ * in the caller side.)
*/
class Owners {
private static final String TAG = "DevicePolicyManagerService";
@@ -78,8 +82,8 @@
private static final String TAG_SYSTEM_UPDATE_POLICY = "system-update-policy";
- private final Context mContext;
private final UserManager mUserManager;
+ private final UserManagerInternal mUserManagerInternal;
// Internal state for the device owner package.
private OwnerInfo mDeviceOwner;
@@ -92,44 +96,48 @@
// Local system update policy controllable by device owner.
private SystemUpdatePolicy mSystemUpdatePolicy;
- public Owners(Context context) {
- mContext = context;
- mUserManager = context.getSystemService(UserManager.class);
+ public Owners(Context context, UserManager userManager,
+ UserManagerInternal userManagerInternal) {
+ mUserManager = userManager;
+ mUserManagerInternal = userManagerInternal;
}
/**
* Load configuration from the disk.
*/
void load() {
- synchronized (this) {
- // First, try to read from the legacy file.
- final File legacy = getLegacyConfigFileWithTestOverride();
+ // First, try to read from the legacy file.
+ final File legacy = getLegacyConfigFileWithTestOverride();
- if (readLegacyOwnerFile(legacy)) {
- if (DEBUG) {
- Log.d(TAG, "Legacy config file found.");
- }
+ final List<UserInfo> users = mUserManager.getUsers();
- // Legacy file exists, write to new files and remove the legacy one.
- writeDeviceOwner();
- for (int userId : getProfileOwnerKeys()) {
- writeProfileOwner(userId);
- }
- if (DEBUG) {
- Log.d(TAG, "Deleting legacy config file");
- }
- if (!legacy.delete()) {
- Slog.e(TAG, "Failed to remove the legacy setting file");
- }
- } else {
- // No legacy file, read from the new format files.
- new DeviceOwnerReadWriter().readFromFileLocked();
-
- final List<UserInfo> users = mUserManager.getUsers();
- for (UserInfo ui : users) {
- new ProfileOwnerReadWriter(ui.id).readFromFileLocked();
- }
+ if (readLegacyOwnerFile(legacy)) {
+ if (DEBUG) {
+ Log.d(TAG, "Legacy config file found.");
}
+
+ // Legacy file exists, write to new files and remove the legacy one.
+ writeDeviceOwner();
+ for (int userId : getProfileOwnerKeys()) {
+ writeProfileOwner(userId);
+ }
+ if (DEBUG) {
+ Log.d(TAG, "Deleting legacy config file");
+ }
+ if (!legacy.delete()) {
+ Slog.e(TAG, "Failed to remove the legacy setting file");
+ }
+ } else {
+ // No legacy file, read from the new format files.
+ new DeviceOwnerReadWriter().readFromFileLocked();
+
+ for (UserInfo ui : users) {
+ new ProfileOwnerReadWriter(ui.id).readFromFileLocked();
+ }
+ }
+ mUserManagerInternal.setDeviceManaged(hasDeviceOwner());
+ for (UserInfo ui : users) {
+ mUserManagerInternal.setUserManaged(ui.id, hasProfileOwner(ui.id));
}
if (hasDeviceOwner() && hasProfileOwner(getDeviceOwnerUserId())) {
Slog.w(TAG, String.format("User %d has both DO and PO, which is not supported",
@@ -169,21 +177,27 @@
boolean userRestrictionsMigrated) {
mDeviceOwner = new OwnerInfo(ownerName, admin, userRestrictionsMigrated);
mDeviceOwnerUserId = userId;
+
+ mUserManagerInternal.setDeviceManaged(true);
}
void clearDeviceOwner() {
mDeviceOwner = null;
mDeviceOwnerUserId = UserHandle.USER_NULL;
+
+ mUserManagerInternal.setDeviceManaged(false);
}
void setProfileOwner(ComponentName admin, String ownerName, int userId) {
// For a newly set PO, there's no need for migration.
mProfileOwners.put(userId, new OwnerInfo(ownerName, admin,
/* userRestrictionsMigrated =*/ true));
+ mUserManagerInternal.setUserManaged(userId, true);
}
void removeProfileOwner(int userId) {
mProfileOwners.remove(userId);
+ mUserManagerInternal.setUserManaged(userId, false);
}
ComponentName getProfileOwnerComponent(int userId) {
diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
index 56d6fc0..435b602 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
@@ -52,7 +52,7 @@
private final File mProfileOwnerBase;
public OwnersTestable(DpmMockContext context) {
- super(context);
+ super(context, context.userManager, context.userManagerInternal);
mLegacyFile = new File(context.dataDir, LEGACY_FILE);
mDeviceOwnerFile = new File(context.dataDir, DEVICE_OWNER_FILE);
mProfileOwnerBase = new File(context.dataDir, PROFILE_OWNER_FILE_BASE);