Don't call DPM from UserManager to avoid lock inversion (e7927da1) · Commits · e / os / android_frameworks_base

core/java/android/os/UserManagerInternal.java

+12 −0

Original line number	Diff line number	Diff line
		@@ -69,4 +69,16 @@ public abstract class UserManagerInternal {

		/** 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);
		}

services/core/java/com/android/server/pm/UserManagerService.java

+45 −14

Original line number	Diff line number	Diff line
		@@ -24,7 +24,6 @@ import android.app.ActivityManagerInternal;
		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 @@ public class UserManagerService extends IUserManager.Stub {
		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 @@ public class UserManagerService extends IUserManager.Stub {

		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 @@ public class UserManagerService extends IUserManager.Stub {
		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,13 +1600,12 @@ public class UserManagerService extends IUserManager.Stub {
		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()) {
		synchronized (mUsersLock) {
		if (!mIsDeviceManaged) {
		flags \|= UserInfo.FLAG_ADMIN;
		}
		}
		}
		userId = getNextAvailableId();
		userInfo = new UserInfo(userId, name, null, flags);
		userInfo.serialNumber = mNextSerialNumber++;
		@@ -1865,6 +1868,13 @@ public class UserManagerService extends IUserManager.Stub {
		// 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 @@ public class UserManagerService extends IUserManager.Stub {
		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 @@ public class UserManagerService extends IUserManager.Stub {
		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 @@ public class UserManagerService extends IUserManager.Stub {
		synchronized (mGuestRestrictions) {
		UserRestrictionsUtils.dumpRestrictions(pw, " ", mGuestRestrictions);
		}
		synchronized (mUsersLock) {
		pw.println();
		pw.println(" Device managed: " + mIsDeviceManaged);
		}
		}
		}

		@@ -2507,6 +2524,20 @@ public class UserManagerService extends IUserManager.Stub {
		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 {

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

+40 −32

Original line number	Diff line number	Diff line
		@@ -1071,7 +1071,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		}

		Owners newOwners() {
		return new Owners(mContext);
		return new Owners(mContext, getUserManager(), getUserManagerInternal());
		}

		UserManager getUserManager() {
		@@ -2150,8 +2150,13 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		}

		private void ensureDeviceOwnerUserStarted() {
		if (mOwners.hasDeviceOwner()) {
		final int userId = mOwners.getDeviceOwnerUserId();
		final int userId;
		synchronized (this) {
		if (!mOwners.hasDeviceOwner()) {
		return;
		}
		userId = mOwners.getDeviceOwnerUserId();
		}
		if (VERBOSE_LOG) {
		Log.v(LOG_TAG, "Starting non-system DO user: " + userId);
		}
		@@ -2166,7 +2171,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		}
		}
		}
		}

		private void onStartUser(int userId) {
		updateScreenCaptureDisabledInWindowManager(userId,
		@@ -4584,7 +4588,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		+ " for device owner");
		}
		synchronized (this) {
		enforceCanSetDeviceOwner(userId);
		enforceCanSetDeviceOwnerLocked(userId);

		// Shutting down backup manager service permanently.
		long ident = mInjector.binderClearCallingIdentity();
		@@ -4751,7 +4755,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		+ " 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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* - 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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* 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,6 +6823,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		public boolean isProvisioningAllowed(String action) {
		final int callingUserId = mInjector.userHandleGetCallingUserId();
		if (DevicePolicyManager.ACTION_PROVISION_MANAGED_PROFILE.equals(action)) {
		synchronized (this) {
		if (mOwners.hasDeviceOwner()) {
		if (!mInjector.userManagerIsSplitSystemUser()) {
		// Only split-system-user systems support managed-profiles in combination with
		@@ -6835,6 +6840,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		return false;
		}
		}
		}
		if (getProfileOwner(callingUserId) != null) {
		// Managed user cannot have a managed profile.
		return false;
		@@ -6877,9 +6883,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		}

		private boolean isDeviceOwnerProvisioningAllowed(int callingUserId) {
		synchronized (this) {
		if (mOwners.hasDeviceOwner()) {
		return false;
		}
		}
		if (getProfileOwner(callingUserId) != null) {
		return false;
		}

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

+43 −29

Original line number	Diff line number	Diff line
		@@ -23,6 +23,7 @@ import android.content.pm.UserInfo;
		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 @@ import libcore.io.IoUtils;
		/**
		* 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 @@ class Owners {

		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,19 +96,21 @@ class Owners {
		// 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();

		final List<UserInfo> users = mUserManager.getUsers();

		if (readLegacyOwnerFile(legacy)) {
		if (DEBUG) {
		Log.d(TAG, "Legacy config file found.");
		@@ -125,11 +131,13 @@ class Owners {
		// 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();
		}
		}
		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 @@ class Owners {
		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) {

services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -52,7 +52,7 @@ public class DevicePolicyManagerServiceTestable extends DevicePolicyManagerServi
		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);