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

Commit 634c34ea authored by Pavel Grafov's avatar Pavel Grafov
Browse files

Serialize key eviction vs. user storage preparation

UserManagerService.onBeforeUnlockUser requires unlocked user key
and is executed on FgThread. Currently key may be locked from
a different thread: UserController.finishUserStopped is executed in
mHandler. This changes moves lockUserKey part to FgThread, so that
key state can be reliably checked before starting onBeforeUnlockUser.

In the worst case user will be RUNNING_LOCKED with "Some functionality
may be limited" warning and unable to start apps. But that seems
fairly harmless.

+ got rid of redundant boolean in finishUserStopped.

Fixes: 72334925
Test: Turn work mode on and off ad nauseam
Test cts-tradefed run commandAndExit cts-dev -m CtsDevicePolicyManagerTestCases
     -t com.android.cts.devicepolicy.ManagedProfileTest
Test: creating another user and swithching back and forth
Test: exercising DPMS.lockNow(FLAG_EVICT_CREDENTIAL_ENCRYPTION_KEY) via TestDPC

Change-Id: I01d4dea183fd1a35a2e47284c7a544725e8a871f
parent 5037beea
Loading
Loading
Loading
Loading
+25 −13
Original line number Diff line number Diff line
@@ -400,6 +400,10 @@ class UserController implements Handler.Callback {

        // Call onBeforeUnlockUser on a worker thread that allows disk I/O
        FgThread.getHandler().post(() -> {
            if (!StorageManager.isUserKeyUnlocked(userId)) {
                Slog.w(TAG, "User key got locked unexpectedly, leaving user locked.");
                return;
            }
            mInjector.getUserManager().onBeforeUnlockUser(userId);
            synchronized (mLock) {
                // Do not proceed if unexpected state
@@ -714,14 +718,11 @@ class UserController implements Handler.Callback {

    void finishUserStopped(UserState uss) {
        final int userId = uss.mHandle.getIdentifier();
        boolean stopped;
        final boolean stopped;
        ArrayList<IStopUserCallback> callbacks;
        boolean forceStopUser = false;
        synchronized (mLock) {
            callbacks = new ArrayList<>(uss.mStopCallbacks);
            if (mStartedUsers.get(userId) != uss) {
                stopped = false;
            } else if (uss.state != UserState.STATE_SHUTDOWN) {
            if (mStartedUsers.get(userId) != uss || uss.state != UserState.STATE_SHUTDOWN) {
                stopped = false;
            } else {
                stopped = true;
@@ -729,10 +730,10 @@ class UserController implements Handler.Callback {
                mStartedUsers.remove(userId);
                mUserLru.remove(Integer.valueOf(userId));
                updateStartedUserArrayLU();
                forceStopUser = true;
            }
        }
        if (forceStopUser) {

        if (stopped) {
            mInjector.getUserManagerInternal().removeUserState(userId);
            mInjector.activityManagerOnUserStopped(userId);
            // Clean up all state and processes associated with the user.
@@ -755,12 +756,23 @@ class UserController implements Handler.Callback {
            if (getUserInfo(userId).isEphemeral()) {
                mInjector.getUserManager().removeUserEvenWhenDisallowed(userId);
            }
            // Evict the user's credential encryption key.

            // Evict the user's credential encryption key. Performed on FgThread to make it
            // serialized with call to UserManagerService.onBeforeUnlockUser in finishUserUnlocking
            // to prevent data corruption.
            FgThread.getHandler().post(() -> {
                synchronized (mLock) {
                    if (mStartedUsers.get(userId) != null) {
                        Slog.w(TAG, "User was restarted, skipping key eviction");
                        return;
                    }
                }
                try {
                    getStorageManager().lockUserKey(userId);
                } catch (RemoteException re) {
                    throw re.rethrowAsRuntimeException();
                }
            });
        }
    }