From 5bb19537c24d3ef6675487bc9870813c8d769b5a Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 13 Oct 2021 09:13:08 -0700 Subject: [PATCH] Drop persisted ledgers for uninstalled packages. Make sure we don't continue to include data from missing apps when loading data from disk. This scenario could arise if an app is uninstalled and the device shuts down before we have a chance to persist the change. Bug: 158300259 Test: atest FrameworksMockingServicesTests:ScribeTest Change-Id: Ibebd5296a5e1968ceeb916fdc076fad994c48373 --- .../java/com/android/server/tare/Scribe.java | 63 ++++++++++++++----- .../com/android/server/tare/ScribeTest.java | 56 ++++++++++++++--- 2 files changed, 93 insertions(+), 26 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/tare/Scribe.java b/apex/jobscheduler/service/java/com/android/server/tare/Scribe.java index 48a373b2c9244..d4c6c8c269482 100644 --- a/apex/jobscheduler/service/java/com/android/server/tare/Scribe.java +++ b/apex/jobscheduler/service/java/com/android/server/tare/Scribe.java @@ -22,13 +22,16 @@ import static com.android.server.tare.TareUtils.appToString; import android.annotation.NonNull; import android.annotation.Nullable; -import android.hardware.biometrics.face.V1_0.UserHandle; +import android.content.pm.PackageInfo; import android.os.Environment; +import android.os.UserHandle; +import android.util.ArraySet; import android.util.AtomicFile; import android.util.IndentingPrintWriter; import android.util.Log; import android.util.Pair; import android.util.Slog; +import android.util.SparseArray; import android.util.SparseArrayMap; import android.util.TypedXmlPullParser; import android.util.TypedXmlSerializer; @@ -36,8 +39,6 @@ import android.util.Xml; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.server.LocalServices; -import com.android.server.pm.UserManagerInternal; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -47,7 +48,6 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; /** @@ -161,10 +161,20 @@ public class Scribe { return; } - UserManagerInternal userManagerInternal = - LocalServices.getService(UserManagerInternal.class); - final int[] userIds = userManagerInternal.getUserIds(); - Arrays.sort(userIds); + final SparseArray> installedPackagesPerUser = new SparseArray<>(); + final List installedPackages = mIrs.getInstalledPackages(); + for (int i = 0; i < installedPackages.size(); ++i) { + final PackageInfo packageInfo = installedPackages.get(i); + if (packageInfo.applicationInfo != null) { + final int userId = UserHandle.getUserId(packageInfo.applicationInfo.uid); + ArraySet pkgsForUser = installedPackagesPerUser.get(userId); + if (pkgsForUser == null) { + pkgsForUser = new ArraySet<>(); + installedPackagesPerUser.put(userId, pkgsForUser); + } + pkgsForUser.add(packageInfo.packageName); + } + } try (FileInputStream fis = mStateFile.openRead()) { TypedXmlPullParser parser = Xml.resolvePullParser(fis); @@ -209,7 +219,8 @@ public class Scribe { break; case XML_TAG_USER: earliestEndTime = Math.min(earliestEndTime, - readUserFromXmlLocked(parser, userIds, endTimeCutoff)); + readUserFromXmlLocked( + parser, installedPackagesPerUser, endTimeCutoff)); break; default: Slog.e(TAG, "Unexpected tag: " + tagName); @@ -280,7 +291,8 @@ public class Scribe { */ @Nullable private static Pair readLedgerFromXml(TypedXmlPullParser parser, - long endTimeCutoff) throws XmlPullParserException, IOException { + ArraySet validPackages, long endTimeCutoff) + throws XmlPullParserException, IOException { final String pkgName; final long curBalance; final List transactions = new ArrayList<>(); @@ -288,6 +300,13 @@ public class Scribe { pkgName = parser.getAttributeValue(null, XML_ATTR_PACKAGE_NAME); curBalance = parser.getAttributeLong(null, XML_ATTR_CURRENT_BALANCE); + final boolean isInstalled = validPackages.contains(pkgName); + if (!isInstalled) { + // Don't return early since we need to go through all the transaction tags and get + // to the end of the ledger tag. + Slog.w(TAG, "Invalid pkg " + pkgName + " is saved to disk"); + } + for (int eventType = parser.next(); eventType != XmlPullParser.END_DOCUMENT; eventType = parser.next()) { final String tagName = parser.getName(); @@ -298,11 +317,14 @@ public class Scribe { } continue; } - if (eventType != XmlPullParser.START_TAG || !"transaction".equals(tagName)) { + if (eventType != XmlPullParser.START_TAG || !XML_TAG_TRANSACTION.equals(tagName)) { // Expecting only "transaction" tags. Slog.e(TAG, "Unexpected event: (" + eventType + ") " + tagName); return null; } + if (!isInstalled) { + continue; + } if (DEBUG) { Slog.d(TAG, "Starting ledger tag: " + tagName); } @@ -320,6 +342,9 @@ public class Scribe { transactions.add(new Ledger.Transaction(startTime, endTime, eventId, tag, delta)); } + if (!isInstalled) { + return null; + } return Pair.create(pkgName, new Ledger(curBalance, transactions)); } @@ -329,12 +354,14 @@ public class Scribe { * @return The earliest valid transaction end time found for the user. */ @GuardedBy("mIrs.getLock()") - private long readUserFromXmlLocked(TypedXmlPullParser parser, int[] validUserIds, + private long readUserFromXmlLocked(TypedXmlPullParser parser, + SparseArray> installedPackagesPerUser, long endTimeCutoff) throws XmlPullParserException, IOException { int curUser = parser.getAttributeInt(null, XML_ATTR_USER_ID); - if (Arrays.binarySearch(validUserIds, curUser) < 0) { + final ArraySet installedPackages = installedPackagesPerUser.get(curUser); + if (installedPackages == null) { Slog.w(TAG, "Invalid user " + curUser + " is saved to disk"); - curUser = UserHandle.NONE; + curUser = UserHandle.USER_NULL; // Don't return early since we need to go through all the ledger tags and get to the end // of the user tag. } @@ -351,10 +378,14 @@ public class Scribe { continue; } if (XML_TAG_LEDGER.equals(tagName)) { - if (curUser == UserHandle.NONE) { + if (curUser == UserHandle.USER_NULL) { + continue; + } + final Pair ledgerData = + readLedgerFromXml(parser, installedPackages, endTimeCutoff); + if (ledgerData == null) { continue; } - final Pair ledgerData = readLedgerFromXml(parser, endTimeCutoff); final Ledger ledger = ledgerData.second; if (ledger != null) { mLedgers.add(curUser, ledgerData.first, ledger); diff --git a/services/tests/mockingservicestests/src/com/android/server/tare/ScribeTest.java b/services/tests/mockingservicestests/src/com/android/server/tare/ScribeTest.java index e2a37eec40062..b72fc23bc16c1 100644 --- a/services/tests/mockingservicestests/src/com/android/server/tare/ScribeTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/tare/ScribeTest.java @@ -17,7 +17,6 @@ package com.android.server.tare; -import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; import static org.junit.Assert.assertEquals; @@ -26,6 +25,9 @@ import static org.junit.Assert.assertNull; import static org.mockito.Mockito.when; import android.content.Context; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageInfo; +import android.os.UserHandle; import android.util.Log; import android.util.SparseArrayMap; @@ -34,7 +36,6 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.server.LocalServices; -import com.android.server.pm.UserManagerInternal; import org.junit.After; import org.junit.Before; @@ -45,6 +46,7 @@ import org.mockito.MockitoSession; import org.mockito.quality.Strictness; import java.io.File; +import java.util.ArrayList; import java.util.List; /** @@ -63,11 +65,10 @@ public class ScribeTest { private MockitoSession mMockingSession; private Scribe mScribeUnderTest; private File mTestFileDir; + private final List mInstalledPackages = new ArrayList<>(); @Mock private InternalResourceService mIrs; - @Mock - private UserManagerInternal mUserManagerInternal; private Context getContext() { return InstrumentationRegistry.getContext(); @@ -80,16 +81,16 @@ public class ScribeTest { .strictness(Strictness.LENIENT) .mockStatic(LocalServices.class) .startMocking(); - doReturn(mUserManagerInternal) - .when(() -> LocalServices.getService(UserManagerInternal.class)); when(mIrs.getLock()).thenReturn(new Object()); when(mIrs.isEnabled()).thenReturn(true); - when(mUserManagerInternal.getUserIds()).thenReturn(new int[]{TEST_USER_ID}); + when(mIrs.getInstalledPackages()).thenReturn(mInstalledPackages); mTestFileDir = new File(getContext().getFilesDir(), "scribe_test"); //noinspection ResultOfMethodCallIgnored mTestFileDir.mkdirs(); Log.d(TAG, "Saving data to '" + mTestFileDir + "'"); mScribeUnderTest = new Scribe(mIrs, mTestFileDir); + + addInstalledPackage(TEST_USER_ID, TEST_PACKAGE); } @After @@ -148,13 +149,11 @@ public class ScribeTest { final SparseArrayMap ledgers = new SparseArrayMap<>(); final int numUsers = 3; final int numLedgers = 5; - final int[] userIds = new int[numUsers]; - when(mUserManagerInternal.getUserIds()).thenReturn(userIds); for (int u = 0; u < numUsers; ++u) { final int userId = TEST_USER_ID + u; - userIds[u] = userId; for (int l = 0; l < numLedgers; ++l) { final String pkgName = TEST_PACKAGE + l; + addInstalledPackage(userId, pkgName); final Ledger ledger = mScribeUnderTest.getLedgerLocked(userId, pkgName); ledger.recordTransaction(new Ledger.Transaction( 0, 1000L * u + l, 1, null, 51L * u + l)); @@ -192,6 +191,34 @@ public class ScribeTest { mScribeUnderTest.getLedgerLocked(TEST_USER_ID, TEST_PACKAGE)); } + @Test + public void testLoadingMissingPackageFromDisk() { + final String pkgName = TEST_PACKAGE + ".uninstalled"; + final Ledger ogLedger = mScribeUnderTest.getLedgerLocked(TEST_USER_ID, pkgName); + ogLedger.recordTransaction(new Ledger.Transaction(0, 1000, 1, null, 51)); + ogLedger.recordTransaction(new Ledger.Transaction(1500, 2000, 2, "green", 52)); + ogLedger.recordTransaction(new Ledger.Transaction(2500, 3000, 3, "blue", 3)); + mScribeUnderTest.writeImmediatelyForTesting(); + + // Package isn't installed, so make sure it's not saved to memory after loading. + mScribeUnderTest.loadFromDiskLocked(); + assertLedgersEqual(new Ledger(), mScribeUnderTest.getLedgerLocked(TEST_USER_ID, pkgName)); + } + + @Test + public void testLoadingMissingUserFromDisk() { + final int userId = TEST_USER_ID + 1; + final Ledger ogLedger = mScribeUnderTest.getLedgerLocked(userId, TEST_PACKAGE); + ogLedger.recordTransaction(new Ledger.Transaction(0, 1000, 1, null, 51)); + ogLedger.recordTransaction(new Ledger.Transaction(1500, 2000, 2, "green", 52)); + ogLedger.recordTransaction(new Ledger.Transaction(2500, 3000, 3, "blue", 3)); + mScribeUnderTest.writeImmediatelyForTesting(); + + // User doesn't show up with any packages, so make sure nothing is saved after loading. + mScribeUnderTest.loadFromDiskLocked(); + assertLedgersEqual(new Ledger(), mScribeUnderTest.getLedgerLocked(userId, TEST_PACKAGE)); + } + private void assertLedgersEqual(Ledger expected, Ledger actual) { if (expected == null) { assertNull(actual); @@ -219,4 +246,13 @@ public class ScribeTest { assertEquals(expected.tag, actual.tag); assertEquals(expected.delta, actual.delta); } + + private void addInstalledPackage(int userId, String pkgName) { + PackageInfo pkgInfo = new PackageInfo(); + pkgInfo.packageName = pkgName; + ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.uid = UserHandle.getUid(userId, Math.abs(pkgName.hashCode())); + pkgInfo.applicationInfo = applicationInfo; + mInstalledPackages.add(pkgInfo); + } }