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

Commit 6cc84a14 authored by Martijn Coenen's avatar Martijn Coenen
Browse files

Fix race condition around CE storage becoming available. 

This fixes a race condition that exists around preparing CE storage, and
a package being reinstalled at the same time.

The normal CE data unlock procedure is something like this:

1. User state is RUNNING_LOCKED
2. UserController calls onBeforeUnlockUser()
3. onBeforeUnlockUser() prepares CE storage
4. onBeforeUnlockUser() calls PackageManagerService.reconcileAppsData()
5. reconcileAppsData() prepares app CE data directories, creating dirs
   when needed
6. UserController changes state to RUNNING_UNLOCKING

The race comes into the picture when an app is installed right between
step 5 and step 6; when a new app is installed, PMS does have a function
to create the CE app data directory; but that function only creates CE
data directories when the user state is RUNNING_UNLOCKING (or later); so
even though technically CE storage became available in step 3, other
parts of PMS will only use it after step 6 has completed.

To fix this, we use StorageManagerService to record the fact that CE
storage for a particular user is prepared; then, PackageManagerService
can query StorageManager to ask whether CE storage is prepared. That in
combination with the user key being unlocked is then used as a condition
for creating CE data directories from PackageManagerService.

Bug: 187103629
Test: manual
Change-Id: Ice81ae98441b9ed287e8db4196a041c3d47afce9
Merged-In: Ice81ae98441b9ed287e8db4196a041c3d47afce9
parent 938b7d01

core/java/android/os/storage/StorageManagerInternal.java

+16 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.os.storage; 


import android.annotation.NonNull;

import android.annotation.Nullable;

import android.annotation.UserIdInt;

import android.os.IVold;



import java.util.List;
@@ -135,4 +136,19 @@ public abstract class StorageManagerInternal { 
     * {@link VolumeInfo#isPrimary()}

     */

    public abstract List<String> getPrimaryVolumeIds();



    /**

     * Tells StorageManager that CE storage for this user has been prepared.

     *

     * @param userId userId for which CE storage has been prepared

     */

    public abstract void markCeStoragePrepared(@UserIdInt int userId);



    /**

     * Returns true when CE storage for this user has been prepared.

     *

     * When the user key is unlocked and CE storage has been prepared,

     * it's ok to access and modify CE directories on volumes for this user.

     */

    public abstract boolean isCeStoragePrepared(@UserIdInt int userId);

}

services/core/java/com/android/server/StorageManagerService.java

+17 −0
Original line number Diff line number Diff line
@@ -221,6 +221,9 @@ class StorageManagerService extends IStorageManager.Stub 
    @GuardedBy("mLock")

    private final Set<Integer> mFuseMountedUser = new ArraySet<>();



    @GuardedBy("mLock")

    private final Set<Integer> mCeStoragePreparedUsers = new ArraySet<>();



    public static class Lifecycle extends SystemService {

        private StorageManagerService mStorageManagerService;
@@ -4864,5 +4867,19 @@ class StorageManagerService extends IStorageManager.Stub 
            }

            return primaryVolumeIds;

        }



        @Override

        public void markCeStoragePrepared(int userId) {

            synchronized (mLock) {

                mCeStoragePreparedUsers.add(userId);

            }

        }



        @Override

        public boolean isCeStoragePrepared(int userId) {

            synchronized (mLock) {

                return mCeStoragePreparedUsers.contains(userId);

            }

        }

    }

}

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

+7 −3
Original line number Diff line number Diff line
@@ -22646,8 +22646,9 @@ public class PackageManagerService extends IPackageManager.Stub 
        removeKeystoreDataIfNeeded(mInjector.getUserManagerInternal(), userId, appId);















        UserManagerInternal umInternal = mInjector.getUserManagerInternal();













        StorageManagerInternal smInternal = mInjector.getLocalService(StorageManagerInternal.class);















        final int flags;













        if (umInternal.isUserUnlockingOrUnlocked(userId)) {













        if (StorageManager.isUserKeyUnlocked(userId) && smInternal.isCeStoragePrepared(userId)) {















            flags = StorageManager.FLAG_STORAGE_DE | StorageManager.FLAG_STORAGE_CE;















        } else if (umInternal.isUserRunning(userId)) {















            flags = StorageManager.FLAG_STORAGE_DE;










@@ -25512,9 +25513,11 @@ public class PackageManagerService extends IPackageManager.Stub













        // Reconcile app data for all started/unlocked users















        final StorageManager sm = mInjector.getSystemService(StorageManager.class);















        UserManagerInternal umInternal = mInjector.getUserManagerInternal();













        StorageManagerInternal smInternal = mInjector.getLocalService(StorageManagerInternal.class);















        for (UserInfo user : mUserManager.getUsers(false /* includeDying */)) {















            final int flags;













            if (umInternal.isUserUnlockingOrUnlocked(user.id)) {













            if (StorageManager.isUserKeyUnlocked(user.id)













                    && smInternal.isCeStoragePrepared(user.id)) {















                flags = StorageManager.FLAG_STORAGE_DE | StorageManager.FLAG_STORAGE_CE;















            } else if (umInternal.isUserRunning(user.id)) {















                flags = StorageManager.FLAG_STORAGE_DE;










@@ -25854,7 +25857,8 @@ public class PackageManagerService extends IPackageManager.Stub













        StorageManagerInternal smInternal = mInjector.getLocalService(StorageManagerInternal.class);















        for (UserInfo user : mUserManager.getUsers(false /*excludeDying*/)) {















            final int flags;













            if (umInternal.isUserUnlockingOrUnlocked(user.id)) {













            if (StorageManager.isUserKeyUnlocked(user.id)













                    && smInternal.isCeStoragePrepared(user.id)) {















                flags = StorageManager.FLAG_STORAGE_DE | StorageManager.FLAG_STORAGE_CE;















            } else if (umInternal.isUserRunning(user.id)) {















                flags = StorageManager.FLAG_STORAGE_DE;

























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



+5
−0




























Original line number
Diff line number
Diff line








@@ -80,6 +80,7 @@ import android.os.UserManager;













import android.os.UserManager.EnforcingUser;















import android.os.UserManager.QuietModeFlag;















import android.os.storage.StorageManager;













import android.os.storage.StorageManagerInternal;















import android.provider.Settings;















import android.security.GateKeeper;















import android.service.gatekeeper.IGateKeeperService;










@@ -4815,6 +4816,10 @@ public class UserManagerService extends IUserManager.Stub {













        // Migrate only if build fingerprints mismatch















        boolean migrateAppsData = !Build.FINGERPRINT.equals(userInfo.lastLoggedInFingerprint);















        mUserDataPreparer.prepareUserData(userId, userSerial, StorageManager.FLAG_STORAGE_CE);



























        StorageManagerInternal smInternal = LocalServices.getService(StorageManagerInternal.class);













        smInternal.markCeStoragePrepared(userId);





























        mPm.reconcileAppsData(userId, StorageManager.FLAG_STORAGE_CE, migrateAppsData);















    }