From debb009a2cad8a08674a097eb1314ad43a22de7f Mon Sep 17 00:00:00 2001 From: jovanak Date: Mon, 28 Oct 2019 09:56:29 -0700 Subject: [PATCH] DO NOT MERGE: Fix several issues with precreated users. 1. Prevent UserManager from destroying storage for precreated users. 2. Modify UMS.getUserIds to exclude precreated users. 3. Remove pre-created users if the system has upgraded. 4. Read permissions during conversion to a "real" user. Permissions should have been granted during the pre-creation. If we cannot read permissions, re-grant them for the user. Fixes: 143464654 Fixes: 143463955 Test: Repeated subsequent boots; observing logs; boot systrace; applied OTA, verified user cleanup Change-Id: I75b031105b2622a8a28e84cf2394e43ec93e4174 --- core/java/android/os/UserManager.java | 8 +++ .../server/pm/PackageManagerService.java | 7 +++ .../java/com/android/server/pm/Settings.java | 4 ++ .../android/server/pm/UserManagerService.java | 59 ++++++++++++++++--- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index af9e592f9036d..65e90eecb6381 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -2042,6 +2042,13 @@ public class UserManager { * by {@link #createUser(String, int)} or {@link #createGuest(Context, String)}), it takes * less time. * + *

This method completes the majority of work necessary for user creation: it + * creates user data, CE and DE encryption keys, app data directories, initializes the user and + * grants default permissions. When pre-created users become "real" users, only then are + * components notified of new user creation by firing user creation broadcasts. + * + *

All pre-created users are removed during system upgrade. + * *

Requires {@link android.Manifest.permission#MANAGE_USERS} permission. * * @param flags UserInfo flags that identify the type of user and other properties. @@ -2055,6 +2062,7 @@ public class UserManager { * * @hide */ + @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public @Nullable UserInfo preCreateUser(@UserInfoFlag int flags) { try { return mService.preCreateUser(flags); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c0d3fd5a8c18d..0e46148e5ad4d 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -23713,6 +23713,13 @@ public class PackageManagerService extends IPackageManager.Stub } } + boolean readPermissionStateForUser(@UserIdInt int userId) { + synchronized (mPackages) { + mSettings.readPermissionStateForUserSyncLPr(userId); + return mSettings.areDefaultRuntimePermissionsGrantedLPr(userId); + } + } + @Override public VerifierDeviceIdentity getVerifierDeviceIdentity() throws RemoteException { mContext.enforceCallingOrSelfPermission( diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 11a8f4b895f58..d9e4db29de071 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -3140,6 +3140,10 @@ public final class Settings { return true; } + void readPermissionStateForUserSyncLPr(@UserIdInt int userId) { + mRuntimePermissionsPersistence.readStateForUserSyncLPr(userId); + } + void applyDefaultPreferredAppsLPw(int userId) { // First pull data from any pre-installed apps. final PackageManagerInternal pmInternal = diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index e23ce9e0d1cb0..531c21c5168bf 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -479,6 +479,10 @@ public class UserManagerService extends IUserManager.Stub { public void onBootPhase(int phase) { if (phase == SystemService.PHASE_ACTIVITY_MANAGER_READY) { mUms.cleanupPartialUsers(); + + if (mUms.mPm.isDeviceUpgrading()) { + mUms.cleanupPreCreatedUsers(); + } } } @@ -603,6 +607,33 @@ public class UserManagerService extends IUserManager.Stub { } } + /** + * Removes any pre-created users from the system. Should be invoked after OTAs, to ensure + * pre-created users are not stale. New pre-created pool can be re-created after the update. + */ + void cleanupPreCreatedUsers() { + final ArrayList preCreatedUsers; + synchronized (mUsersLock) { + final int userSize = mUsers.size(); + preCreatedUsers = new ArrayList<>(userSize); + for (int i = 0; i < userSize; i++) { + UserInfo ui = mUsers.valueAt(i).info; + if (ui.preCreated) { + preCreatedUsers.add(ui); + addRemovingUserIdLocked(ui.id); + ui.flags |= UserInfo.FLAG_DISABLED; + ui.partial = true; + } + } + } + final int preCreatedSize = preCreatedUsers.size(); + for (int i = 0; i < preCreatedSize; i++) { + UserInfo ui = preCreatedUsers.get(i); + Slog.i(LOG_TAG, "Removing pre-created user " + ui.id); + removeUserState(ui.id); + } + } + @Override public String getUserAccount(int userId) { checkManageUserAndAcrossUsersFullPermission("get user account"); @@ -2762,11 +2793,17 @@ public class UserManagerService extends IUserManager.Stub { preCreatedUser.preCreated = false; preCreatedUser.creationTime = getCreationTime(); + synchronized (mPackagesLock) { + writeUserLP(preCreatedUserData); + writeUserListLP(); + } + + updateUserIds(); + if (!mPm.readPermissionStateForUser(preCreatedUser.id)) { + // Could not read the existing permissions, re-grant them. + mPm.onNewUserCreated(preCreatedUser.id); + } dispatchUserAddedIntent(preCreatedUser); - - writeUserLP(preCreatedUserData); - writeUserListLP(); - return preCreatedUser; } } @@ -2900,7 +2937,10 @@ public class UserManagerService extends IUserManager.Stub { synchronized (mRestrictionsLock) { mBaseUserRestrictions.append(userId, restrictions); } + + t.traceBegin("PM.onNewUserCreated-" + userId); mPm.onNewUserCreated(userId); + t.traceEnd(); if (preCreate) { // Must start user (which will be stopped right away, through // UserController.finishUserUnlockedCompleted) so services can properly @@ -3599,14 +3639,16 @@ public class UserManagerService extends IUserManager.Stub { synchronized (mUsersLock) { final int userSize = mUsers.size(); for (int i = 0; i < userSize; i++) { - if (!mUsers.valueAt(i).info.partial) { + UserInfo userInfo = mUsers.valueAt(i).info; + if (!userInfo.partial && !userInfo.preCreated) { num++; } } final int[] newUsers = new int[num]; int n = 0; for (int i = 0; i < userSize; i++) { - if (!mUsers.valueAt(i).info.partial) { + UserInfo userInfo = mUsers.valueAt(i).info; + if (!userInfo.partial && !userInfo.preCreated) { newUsers[n++] = mUsers.keyAt(i); } } @@ -3658,7 +3700,10 @@ public class UserManagerService extends IUserManager.Stub { * recycled. */ void reconcileUsers(String volumeUuid) { - mUserDataPreparer.reconcileUsers(volumeUuid, getUsers(true /* excludeDying */)); + mUserDataPreparer.reconcileUsers(volumeUuid, getUsers( + /* excludePartial= */ true, + /* excludeDying= */ true, + /* excludePreCreated= */ false)); } /**