From 37a49da1aa47f1bb23dfa26c1fa7d2720f82ac6b Mon Sep 17 00:00:00 2001 From: Keun young Park Date: Thu, 18 Jun 2020 16:01:05 -0700 Subject: [PATCH] Restore lost unlocked users after vold reset - In automotive, vold reset / resetIfBootedAndConnected() call can lose pending unlock request for a user if boot completion and unlock happens at the same time. In automotive, user unlock can happen before boot complete. - After vold reset, check for such lost user (=marked unlocking or unlocked in UserManager level but is still locked in StorageManager level) and retry unlocking. - This also undo previous CL to keep handler messages as that cannot prevent all the race, especially race of vold reset right after onUnlockUser call. But handler update is kept inside the service. - The restoration is done only for automotive device for now as phone does not have such case. Bug: 151457974 Test: try reboot in auto devices where this race can happen. Change-Id: I3b50750f5602c805e79f79f061eed44f880b25bd --- .../android/server/StorageManagerService.java | 57 +++++++++++++++++-- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index b46bebbd03d5c..6e8eca3f46cf1 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -458,6 +458,12 @@ class StorageManagerService extends IStorageManager.Stub "(?i)(^/storage/[^/]+/(?:([0-9]+)/)?Android/(?:data|media|obb|sandbox)/)([^/]+)(/.*)?"); + /** Automotive device unlockes users before system boot complete and this requires special + * handling as vold reset can lead into race conditions. When this is set, all users unlocked + * in {@code UserManager} level are unlocked after vold reset. + */ + private final boolean mIsAutomotive; + private VolumeInfo findVolumeByIdOrThrow(String id) { synchronized (mLock) { final VolumeInfo vol = mVolumes.get(id); @@ -1082,14 +1088,12 @@ class StorageManagerService extends IStorageManager.Stub Slog.d(TAG, "Thinking about reset, mBootCompleted=" + mBootCompleted + ", mDaemonConnected=" + mDaemonConnected); if (mBootCompleted && mDaemonConnected) { - final List users = mContext.getSystemService(UserManager.class).getUsers(); + final UserManager userManager = mContext.getSystemService(UserManager.class); + final List users = userManager.getUsers(); if (mIsFuseEnabled) { mStorageSessionController.onReset(mVold, () -> { - mHandler.removeMessages(H_RESET); - mHandler.removeMessages(H_VOLUME_BROADCAST); - mHandler.removeMessages(H_VOLUME_MOUNT); - mHandler.removeMessages(H_VOLUME_UNMOUNT); + mHandler.removeCallbacksAndMessages(null); }); } else { killMediaProvider(users); @@ -1097,7 +1101,9 @@ class StorageManagerService extends IStorageManager.Stub final int[] systemUnlockedUsers; synchronized (mLock) { - systemUnlockedUsers = mSystemUnlockedUsers; + // make copy as sorting can change order + systemUnlockedUsers = Arrays.copyOf(mSystemUnlockedUsers, + mSystemUnlockedUsers.length); mDisks.clear(); mVolumes.clear(); @@ -1119,6 +1125,9 @@ class StorageManagerService extends IStorageManager.Stub mVold.onUserStarted(userId); mStoraged.onUserStarted(userId); } + if (mIsAutomotive) { + restoreAllUnlockedUsers(userManager, users, systemUnlockedUsers); + } mVold.onSecureKeyguardStateChanged(mSecureKeyguardShowing); mStorageManagerInternal.onReset(mVold); } catch (Exception e) { @@ -1127,6 +1136,29 @@ class StorageManagerService extends IStorageManager.Stub } } + private void restoreAllUnlockedUsers(UserManager userManager, List allUsers, + int[] systemUnlockedUsers) throws Exception { + Arrays.sort(systemUnlockedUsers); + UserManager.invalidateIsUserUnlockedCache(); + for (UserInfo user : allUsers) { + int userId = user.id; + if (!userManager.isUserRunning(userId)) { + continue; + } + if (Arrays.binarySearch(systemUnlockedUsers, userId) >= 0) { + continue; + } + boolean unlockingOrUnlocked = userManager.isUserUnlockingOrUnlocked(userId); + if (!unlockingOrUnlocked) { + continue; + } + Slog.w(TAG, "UNLOCK_USER lost from vold reset, will retry, user:" + userId); + mVold.onUserStarted(userId); + mStoraged.onUserStarted(userId); + mHandler.obtainMessage(H_COMPLETE_UNLOCK_USER, userId).sendToTarget(); + } + } + private void onUnlockUser(int userId) { Slog.d(TAG, "onUnlockUser " + userId); @@ -1157,6 +1189,15 @@ class StorageManagerService extends IStorageManager.Stub // Record user as started so newly mounted volumes kick off events // correctly, then synthesize events for any already-mounted volumes. synchronized (mLock) { + if (mIsAutomotive) { + for (int unlockedUser : mSystemUnlockedUsers) { + if (unlockedUser == userId) { + // This can happen as restoreAllUnlockedUsers can double post the message. + Log.i(TAG, "completeUnlockUser called for already unlocked user:" + userId); + return; + } + } + } for (int i = 0; i < mVolumes.size(); i++) { final VolumeInfo vol = mVolumes.valueAt(i); if (vol.isVisibleForRead(userId) && vol.isMountedReadable()) { @@ -1822,6 +1863,9 @@ class StorageManagerService extends IStorageManager.Stub if (WATCHDOG_ENABLE) { Watchdog.getInstance().addMonitor(this); } + + mIsAutomotive = context.getPackageManager().hasSystemFeature( + PackageManager.FEATURE_AUTOMOTIVE); } /** @@ -4477,6 +4521,7 @@ class StorageManagerService extends IStorageManager.Stub pw.println("Forced scoped storage app list: " + DeviceConfig.getProperty(DeviceConfig.NAMESPACE_STORAGE_NATIVE_BOOT, PROP_FORCED_SCOPED_STORAGE_WHITELIST)); + pw.println("isAutomotive:" + mIsAutomotive); } synchronized (mObbMounts) {