diff options
author | Eric Biggers <ebiggers@google.com> | 2022-03-04 00:07:43 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-05-16 21:11:18 +0000 |
commit | 16cf824d3a3dab638698ffaa995621ae18cfcf4e (patch) | |
tree | 6def71abce8699929f843d9ca10951d2950f37c7 | |
parent | 6668fcafe221934d94c83eea534caeeddc1e8587 (diff) | |
download | base-16cf824d3a3dab638698ffaa995621ae18cfcf4e.tar.gz |
Ignore errors preparing user storage for existing users
Unfortunately we can't rule out the existence of devices where the user
storage wasn't properly prepared, due to StorageManagerService
previously ignoring errors from mVold.prepareUserStorage, combined with
OEMs potentially creating files in per-user directories too early. And
forcing these broken devices to be factory reset upon taking an OTA is
not currently considered to be acceptable.
One option is to only check for prepareUserStorage errors on devices
that launched with T or later. However, this is a serious issue and it
would be strongly preferable to do more than that.
Therefore, this CL makes it so that errors are checked for all new
users, rather than all new devices. A field ignorePrepareStorageErrors
is added to the user record; it is only ever set to true implicitly,
when reading a user record from disk that lacks this field. This field
is used by StorageManagerService to decide whether to check for errors.
Bug: 164488924
Bug: 224585613
Test: Intentionally made a device affected by this issue by reverting
the CLs that introduced the error checks, and changing vold to
inject an error into prepareUserStorage. Then, flashed a build
with this CL without wiping userdata. The device still boots, as
expected, and the log shows that the error was intentionally
ignored. Tested that if a second user is added, the error is
*not* ignored and the second user's storage is destroyed before it
can be used. Finally, wiped the device and verified that it won't
boot up anymore, as expected since error checking is enabled for
the system user in that case.
Change-Id: I9bdd1a4bf5b14542adb901f264a91d489115c89b
(cherry picked from commit 60d8318c47b7b659716d71243d087b34ab327f64)
Merged-In: I9bdd1a4bf5b14542adb901f264a91d489115c89b
(cherry picked from commit e667b604021e543436aabece21dc78d19fe9948e)
Merged-In: I9bdd1a4bf5b14542adb901f264a91d489115c89b
3 files changed, 60 insertions, 1 deletions
diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 546d008857bb..e5841f9e7a59 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -3400,11 +3400,20 @@ class StorageManagerService extends IStorageManager.Stub mInstaller.tryMountDataMirror(volumeUuid); } } - } catch (RemoteException | Installer.InstallerException e) { + } catch (Exception e) { Slog.wtf(TAG, e); // Make sure to re-throw this exception; we must not ignore failure // to prepare the user storage as it could indicate that encryption // wasn't successfully set up. + // + // Very unfortunately, these errors need to be ignored for broken + // users that already existed on-disk from older Android versions. + UserManagerInternal umInternal = LocalServices.getService(UserManagerInternal.class); + if (umInternal.shouldIgnorePrepareStorageErrors(userId)) { + Slog.wtf(TAG, "ignoring error preparing storage for existing user " + userId + + "; device may be insecure!"); + return; + } throw new RuntimeException(e); } } diff --git a/services/core/java/com/android/server/pm/UserManagerInternal.java b/services/core/java/com/android/server/pm/UserManagerInternal.java index eb2de6012745..0e6d5e5ed463 100644 --- a/services/core/java/com/android/server/pm/UserManagerInternal.java +++ b/services/core/java/com/android/server/pm/UserManagerInternal.java @@ -312,4 +312,12 @@ public abstract class UserManagerInternal { */ public abstract void setDefaultCrossProfileIntentFilters( @UserIdInt int parentUserId, @UserIdInt int profileUserId); + + /** + * Returns {@code true} if the system should ignore errors when preparing + * the storage directories for the user with ID {@code userId}. This will + * return {@code false} for all new users; it will only return {@code true} + * for users that already existed on-disk from an older version of Android. + */ + public abstract boolean shouldIgnorePrepareStorageErrors(int userId); } diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 6d8137e74061..09c6b50571d4 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -204,6 +204,8 @@ public class UserManagerService extends IUserManager.Stub { private static final String TAG_SEED_ACCOUNT_OPTIONS = "seedAccountOptions"; private static final String TAG_LAST_REQUEST_QUIET_MODE_ENABLED_CALL = "lastRequestQuietModeEnabledCall"; + private static final String TAG_IGNORE_PREPARE_STORAGE_ERRORS = + "ignorePrepareStorageErrors"; private static final String ATTR_KEY = "key"; private static final String ATTR_VALUE_TYPE = "type"; private static final String ATTR_MULTIPLE = "m"; @@ -313,6 +315,14 @@ public class UserManagerService extends IUserManager.Stub { private long mLastRequestQuietModeEnabledMillis; + /** + * {@code true} if the system should ignore errors when preparing the + * storage directories for this user. This is {@code false} for all new + * users; it will only be {@code true} for users that already existed + * on-disk from an older version of Android. + */ + private boolean mIgnorePrepareStorageErrors; + void setLastRequestQuietModeEnabledMillis(long millis) { mLastRequestQuietModeEnabledMillis = millis; } @@ -321,6 +331,14 @@ public class UserManagerService extends IUserManager.Stub { return mLastRequestQuietModeEnabledMillis; } + boolean getIgnorePrepareStorageErrors() { + return mIgnorePrepareStorageErrors; + } + + void setIgnorePrepareStorageErrors() { + mIgnorePrepareStorageErrors = true; + } + void clearSeedAccountData() { seedAccountName = null; seedAccountType = null; @@ -3177,6 +3195,10 @@ public class UserManagerService extends IUserManager.Stub { serializer.endTag(/* namespace */ null, TAG_LAST_REQUEST_QUIET_MODE_ENABLED_CALL); } + serializer.startTag(/* namespace */ null, TAG_IGNORE_PREPARE_STORAGE_ERRORS); + serializer.text(String.valueOf(userData.getIgnorePrepareStorageErrors())); + serializer.endTag(/* namespace */ null, TAG_IGNORE_PREPARE_STORAGE_ERRORS); + serializer.endTag(null, TAG_USER); serializer.endDocument(); @@ -3286,6 +3308,7 @@ public class UserManagerService extends IUserManager.Stub { Bundle legacyLocalRestrictions = null; RestrictionsSet localRestrictions = null; Bundle globalRestrictions = null; + boolean ignorePrepareStorageErrors = true; // default is true for old users final TypedXmlPullParser parser = Xml.resolvePullParser(is); int type; @@ -3364,6 +3387,11 @@ public class UserManagerService extends IUserManager.Stub { if (type == XmlPullParser.TEXT) { lastRequestQuietModeEnabledTimestamp = Long.parseLong(parser.getText()); } + } else if (TAG_IGNORE_PREPARE_STORAGE_ERRORS.equals(tag)) { + type = parser.next(); + if (type == XmlPullParser.TEXT) { + ignorePrepareStorageErrors = Boolean.parseBoolean(parser.getText()); + } } } } @@ -3391,6 +3419,9 @@ public class UserManagerService extends IUserManager.Stub { userData.persistSeedData = persistSeedData; userData.seedAccountOptions = seedAccountOptions; userData.setLastRequestQuietModeEnabledMillis(lastRequestQuietModeEnabledTimestamp); + if (ignorePrepareStorageErrors) { + userData.setIgnorePrepareStorageErrors(); + } synchronized (mRestrictionsLock) { if (baseRestrictions != null) { @@ -5232,6 +5263,9 @@ public class UserManagerService extends IUserManager.Stub { pw.println(); } } + + pw.println(" Ignore errors preparing storage: " + + userData.getIgnorePrepareStorageErrors()); } } @@ -5721,6 +5755,14 @@ public class UserManagerService extends IUserManager.Stub { UserManagerService.this.setDefaultCrossProfileIntentFilters( profileUserId, userTypeDetails, restrictions, parentUserId); } + + @Override + public boolean shouldIgnorePrepareStorageErrors(int userId) { + synchronized (mUsersLock) { + UserData userData = mUsers.get(userId); + return userData != null && userData.getIgnorePrepareStorageErrors(); + } + } } /** |