Merge "UserDataPreparer: be more careful about auto-deleting data on error" into main (2506df92) · Commits · e / os / android_frameworks_base

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

+36 −18

Original line number	Diff line number	Diff line
		@@ -23,10 +23,10 @@ import android.content.pm.UserInfo;
		import android.os.Environment;
		import android.os.FileUtils;
		import android.os.RecoverySystem;
		import android.os.storage.StorageManager;
		import android.os.storage.VolumeInfo;
		import android.os.SystemProperties;
		import android.os.UserHandle;
		import android.os.storage.StorageManager;
		import android.os.storage.VolumeInfo;
		import android.system.ErrnoException;
		import android.system.Os;
		import android.system.OsConstants;
		@@ -35,6 +35,7 @@ import android.util.Slog;
		import android.util.SparseArray;

		import com.android.internal.annotations.VisibleForTesting;
		import com.android.server.utils.Slogf;

		import java.io.File;
		import java.io.IOException;
		@@ -43,7 +44,6 @@ import java.util.ArrayList;
		import java.util.Collections;
		import java.util.List;
		import java.util.Objects;
		import java.util.Set;

		/**
		* Helper class for preparing and destroying user storage
		@@ -65,31 +65,37 @@ class UserDataPreparer {
		/**
		* Prepare storage areas for given user on all mounted devices.
		*/
		void prepareUserData(int userId, int userSerial, int flags) {
		void prepareUserData(UserInfo userInfo, int flags) {
		synchronized (mInstallLock) {
		final StorageManager storage = mContext.getSystemService(StorageManager.class);
		/*
		* Internal storage must be prepared before adoptable storage, since the user's volume
		* keys are stored in their internal storage.
		*/
		prepareUserDataLI(null /* internal storage */, userId, userSerial, flags, true);
		prepareUserDataLI(null /* internal storage */, userInfo, flags, true);
		for (VolumeInfo vol : storage.getWritablePrivateVolumes()) {
		final String volumeUuid = vol.getFsUuid();
		if (volumeUuid != null) {
		prepareUserDataLI(volumeUuid, userId, userSerial, flags, true);
		prepareUserDataLI(volumeUuid, userInfo, flags, true);
		}
		}
		}
		}

		private void prepareUserDataLI(String volumeUuid, int userId, int userSerial, int flags,
		private void prepareUserDataLI(String volumeUuid, UserInfo userInfo, int flags,
		boolean allowRecover) {
		// Prepare storage and verify that serial numbers are consistent; if
		// there's a mismatch we need to destroy to avoid leaking data
		final int userId = userInfo.id;
		final int userSerial = userInfo.serialNumber;
		final StorageManager storage = mContext.getSystemService(StorageManager.class);
		final boolean isNewUser = userInfo.lastLoggedInTime == 0;
		Slogf.d(TAG, "Preparing user data; volumeUuid=%s, userId=%d, flags=0x%x, isNewUser=%s",
		volumeUuid, userId, flags, isNewUser);
		try {
		// Prepare CE and/or DE storage.
		storage.prepareUserStorage(volumeUuid, userId, userSerial, flags);

		// Ensure that the data directories of a removed user with the same ID are not being
		// reused. New users must get fresh data directories, to avoid leaking data.
		if ((flags & StorageManager.FLAG_STORAGE_DE) != 0) {
		enforceSerialNumber(getDataUserDeDirectory(volumeUuid, userId), userSerial);
		if (Objects.equals(volumeUuid, StorageManager.UUID_PRIVATE_INTERNAL)) {
		@@ -103,9 +109,10 @@ class UserDataPreparer {
		}
		}

		// Prepare the app data directories.
		mInstaller.createUserData(volumeUuid, userId, userSerial, flags);

		// CE storage is available after they are prepared.
		// If applicable, record that the system user's CE storage has been prepared.
		if ((flags & StorageManager.FLAG_STORAGE_CE) != 0 &&
		(userId == UserHandle.USER_SYSTEM)) {
		String propertyName = "sys.user." + userId + ".ce_available";
		@@ -113,20 +120,31 @@ class UserDataPreparer {
		SystemProperties.set(propertyName, "true");
		}
		} catch (Exception e) {
		logCriticalInfo(Log.WARN, "Destroying user " + userId + " on volume " + volumeUuid
		// Failed to prepare user data. For new users, specifically users that haven't ever
		// been unlocked, destroy the user data, and try again (if not already retried). This
		// might be effective at resolving some errors, such as stale directories from a reused
		// user ID. Don't auto-destroy data for existing users, since issues with existing
		// users might be fixable via an OTA without having to wipe the user's data.
		if (isNewUser) {
		logCriticalInfo(Log.ERROR, "Destroying user " + userId + " on volume " + volumeUuid
		+ " because we failed to prepare: " + e);
		destroyUserDataLI(volumeUuid, userId, flags);

		} else {
		logCriticalInfo(Log.ERROR, "Failed to prepare user " + userId + " on volume "
		+ volumeUuid + ": " + e);
		}
		if (allowRecover) {
		// Try one last time; if we fail again we're really in trouble
		prepareUserDataLI(volumeUuid, userId, userSerial,
		flags \| StorageManager.FLAG_STORAGE_DE, false);
		prepareUserDataLI(volumeUuid, userInfo, flags \| StorageManager.FLAG_STORAGE_DE,
		false);
		} else {
		// If internal storage of the system user fails to prepare on first boot, then
		// things are really broken, so we might as well reboot to recovery right away.
		try {
		Log.wtf(TAG, "prepareUserData failed for user " + userId, e);
		if (userId == UserHandle.USER_SYSTEM) {
		if (isNewUser && userId == UserHandle.USER_SYSTEM && volumeUuid == null) {
		RecoverySystem.rebootPromptAndWipeUserData(mContext,
		"prepareUserData failed for system user");
		"failed to prepare internal storage for system user");
		}
		} catch (IOException e2) {
		throw new RuntimeException("error rebooting into recovery", e2);

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

+3 −6

Original line number	Diff line number	Diff line
		@@ -4914,8 +4914,7 @@ public class UserManagerService extends IUserManager.Stub {
		// unlocked. We do this to ensure that CE storage isn't prepared before the CE key is
		// saved to disk. This also matches what is done for user 0.
		t.traceBegin("prepareUserData");
		mUserDataPreparer.prepareUserData(userId, userInfo.serialNumber,
		StorageManager.FLAG_STORAGE_DE);
		mUserDataPreparer.prepareUserData(userInfo, StorageManager.FLAG_STORAGE_DE);
		t.traceEnd();

		t.traceBegin("LSS.createNewUser");
		@@ -6199,12 +6198,11 @@ public class UserManagerService extends IUserManager.Stub {
		}
		TimingsTraceAndSlog t = new TimingsTraceAndSlog();
		t.traceBegin("onBeforeStartUser-" + userId);
		final int userSerial = userInfo.serialNumber;
		// Migrate only if build fingerprints mismatch
		boolean migrateAppsData = !PackagePartitions.FINGERPRINT.equals(
		userInfo.lastLoggedInFingerprint);
		t.traceBegin("prepareUserData");
		mUserDataPreparer.prepareUserData(userId, userSerial, StorageManager.FLAG_STORAGE_DE);
		mUserDataPreparer.prepareUserData(userInfo, StorageManager.FLAG_STORAGE_DE);
		t.traceEnd();
		t.traceBegin("reconcileAppsData");
		getPackageManagerInternal().reconcileAppsData(userId, StorageManager.FLAG_STORAGE_DE,
		@@ -6230,14 +6228,13 @@ public class UserManagerService extends IUserManager.Stub {
		if (userInfo == null) {
		return;
		}
		final int userSerial = userInfo.serialNumber;
		// Migrate only if build fingerprints mismatch
		boolean migrateAppsData = !PackagePartitions.FINGERPRINT.equals(
		userInfo.lastLoggedInFingerprint);

		final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
		t.traceBegin("prepareUserData-" + userId);
		mUserDataPreparer.prepareUserData(userId, userSerial, StorageManager.FLAG_STORAGE_CE);
		mUserDataPreparer.prepareUserData(userInfo, StorageManager.FLAG_STORAGE_CE);
		t.traceEnd();

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

services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/UserDataPreparerTest.java

+29 −4

Original line number	Diff line number	Diff line
		@@ -21,6 +21,8 @@ import static org.junit.Assert.assertFalse;
		import static org.junit.Assert.assertTrue;
		import static org.mockito.Matchers.eq;
		import static org.mockito.Matchers.isNull;
		import static org.mockito.Mockito.doThrow;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.when;

		@@ -56,6 +58,7 @@ public class UserDataPreparerTest {

		private static final int TEST_USER_SERIAL = 1000;
		private static final int TEST_USER_ID = 10;
		private static final UserInfo TEST_USER = new UserInfo();

		private TestUserDataPreparer mUserDataPreparer;

		@@ -72,6 +75,8 @@ public class UserDataPreparerTest {

		@Before
		public void setup() {
		TEST_USER.id = TEST_USER_ID;
		TEST_USER.serialNumber = TEST_USER_SERIAL;
		Context ctx = InstrumentationRegistry.getContext();
		FileUtils.deleteContents(ctx.getCacheDir());
		mInstallLock = new Object();
		@@ -92,8 +97,7 @@ public class UserDataPreparerTest {
		userDeDir.mkdirs();
		File systemDeDir = mUserDataPreparer.getDataSystemDeDirectory(TEST_USER_ID);
		systemDeDir.mkdirs();
		mUserDataPreparer
		.prepareUserData(TEST_USER_ID, TEST_USER_SERIAL, StorageManager.FLAG_STORAGE_DE);
		mUserDataPreparer.prepareUserData(TEST_USER, StorageManager.FLAG_STORAGE_DE);
		verify(mStorageManagerMock).prepareUserStorage(isNull(String.class), eq(TEST_USER_ID),
		eq(TEST_USER_SERIAL), eq(StorageManager.FLAG_STORAGE_DE));
		verify(mInstaller).createUserData(isNull(String.class), eq(TEST_USER_ID),
		@@ -110,8 +114,7 @@ public class UserDataPreparerTest {
		userCeDir.mkdirs();
		File systemCeDir = mUserDataPreparer.getDataSystemCeDirectory(TEST_USER_ID);
		systemCeDir.mkdirs();
		mUserDataPreparer
		.prepareUserData(TEST_USER_ID, TEST_USER_SERIAL, StorageManager.FLAG_STORAGE_CE);
		mUserDataPreparer.prepareUserData(TEST_USER, StorageManager.FLAG_STORAGE_CE);
		verify(mStorageManagerMock).prepareUserStorage(isNull(String.class), eq(TEST_USER_ID),
		eq(TEST_USER_SERIAL), eq(StorageManager.FLAG_STORAGE_CE));
		verify(mInstaller).createUserData(isNull(String.class), eq(TEST_USER_ID),
		@@ -122,6 +125,28 @@ public class UserDataPreparerTest {
		assertEquals(TEST_USER_SERIAL, serialNumber);
		}

		@Test
		public void testPrepareUserData_forNewUser_destroysOnFailure() throws Exception {
		TEST_USER.lastLoggedInTime = 0;
		doThrow(new IllegalStateException("expected exception for test")).when(mStorageManagerMock)
		.prepareUserStorage(isNull(String.class), eq(TEST_USER_ID), eq(TEST_USER_SERIAL),
		eq(StorageManager.FLAG_STORAGE_CE));
		mUserDataPreparer.prepareUserData(TEST_USER, StorageManager.FLAG_STORAGE_CE);
		verify(mStorageManagerMock).destroyUserStorage(isNull(String.class), eq(TEST_USER_ID),
		eq(StorageManager.FLAG_STORAGE_CE));
		}

		@Test
		public void testPrepareUserData_forExistingUser_doesNotDestroyOnFailure() throws Exception {
		TEST_USER.lastLoggedInTime = System.currentTimeMillis();
		doThrow(new IllegalStateException("expected exception for test")).when(mStorageManagerMock)
		.prepareUserStorage(isNull(String.class), eq(TEST_USER_ID), eq(TEST_USER_SERIAL),
		eq(StorageManager.FLAG_STORAGE_CE));
		mUserDataPreparer.prepareUserData(TEST_USER, StorageManager.FLAG_STORAGE_CE);
		verify(mStorageManagerMock, never()).destroyUserStorage(isNull(String.class),
		eq(TEST_USER_ID), eq(StorageManager.FLAG_STORAGE_CE));
		}

		@Test
		public void testDestroyUserData_De_DoesNotDestroyCe() throws Exception {
		// Add file in CE storage