From 0a2b53f36acd3ac9da5cb9e5df4dc3eec9315666 Mon Sep 17 00:00:00 2001 From: felipeal Date: Tue, 9 Jun 2020 17:39:44 -0700 Subject: [PATCH] Add a guard on UserController to avoid user switches when not ready. UserController defines some internal variables (such as mUserSwitchUiEnabled) which are set by setInitialConfig(), which in turn is called by ActivityManagerService.systemReady(). Under normal circumstances, UserController.switch() is never called before setInitialConfig(), but it could through adb (for example, when calling 'adb shell am switch-user'). In particular, CtsDevicePolicyManagerTestCases uses switch-user to switch users, and it retries on failure. So, if the system crashes while it retries, the next call might happen before setInitialConfig() is called, which could leave the system in a bad state (for example, if the OEM overrides config_customUserSwitchUi with true, it might end up in a state where mTargetUserId is not reset, so the user could never be switched again). Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.MixedDeviceOwnerTest Fixes: 153115123 Change-Id: I9ea2062d32cd2b4f328ca877649a94d670b0b1b1 --- .../com/android/server/am/UserController.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java index ea7059894f381..54c161ef61dac 100644 --- a/services/core/java/com/android/server/am/UserController.java +++ b/services/core/java/com/android/server/am/UserController.java @@ -348,6 +348,17 @@ class UserController implements Handler.Callback { @GuardedBy("mUserIdToUserJourneyMap") private final SparseArray mUserIdToUserJourneyMap = new SparseArray<>(); + /** + * Sets on {@link #setInitialConfig(boolean, int, boolean)}, which is called by + * {@code ActivityManager} when the system is started. + * + *

It's useful to ignore external operations (i.e., originated outside {@code system_server}, + * like from {@code adb shell am switch-user})) that could happen before such call is made and + * the system is ready. + */ + @GuardedBy("mLock") + private boolean mInitialized; + UserController(ActivityManagerService service) { this(new Injector(service)); } @@ -372,6 +383,7 @@ class UserController implements Handler.Callback { mUserSwitchUiEnabled = userSwitchUiEnabled; mMaxRunningUsers = maxRunningUsers; mDelayUserDataLocking = delayUserDataLocking; + mInitialized = true; } } @@ -1587,6 +1599,11 @@ class UserController implements Handler.Callback { } boolean userSwitchUiEnabled; synchronized (mLock) { + if (!mInitialized) { + Slog.e(TAG, "Cannot switch to User #" + targetUserId + + ": UserController not ready yet"); + return false; + } mTargetUserId = targetUserId; userSwitchUiEnabled = mUserSwitchUiEnabled; } @@ -2422,6 +2439,7 @@ class UserController implements Handler.Callback { pw.println(" mDelayUserDataLocking:" + mDelayUserDataLocking); pw.println(" mMaxRunningUsers:" + mMaxRunningUsers); pw.println(" mUserSwitchUiEnabled:" + mUserSwitchUiEnabled); + pw.println(" mInitialized:" + mInitialized); } }