From 8508f2f5a3dbfe17296becea75cd49f116e0bfcb Mon Sep 17 00:00:00 2001 From: jovanak Date: Mon, 28 Oct 2019 09:56:29 -0700 Subject: [PATCH] Fix several issues with precreated users. The feature is currently broken in master - so this CL fixes it. 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: Ib25eac7c27ec3aea2b2a7eef040651bd06590a2e (cherry picked from commit debb009a2cad8a08674a097eb1314ad43a22de7f) --- core/java/android/os/UserManager.java | 8 +++ .../server/pm/PackageManagerService.java | 8 ++- .../java/com/android/server/pm/Settings.java | 4 ++ .../android/server/pm/UserManagerService.java | 54 ++++++++++++++++--- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index 7efe0b7ce389b..8e7da9c08ae42 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -2417,6 +2417,13 @@ public class UserManager { * by {@link #createUser(String, 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 userType the type of user, such as {@link UserManager#USER_TYPE_FULL_GUEST}. @@ -2428,6 +2435,7 @@ public class UserManager { * * @hide */ + @RequiresPermission(android.Manifest.permission.MANAGE_USERS) public @Nullable UserInfo preCreateUser(@NonNull String userType) { try { return mService.preCreateUser(userType); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index cb362b0f0a23a..3cfdf0a0b9578 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -249,7 +249,6 @@ import android.os.storage.VolumeInfo; import android.os.storage.VolumeRecord; import android.permission.IPermissionManager; import android.provider.DeviceConfig; -import android.provider.MediaStore; import android.provider.Settings.Global; import android.provider.Settings.Secure; import android.security.KeyStore; @@ -21940,6 +21939,13 @@ public class PackageManagerService extends IPackageManager.Stub mPermissionManager.onNewUserCreated(userId); } + 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 6653011b8b222..ff2ddc34a77c3 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -3123,6 +3123,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 3515cb71cd59c..fd06d4ab4dba2 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -492,6 +492,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(); + } } } @@ -617,6 +621,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(@UserIdInt int userId) { checkManageUserAndAcrossUsersFullPermission("get user account"); @@ -3079,7 +3110,6 @@ public class UserManagerService extends IUserManager.Stub { @NonNull String userType, @UserInfoFlag int flags, @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages, @NonNull TimingsTraceAndSlog t) { - final UserTypeDetails userTypeDetails = mUserTypes.get(userType); if (userTypeDetails == null) { Slog.e(LOG_TAG, "Cannot create user of invalid user type: " + userType); @@ -3255,9 +3285,9 @@ public class UserManagerService extends IUserManager.Stub { mBaseUserRestrictions.append(userId, restrictions); } - t.traceBegin("PM.onNewUserCreated"); + 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 @@ -3324,11 +3354,16 @@ public class UserManagerService extends IUserManager.Stub { preCreatedUser.preCreated = false; preCreatedUser.creationTime = getCreationTime(); - dispatchUserAddedIntent(preCreatedUser); 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); return preCreatedUser; } @@ -4024,14 +4059,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); } } @@ -4092,7 +4129,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)); } /**